Skip to content

tooling(lint): run_lint_gates.sh fails loudly when a lint step yields no command — it silently dropped the public-baseline step - #9969

Closed
proggeramlug wants to merge 3 commits into
PerryTS:mainfrom
proggeramlug:fix/lint-gates-extractor-loud
Closed

tooling(lint): run_lint_gates.sh fails loudly when a lint step yields no command — it silently dropped the public-baseline step#9969
proggeramlug wants to merge 3 commits into
PerryTS:mainfrom
proggeramlug:fix/lint-gates-extractor-loud

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Tooling only (scripts/run_lint_gates.sh + its self-test + a changelog fragment). Found while replaying the lint job locally on a Linux box because GitHub's runners were unavailable.

What was wrong

run_lint_gates.sh derives its gate list from .github/workflows/test.yml's lint job and then reports "all N gates passed". Its extractor only accepted commands starting with python3 scripts/, ./scripts/ or cargo fmt, so the "Public benchmark evidence freshness" step (PYTHONPATH=. python3 tests/test_public_baseline.py and python3 benchmarks/ci_public_baseline_check.py) was silently dropped: the driver derived 67 commands from 44 run: steps, ran 65, and printed green — while that step has been red on main since 2026-07-29 (the workflow's own comment). The script's header already states the failure mode: a gate you did not run is indistinguishable from one that passed.

What changes

  • The extractor derives every command of every run: step (76 commands now, all 44 steps covered); the two CI-only steps that cannot run offline (check_changeset_fragment.sh needing gh api …/pulls/N/files, ci_cargo_test_shard.py --validate needing the plan's shard count) are on an explicit, printed skip list.
  • After extraction the script asserts every run: step produced ≥ 1 command or is on that list, and exits non-zero naming the step otherwise. The derived list is printed with step names so it can be diffed against the workflow.
  • --self-test gains a positive case (a comment-led multi-line run: block is derived) and a negative case (a step yielding nothing exits 4 naming Synthetic empty run step).

Verified locally

Self-test passes. The real replay with SKIP_COMPILE_GATES=1 now lists and runs the public-baseline step: tests/test_public_baseline.py passes and benchmarks/ci_public_baseline_check.py fails with public baseline error: public artifact benchmark inputs changed; regenerate it with ./benchmarks/run_public_baseline.sh — the actual red on main, now visible to the driver. Regenerating that baseline is a separate maintenance item, not part of this PR.

Second blind spot, same driver (3e6aeef)

Replaying CI's warnings job verbatim on a PR head showed RUSTFLAGS="-D warnings" cargo check -p perry --bins failing with seven dead-code errors while the driver's compile tier reported green: the tier only ran cargo check --workspace --all-targets (tests keep those items alive) plus clippy, never the product-scope check. The compile tier is now derived from test.yml's warnings and check jobs like the lint steps: the -D warnings product check, the -D warnings workspace --all-targets check minus the host-excluded UI crates, cargo clippy -p perry --bins, the workspace clippy minus the same set, and the api-docs drift pair; the self-test asserts the compile commands derive, that an empty or missing step is rejected, and that the product warnings command is replayed. (The seven errors themselves are main's: #9970.)

https://claude.ai/code/session_011dhBmdn4vGgNibjo3oZqTo

The extractor only admitted commands beginning with `python3 scripts/`,
`./scripts/`, or `cargo fmt`. The public-baseline commands instead begin
with an inline PYTHONPATH assignment and `python3 benchmarks/`, so both
were filtered out; the YAML block's comments and step metadata were not
the cause.

Recognize every executable family currently used by lint, retain workflow
step names in the derived list, and reject any run step that produces no
commands. Keep the two GitHub-context commands on a stale-checked explicit
skip list, and self-test both comment-led extraction and the loud failure.

Claude-Session: https://claude.ai/code/session_011dhBmdn4vGgNibjo3oZqTo
@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 8 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 10d3e9a9-eee4-4044-96de-1da3bb8f13fc

📥 Commits

Reviewing files that changed from the base of the PR and between 8b7dc33 and 3e6aeef.

📒 Files selected for processing (2)
  • changelog.d/9969-lint-gates-extractor-loud.md
  • scripts/run_lint_gates.sh

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

Comment @coderabbitai help to get the list of available commands.

Derive the warnings and check job gates from test.yml, including the
product-only warnings check, both host-compatible workspace scopes, and the
API-docs regeneration and drift assertion. Expand the workflow's package
exclusions portably for macOS Bash 3.2.

Extend the extractor self-test to require the product check under -D warnings,
reject its removal, and prove newly added warnings commands are replayed.

Claude-Session: https://claude.ai/code/session_011dhBmdn4vGgNibjo3oZqTo
@proggeramlug
proggeramlug marked this pull request as ready for review September 7, 2026 20:45
proggeramlug pushed a commit that referenced this pull request Sep 7, 2026
#9969 makes run_lint_gates.sh derive every command of every run: step (80
now, up from 67). Two of the newly-run gates were red on this train:

- addr_class_inventory rejected #9973's two GcHeader casts in
  hot_diag/receiver_repr.rs. Both are allowlisted with reasons rather
  than converted: the block is a #[cfg(debug_assertions)] trust-the-tag
  audit that compares the ownership-derived header against a direct
  byte-offset read and counts disagreements. Routing the raw side through
  try_read_gc_header would validate the address first and return None for
  exactly the implausible cases the audit exists to catch, so the
  canonical predicate cannot stand in there.

- RUSTFLAGS="-D warnings" cargo check --workspace --all-targets rejected
  #9861's doc comment on a thread_local! macro invocation, which cannot
  carry one. Moved inside the macro onto the static it describes. This is
  a warning, not an error, so it only fails under -D warnings — which the
  driver never replayed before #9969.
@proggeramlug

Copy link
Copy Markdown
Contributor Author

Landed on main via merge train #9978. Validated as a tree: 77 of 80 lint gates pass, and perry-runtime/codegen/hir/stdlib all green (5,980 tests, 0 failures).

The three non-passing gates are accounted for: public-baseline is pre-existing on main (verified on a pristine worktree; red since 2026-07-29), and the two API docs gates are an artifact of this session's CARGO_TARGET_DIR override — regen_api_docs.sh hardcodes $ROOT/target/release/perry. With the binary placed where the script expects, regeneration succeeds and the drift check is clean. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant