Remove temporary /posts_page: Pro regression-issue suppression (#3669 revert)

[justin808]

Summary

The revert gate for the temporary /posts_page: Pro regression-issue suppression has been verified PASSED (gate evidence: #3669 (comment) — the failure-era samples have rolled out of Bencher's 64-run t-test window and the baseline reflects real timings). This PR removes the suppression entry so a confirmed regression on /posts_page: Pro files an issue again.

This is a minimal revert: IGNORED_REGRESSION_BENCHMARKS is emptied, but the general ignore machinery is kept (see decision log below). The issue's original "How to revert" steps are partially stale after #3670/#3810; this follows the updated plan.

Closes #3669

Changes

• benchmarks/lib/regression_report.rb: IGNORED_REGRESSION_BENCHMARKS is now [].freeze; the /posts_page-specific temporary-suppression comment block is replaced with a short doc of the general-purpose mechanism (exact Bencher name matching, pre-rerun short-circuit, tracking-issue requirement for any future entry).
• Specs that read IGNORED_REGRESSION_BENCHMARKS.first (regression_report_spec, plan_confirmation_spec, and the confirmation-outcome example in track_benchmarks_spec) now stub_const a sample entry ("/ignored: Pro") so they keep pinning the machinery against the now-empty production list. No coverage was deleted.
• "/posts_page: Pro" remains in track_benchmarks_spec only as an inert fixture name in report-parsing examples.

Validation

• bundle exec rspec benchmarks/spec/regression_report_spec.rb benchmarks/spec/plan_confirmation_spec.rb benchmarks/spec/track_benchmarks_spec.rb (root bundle) → 63 examples, 0 failures
• (cd react_on_rails && BUNDLE_GEMFILE=../Gemfile bundle exec rubocop) → 214 files, no offenses
• BUNDLE_GEMFILE=Gemfile bundle exec rubocop benchmarks/ → 36 files, no offenses
• script/ci-changes-detector origin/main → Benchmark scripts; recommended job: Lint (Ruby + JS)
• git grep "3669" / git grep "posts_page: Pro" → no live suppression references remain (fixture-only uses kept)
• Codex review: skipped — CLI usage limit reached (resets Jun 10 6:30 PM); self-review performed instead

Labels: none — this changes benchmark regression-reporting tooling only, fully validated by its specs; it is not a performance-sensitive runtime path, and a benchmark run would not exercise the suppression branch (it only activates when a Bencher alert fires on a main push).

Codex Decision Log

• Minimal revert scope (coordinator decision): emptied the ignore list but KEPT ignored_benchmark?, actionable_alerts, actionable_benchmarks, and plan_confirmation.rb's fully_ignored?/suppressed handling. Why: Confirm benchmark regressions on a fresh runner before filing the main issue #3810 made the ignore list load-bearing general plumbing in the confirmation gate; full machinery removal is out of scope for Revert the temporary /posts_page: Pro regression-issue suppression #3669. Maintainers may revisit whether the machinery should be removed entirely if no new suppression is ever needed.
• Specs converted to stub_const instead of deleted: the examples previously sampled the live list (.first), which is now empty. Stubbing RegressionReport::IGNORED_REGRESSION_BENCHMARKS keeps the machinery pinned independent of production contents. Note: track_benchmarks_spec.rb line 332 also sampled the live list (the issue's plan assumed it was fixture-only — stale), so it got the same conversion to avoid a nil-driven false :confirmed outcome.
• plan_confirmation.rb comments left untouched: they document the general mechanism and never claim an active suppression.

🤖 Generated with Claude Code

---

Note

Low Risk
Benchmark CI reporting tooling only; no runtime app behavior, and specs still cover the ignore path via stubs.

Overview
Reverts the temporary /posts_page: Pro regression-issue suppression (#3669): IGNORED_REGRESSION_BENCHMARKS is now [], so confirmed regressions on that benchmark can open issues again. The long /posts_page-specific comment is replaced with a short description of the general ignore mechanism (exact Bencher names, pre-rerun short-circuit, tracking issue required for future entries).

Specs that previously used IGNORED_REGRESSION_BENCHMARKS.first now stub_const a sample "/ignored: Pro" so ignore-list behavior stays covered while production has no active suppressions.

^{Reviewed by Cursor Bugbot for commit a79526d. Bugbot is set up for automated code reviews on this repo. Configure here.}

Summary by CodeRabbit

• Chores
  • Cleared the regression benchmark suppression list; all confirmed regressions will now be reported.
  • Updated test setup with deterministic test data to ensure stable regression report behavior.

Agent Merge Confidence

Mode: accelerated-rc
Score: 9/10
Auto-merge recommendation: yes
Affected areas: benchmarks regression-reporting tooling (no runtime/perf surface)
CI detector: script/ci-changes-detector origin/main -> Benchmark scripts; recommended job: Lint (Ruby + JS)
Validation run:

• bundle exec rspec benchmarks/spec/regression_report_spec.rb benchmarks/spec/plan_confirmation_spec.rb benchmarks/spec/track_benchmarks_spec.rb -> 63 examples, 0 failures
• (cd react_on_rails && BUNDLE_GEMFILE=../Gemfile bundle exec rubocop) -> no offenses; BUNDLE_GEMFILE=Gemfile bundle exec rubocop benchmarks/ -> no offenses
  Review/check gate:
• Claude review: complete for a79526d (claude-review SUCCESS), no confirmed blocker; one OPTIONAL spec-scoping nit triaged with reply + resolved
• Codex review: skipped — CLI usage limit (resets Jun 10 6:30 PM), evidence in PR body; claude-review served as the independent gate
• GitHub checks: complete for a79526d, all pass; skips are path-selection per detector
  Known residual risk: none — suppression-list removal is fully covered by the updated specs; 1-point deduction for the skipped secondary Codex pass
  Finalized by: Claude Code Review check claude-review (SUCCESS for head a79526d, GitHub Checks API record)

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[coderabbitai]

Walkthrough

This PR removes a temporary suppression for the /posts_page: Pro benchmark regression by emptying IGNORED_REGRESSION_BENCHMARKS in production, then updates three regression-reporting specs to stub that constant with deterministic test values instead of depending on the now-empty production list.

Changes

Revert temporary regression suppression

Layer / File(s)	Summary
Regression ignore-list cleanup benchmarks/lib/regression_report.rb	RegressionReport::IGNORED_REGRESSION_BENCHMARKS is changed from ["/posts_page: Pro"].freeze to [].freeze; comment updated to state that an empty list causes all confirmed regressions to be reported.
Test isolation for ignore-list behavior benchmarks/spec/plan_confirmation_spec.rb, benchmarks/spec/regression_report_spec.rb, benchmarks/spec/track_benchmarks_spec.rb	Three specs updated to define local ignored benchmark strings and stub RegressionReport::IGNORED_REGRESSION_BENCHMARKS with those known values, replacing direct references to the now-empty production constant and ensuring ignore-list filtering examples remain deterministic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• shakacode/react_on_rails#3668: Original PR that introduced the /posts_page: Pro suppression; this PR removes it.
• shakacode/react_on_rails#3810: Introduces RegressionReport::IGNORED_REGRESSION_BENCHMARKS in the regression confirmation pipeline; this PR changes that constant to empty and updates specs accordingly.
• shakacode/react_on_rails#3822: Also updates benchmarks/spec/track_benchmarks_spec.rb around temporarily-ignored benchmark handling and RegressionReport::IGNORED_REGRESSION_BENCHMARKS stubbing.

Suggested labels

enhancement, review-needed, full-ci, benchmark, P3

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: removing the temporary /posts_page: Pro suppression from the regression benchmarks.
Linked Issues check	✅ Passed	The PR successfully implements all coding requirements from issue #3669: empties IGNORED_REGRESSION_BENCHMARKS [#3669], updates specs to stub with sample data [#3669], and maintains the ignore machinery for general use [#3669].
Out of Scope Changes check	✅ Passed	All changes are directly scoped to reverting the /posts_page: Pro suppression and updating dependent specs; no unrelated modifications detected.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch jg/3669-revert-posts-page-suppression

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Greptile Summary

This PR removes the temporary /posts_page: Pro regression suppression by emptying IGNORED_REGRESSION_BENCHMARKS now that the failure-era Bencher samples have rolled out of the 64-run t-test window. The general ignore machinery is intentionally preserved as documented general-purpose plumbing.

• regression_report.rb: IGNORED_REGRESSION_BENCHMARKS set to [].freeze; comment updated to document the mechanism rather than the now-resolved specific case.
• Three spec files converted from reading .first on the (now-empty) production constant to using stub_const with a synthetic "/ignored: Pro" entry, keeping full coverage of the ignore machinery independent of production list contents.

Confidence Score: 5/5

Safe to merge — the change re-enables reporting for a previously suppressed benchmark route, and all suppression machinery tests are correctly pinned via stub_const.

The diff is a clean, well-scoped removal of a temporary constant value. The core constant change is a one-liner, and the three spec updates correctly use stub_const to isolate tests from production list contents — no examples were deleted and no coverage gaps were introduced. The ignore machinery itself is untouched, and the PR description matches the code exactly.

No files require special attention.

Important Files Changed

Filename	Overview
benchmarks/lib/regression_report.rb	Empties IGNORED_REGRESSION_BENCHMARKS to [].freeze and replaces the /posts_page: Pro-specific comment with a general-purpose mechanism doc; no logic changes.
benchmarks/spec/plan_confirmation_spec.rb	Converts .first on the now-empty production list to a hardcoded stub entry via stub_const, preserving all machinery coverage without depending on list contents.
benchmarks/spec/regression_report_spec.rb	Same stub_const pattern applied to the ignore-list helpers describe block; no tests removed, full machinery coverage retained.
benchmarks/spec/track_benchmarks_spec.rb	Single example updated to use a local stub_const instead of reading from the now-empty production list; /posts_page: Pro remains only as an inert fixture name in other examples.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Bencher alert fires on main push] --> B[track_benchmarks.rb writes CANDIDATE payload]
    B --> C[plan_confirmation.rb reads candidates]
    C --> D{All regressed benchmarks\nin IGNORED_REGRESSION_BENCHMARKS?}
    D -->|Yes - previously /posts_page: Pro| E[Short-circuit: no rerun, no issue\nSkipped confirmation logged]
    D -->|No - now always this path| F[Emit matrix row for confirmation rerun]
    F --> G[track_benchmarks.rb confirm run\non fresh runner]
    G --> H{Same benchmark+measure\nre-alerts?}
    H -->|Yes| I[Write CONFIRMED payload]
    H -->|No| J[Cleared - noise]
    I --> K[report_regressions.rb files/updates issue]

    style E fill:#f9f,stroke:#333
    style D fill:#ffe,stroke:#333

Loading

_{Reviews (1): Last reviewed commit: "Remove temporary /posts_page: Pro regres..." | Re-trigger Greptile}

[claude]

Code Review

Overall: Clean, minimal revert. The change is exactly scoped to what is described — emptying the suppression list while preserving the machinery. The PR description and decision log are thorough.

What the PR Does

Removes the temporary /posts_page: Pro entry from IGNORED_REGRESSION_BENCHMARKS now that Bencher's 64-run t-test window has rolled past the failure-era samples, restoring unconditional regression issue filing for that benchmark.

Code Quality

regression_report.rb change: The replacement comment is an improvement — it documents the general mechanism (purpose, short-circuit behavior, "any entry needs a tracking issue") rather than recounting the now-resolved incident. Future contributors will understand how to use the list without needing to read issue history.

Spec changes — stub_const pattern: The conversion from IGNORED_REGRESSION_BENCHMARKS.first to stub_const with a synthetic entry is correct RSpec idiom. It ensures the tests pin the machinery independently of production contents. Importantly, stub_const auto-reverts after each example, so there is no cross-test pollution.

Minor Observation

In plan_confirmation_spec.rb, the before { stub_const(...) } is at the outermost RSpec.describe level (line 36), so it runs before every example in the file — including the failure-mode tests (corrupt candidates, unmatched candidates) that have no relation to the suppression path. This is harmless because those tests use different benchmark names (/hello: Core, /x: Ghost) that are not in the stubbed list. But those tests now run against a non-empty ignore list rather than the empty production default.

The other two spec files (regression_report_spec.rb, track_benchmarks_spec.rb) scope the stub more precisely to the examples that exercise the ignore path, which is ideal. Not a blocker, just something to be aware of if a failure-mode test is ever debugged in isolation.

Correctness Check on plan_confirmation.rb line 75

suppressed.concat(Array(regressed) & RegressionReport::IGNORED_REGRESSION_BENCHMARKS)

With the list now empty, this intersection always produces [], so announce_suppressed is effectively a no-op in production — correct behavior. If a future entry is added back, this line correctly surfaces it in the workflow notice.

Validation

The PR's own validation section is solid: 63 specs + 0 failures, RuboCop clean on both Gemfiles, and git grep confirms no live suppression references remain outside of fixture-only uses.

Summary

LGTM. Minimal, well-tested, and correctly scoped. The one minor observation about stub scope in plan_confirmation_spec.rb is not a blocker.