Skip to content

refactor(rerank): split analysis_rerank.rs — it crossed the 1000-line error gate - #432

Closed
runyourempire wants to merge 2 commits into
mainfrom
worktree-rerank-file-size-split
Closed

refactor(rerank): split analysis_rerank.rs — it crossed the 1000-line error gate#432
runyourempire wants to merge 2 commits into
mainfrom
worktree-rerank-file-size-split

Conversation

@runyourempire

Copy link
Copy Markdown
Collaborator

The bug I shipped

My own #423 pushed src-tauri/src/analysis_rerank.rs from 739 to 1032 lines, past the 1000-line ERROR threshold in scripts/check-file-sizes.cjs.

Why CI didn't catch it

The size gate runs as a step inside CI's Frontend job:

# .github/workflows/validate.yml:114 (Frontend job)
- name: Check file sizes
  run: node scripts/check-file-sizes.cjs --ci

Detect changes skips the Frontend job on Rust-only PRs. #423 touched only Rust + .gitignore, so Frontend was skipped and the gate never ran. The violation landed on green CI.

The consequence: the next PR to touch any frontend file would have failed on my file, with nothing connecting the failure to the change that caused it.

The fix

Extracted the outcome-reporting and budget-pacing concern into analysis_rerank_outcome.rs, using the #[path] submodule pattern this file already uses for analysis_dedup.rs:

  • BUDGET_PACE_HEADROOM, budget_allowance_by_now, secs_into_utc_day
  • RerankSkip (+ reason/detail) and RerankOutcome (+ log)
  • the 9 pacing and skip-reporting tests covering them

These are genuinely a separate concern from "run the LLM rerank": deciding whether a pass may spend budget, and reporting honestly what a pass actually did.

analysis_rerank.rs: 1032 → 760 (below error, back near its pre-#423 739).

Deliberately NOT added to the EXCEPTIONS allowlist. Granting an exception to a file I had just bloated is precisely the quiet rule-erosion the doctrine warns against, and that list already carries a "candidates for splitting" backlog that this would have joined.

Pure code motion — no behaviour change.

Verification

  • node scripts/check-file-sizes.cjsexit 0, 44 → 43 warnings, zero errors
  • cargo fmt --check clean
  • cargo clippy -- -D warnings clean on both CI legs (default and --features experimental)
  • full lib suite: 4,300 passed / 0 failed / 10 ignored
  • full local pre-commit gate green (doc locations, IPC contracts, dead-code expiry, secret scan, no-window spawns)

Follow-up worth considering (not in this PR)

Move the file-size gate out of the Frontend job so it runs on every PR. It enforces a repo-wide invariant — Rust thresholds included — but currently only executes when frontend files change. That gap is what let this through, and it will let the next one through too.

🤖 Generated with Claude Code

… error gate

My own #423 pushed `analysis_rerank.rs` from 739 to 1032 lines, past the
1000-line ERROR threshold in scripts/check-file-sizes.cjs.

It reached main because the gate runs as a step in CI's **Frontend** job, and
`Detect changes` skips that job on Rust-only PRs. So a Rust-only change can
violate a Rust file-size rule and still go green — the next PR to touch any
frontend file would have failed on MY file, with no obvious connection to the
change that caused it.

Extracted the outcome-reporting and budget-pacing concern into
`analysis_rerank_outcome.rs` via the `#[path]` submodule pattern this file
already uses for `analysis_dedup.rs`:
  - BUDGET_PACE_HEADROOM / budget_allowance_by_now / secs_into_utc_day
  - RerankSkip (+ reason/detail) and RerankOutcome (+ log)
  - the 9 pacing and skip-reporting tests that cover them

These genuinely are a separate concern from "run the LLM rerank": deciding
whether a pass may spend budget, and reporting honestly what a pass did.

analysis_rerank.rs 1032 -> 760 (below error, and back near its pre-#423 739).
Deliberately NOT added to the EXCEPTIONS allowlist: granting an exception to a
file I had just bloated is exactly the quiet rule-erosion the doctrine warns
about, and that list already carries a "candidates for splitting" backlog.

Pure code motion — no behaviour change.

Verified: check-file-sizes exit 0 (44 -> 43 warnings, zero errors); cargo fmt
clean; clippy -D warnings clean on BOTH CI legs (default + experimental);
full lib suite 4300 passed / 0 failed / 10 ignored.

Follow-up worth considering (not done here): move the file-size gate out of the
Frontend job so it runs on every PR. It is a repo-wide invariant, not a
frontend one.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01RmAB6P1r22WBwdM6eUyGgk
… or reopen)

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@runyourempire

Copy link
Copy Markdown
Collaborator Author

⚠️ CI status: pull_request events are not firing for this PR

GitHub has not delivered a single pull_request workflow event for #432. Attempted three times:

  1. PR opened → no run
  2. PR closed + reopened → no run
  3. Push (empty commit 466b1318, fires synchronize) → no run

gh run list --branch worktree-rerank-file-size-split returned empty after each. Actions itself is healthy — concurrent Dependabot PRs were running Validate and Hermetic Fresh-Clone throughout, and repos/.../actions/permissions reports enabled=true, allowed_actions=all. PR head SHA matches the branch head, base is main.

A workflow_dispatch run does start (run 31812772857) but is vacuously green — with no PR context the Detect changes job skips, cascading to skip Frontend, MCP Server and Rust. It verified nothing and should not be read as a pass:

skipped  Detect changes
skipped  Rust (${{ matrix.label }})
skipped  MCP Server
skipped  Frontend
success  Validate Success     <- vacuous

Local verification (complete, and green)

Because CI could not run, here is the full local evidence:

Check Result
node scripts/check-file-sizes.cjs exit 0 — 44 → 43 warnings, zero errors (the bug this PR fixes)
cargo fmt --check clean
cargo clippy -- -D warnings clean (CI leg 1: default features)
cargo clippy --features experimental -- -D warnings clean (CI leg 2)
cargo test --lib 4,300 passed / 0 failed / 10 ignored
full local pre-commit gate green — doc locations, LLM-gate honesty, vanity metrics, release channel, no-window spawns, dead-code expiry, ghost commands, IPC contracts, boundary calls, ESLint, secret scan

The change is pure code motion (one module extracted, zero behaviour change), so the local suite is a strong signal — but it is not a substitute for CI, and I am not claiming it is.

Needs a human call: either investigate the missing pull_request event delivery, or merge on the local evidence above.

@runyourempire

Copy link
Copy Markdown
Collaborator Author

Superseded by #430, which landed first and solved the same 1000-line gate failure with a different seam — extracting the #[cfg(test)] mod rerank_breaker_tests block to analysis_rerank_tests.rs via the repo's established #[path] pattern (1032 -> 866 lines, confirmed on main at analysis_rerank.rs:865).

This PR's seam (analysis_rerank_outcome.rs) is a reasonable alternative but now conflicts, and the gate is already green on main. Closing to keep the queue clean — no reflection on the work, it was independent confirmation of both the diagnosis and the fix.

@runyourempire

Copy link
Copy Markdown
Collaborator Author

Superseded — closing

While this PR sat unverifiable, #430 landed the same fix from another lane: "URGENT unblock: split analysis_rerank tests to clear the 1000-line commit gate (fleet cannot commit)" (02c105d9).

Verified against current origin/main (a8365d74) on real files, not a piped read:

src-tauri/src/analysis_rerank.rs        866 lines
src-tauri/src/analysis_rerank_tests.rs  168 lines   (_tests.rs — exempt from the gate)

node scripts/check-file-sizes.cjs  ->  exit 0, 42 warnings, ZERO errors

#430 moved the tests out; this PR moved the outcome types + budget pacing out (866 vs 760). Both clear the threshold. Theirs is merged, so this is duplicate work and re-litigating the split would be pure churn. Closing.

The part that matters more than this PR

My #423 blocked the entire fleet from committing. check-file-sizes.cjs runs in .husky/pre-commit, so once analysis_rerank.rs crossed 1000 lines, every terminal in the fleet was blocked on every commit — not just PRs touching that file. That is why #430 was filed as URGENT.

Root cause of how it reached main is unchanged and still open:

The size gate runs as a step inside CI's Frontend job, and Detect changes skips that job on Rust-only PRs. A Rust-only change can therefore violate a Rust file-size rule and still go green — and the cost lands on whoever commits next, not on the change that caused it.

Recommended follow-up (not filed by me): move Check file sizes out of the Frontend job into one that always runs. It enforces a repo-wide invariant, so gating it behind a frontend path filter will keep producing this exact failure.

Also worth recording: why this PR never got CI

No pull_request workflow event ever fired here — not on open, reopen, or push. The cause turned out to be mundane: the PR was in state=CLOSED (a gh pr reopen reported success but did not persist) and had gone CONFLICTING as main advanced. GitHub does not run pull_request workflows on a closed PR. A workflow_dispatch run did start but was vacuously green — with no PR context Detect changes skips, cascading to skip every real job.

@runyourempire
runyourempire deleted the worktree-rerank-file-size-split branch August 14, 2026 21:57
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