Uh oh!
There was an error while loading. Please reload this page.
fix(canary): distinguish a goldens harness error from real drift - #325
Conversation
LukasWodka
commented
Jul 14, 2026
@BugBot run |
LukasWodka
commented
Jul 14, 2026
👋 Heads-up — Code review queue is at 47 / 30 Above the WIP limit. The team convention is to review existing PRs before opening new work. Open PRs currently in Code review (oldest first):
Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.) |
Uh oh!
There was an error while loading. Please reload this page.
LukasWodka
commented
Jul 14, 2026
@BugBot run |
95fec50 to
16466b1CompareUh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
The HEAD-drift canary set drift=true on ANY non-zero exit from sync-validator-goldens.sh --check. But that exit also covers harness failures -- missing deps, a bad DATA_INGESTORS_DIR, a generator crash -- so the tracking issue reported "DRIFT -- verdicts/values changed" even when parity was never evaluated, sending someone down the pin-bump playbook for an infra bug. (Cursor Bugbot, Medium, surfaced on the #323 promotion.) sync-validator-goldens.sh --check now uses reserved exit codes so consumers can tell the two apart: 0 in sync 2 HARNESS ERROR -- could not regenerate; parity NOT evaluated (not drift) 3 DRIFT -- verdicts / read-path values changed The required goldens-drift check is unaffected (it reds on any non-zero). The canary adapts the WHOLE tracking issue to the signal via a pin_actionable flag (real drift OR source diff OR unreachable pin): - pin_actionable -> drift framing: the pin-bump Playbook, the "HEAD has drifted" title, and the "close when the pin bump lands" footer. - harness-only (evaluated=false, no source diff, pin reachable) -> harness framing: NO Playbook, a "could not run (harness error)" title, and a "fix the harness, do NOT bump the pin" footer. The harness-error note is purely factual (no bump/don't-bump wording) so it never contradicts the Playbook when a harness error co-occurs with a real source diff. The title is recomputed from the current signal and refreshed on EVERY run (issue edit, not only create) so a persistent issue's title never lags the latest failure mode. "All clear" requires evaluated=true so it cannot mask a harness failure. Verified: --check with no data-ingestors exits 2 with a HARNESS ERROR message and does not mutate goldens.json; the reporting script, exercised across harness-only, harness+source-diff, real-drift, and existing-vs-new-issue scenarios, produces a self-consistent row / note / playbook / footer / title each time; actionlint + shellcheck clean; YAML valid. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Jul 14, 2026
@BugBot run |
16466b1 to
69ed892CompareThere was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 69ed892. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
What
Fixes the Cursor Bugbot "Canary treats script failures as drift" (Medium) finding, surfaced on the #323 develop→main promotion.
head-drift-canary.ymlsetdrift=trueon any non-zero exit fromscripts/sync-validator-goldens.sh --check. But that exit also fires on harness failures — missing deps, a badDATA_INGESTORS_DIR, a generator crash — so the tracking issue reported "DRIFT — verdicts/values changed" even when parity was never evaluated, sending someone down the pin-bump playbook to chase an infra bug.Fix
sync-validator-goldens.sh --checknow uses reserved exit codes so consumers can distinguish the cases:02DATA_INGESTORS_DIR/ generator crash); parity not evaluated3The required
goldens-driftcheck is unaffected — it reds on any non-zero, and both2and3are non-zero.The canary maps them:
drift=true→ advisory tracking issue (run stays green, as before).evaluated=false→ reported as "could not evaluate — harness error" with a note to fix the harness rather than bump the pin.evaluated=true, so a harness error can't be silently reported as all-clear.Verification
--checkwith no data-ingestors → exits 2, printsHARNESS ERROR, and does not mutategoldens.jsonactionlintclean,shellcheckclean, YAML validOnce merged to develop, #323 inherits it and Bugbot re-clears.
Note
Low Risk
Advisory CI reporting and shell script exit semantics only; blocking goldens-drift still fails on any non-zero exit.
Overview
The HEAD-drift canary no longer treats every non-zero exit from
sync-validator-goldens.sh --checkas goldens drift.--checknow documents and uses reserved exit codes:0in sync,2harness failure (generator/deps/DATA_INGESTORS_DIR— parity never ran),3real verdict/value drift. Generator failures exit2after restoringgoldens.jsoninstead of being lumped with drift.head-drift-canary.ymlmaps those codes: exit3→drift=true; exit2or other failures →evaluated=falsewith messaging that parity was not checked. Tracking issues distinguish harness vs drift in the table and body, usepin_actionableso harness-only failures do not show the pin-bump playbook, refresh issue title on update, and requireevaluated=truefor the "All clear" step.The blocking
goldens-driftjob still fails on any non-zero exit (2or3); behavior there is unchanged.Reviewed by Cursor Bugbot for commit 69ed892. Bugbot is set up for automated code reviews on this repo. Configure here.