feat: split apply health lane reporting#397
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 9:56 PM ET / 01:56 UTC. Summary Reproducibility: not applicable. as a feature PR. Source review confirms current main lacks lane-level apply-health output while PR head adds and tests it. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the lane split after normal maintainer review and required checks, preserving the existing top-level fields and focused tests for close-mode durable-comment side effects. Do we have a high-confidence way to reproduce the issue? Not applicable as a feature PR. Source review confirms current main lacks lane-level apply-health output while PR head adds and tests it. Is this the best way to solve the issue? Yes. The PR is a narrow follow-up to the merged apply-health surface, keeps existing top-level fields for compatibility, and now reuses the durable review-comment sync classifier for lane counts. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 279d3bd60f3b. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
cad6163 to
59aae05
Compare
|
Maintainer review complete on exact head This cleanly separates closure and durable-comment-sync health without changing the existing aggregate status contract. The final fix also preserves comment refresh counts performed during closure processing and exposes that split in the dashboard tooltip. Proof:
No known proof gaps for this reporting-only change. |
What Problem This Solves
Apply runs can close records and refresh durable review comments in the same applicator surface. Before this change, the status summary and dashboard counters made it harder to tell whether a run was doing closure work, comment-sync work, or both. A run that only refreshed comments could look like a close apply run with no pruning progress.
Why This Change Was Made
lanes.closureandlanes.comment_syncwhile preserving the existing top-level counts for compatibility.closedandcomments syncedpills, with clearer hover text showing lane-specific processed counts.User Impact
Maintainers can now distinguish actual pruning throughput from durable comment maintenance. A zero-close run that is only doing comment sync should no longer look like failed pruning, and comment-write failures are attributed to the comment-sync lane instead of closure.
Evidence
pnpm run build:allpassed after the accepted review fix.node --test test\repair\workflow-utils.test.ts test\dashboard-worker.test.tspassed: 72 passed, 0 failed.pnpm run format:check -- src/repair/workflow-utils.ts test/repair/workflow-utils.test.ts dashboard/worker.ts test/dashboard-worker.test.ts docs/live-dashboard.mdpassed.PNPM_BIN=$env:LOCALAPPDATA\Corepack\shims\pnpm.CMD; pnpm run checkpassed after the review fix: 1135 passed, 1 skipped, coverage lane 100%, format clean.pnpm run --silent workflow -- summarize-apply-report ...against a close-mode report with one close, one closure skip, one comment sync, and two comment-write skips. Output showedstatus=ok,processed=5,closed=1,comment_synced=1, closure lane{"processed":2,"closed":1,"comment_synced":0,"skipped":1,"skip_reasons":{"skipped_changed_since_review":1}}, and comment-sync lane{"processed":3,"closed":0,"comment_synced":1,"skipped":2,"skip_reasons":{"skipped_comment_auth":1,"skipped_locked_conversation":1}}.codex.cmd -c service_tier='"'"'fast'"'"' review --uncommittedreported no actionable correctness issues.Screenshot Proof
No new dashboard card or always-visible metric is added in this PR. The existing #391 pruning alert remains the visual surface; this follow-up changes the status JSON and the hover text on the existing
closedandcomments syncedpills so the dashboard stays quiet unless #391 already decides the apply health needs attention.Risk Notes
processed,closed,comment_synced, andskip_reasonsfields remain unchanged for downstream consumers.