Run vendored-sync on every PR instead of almost never (#184) - #196
Conversation
check_vendored_sync.sh compares six vendored files against the canonical hub. Exactly one of them was listed in the paths: filter of the workflow the job lived in, so a PR touching chem_formula.py, any tests/test_id_label_*.py, or src/traitmech/schema/mech_shared.yaml never fired the guard. Neither the checker nor the pinned ref was listed either: #182 rewrote check_vendored_sync.sh and changed .vendored_canon_ref without this job running once. The workflow has not run at all since 2026-07-22; #182, #185 and #190 all merged without it. The cause is not four missing path entries. A 4s bash+curl job was sharing a paths: filter with an 88s job that needs uv and a cached OAK download. The filter is right for the expensive job and was inherited by the cheap one. Deriving the filter from the checker's own FILES array is not possible — GitHub evaluates paths: from static YAML before checkout, so it cannot read the repo. Any hand-maintained list has to be kept in sync with the checker by hand, which is the same drift this guard exists to catch. So move the job to its own workflow with no filter rather than re-syncing two lists: at 4s there is nothing to save by filtering. label-correspondence keeps its filter. Adds a 3-attempt retry, since the job now makes 6 raw.githubusercontent fetches on every PR and a hub blip should not fail unrelated work. Exit 2 (missing/empty pinned ref) short-circuits — it is a local precondition failure that retrying cannot fix. The retry lives in the workflow, not in check_vendored_sync.sh: that script is byte-identical to CommunityMech's copy and absent from CultureMech, so it has no canonical copy to diff against (CommunityMech#278) and editing it here would create cross-repo drift that nothing detects. Verified the retry against stubs — success exits 0 immediately, exit 2 short-circuits with no retry, persistent failure exits 1 after 2 retries, and a transient failure recovers on attempt 2. Both workflows parse, vendored-sync resolves to no paths filter, the real checker passes (6/6 at 6be694f3), qc and 152 tests green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes an enforcement gap where the vendored-sync drift check was effectively skipped for most of the files it’s meant to protect due to an inherited paths: filter. It does so by moving vendored-sync into its own workflow that runs on every PR/push, and adds retry logic in the workflow to reduce flakes from transient hub fetch failures.
Changes:
- Adds a dedicated
.github/workflows/vendored-sync.yamlworkflow with nopaths:filter so the drift guard runs reliably. - Removes the
vendored-syncjob from.github/workflows/label-correspondence.yamland updates the workflow header comment to reflect the new split. - Implements 3-attempt retry (with short-circuit for exit code 2) at the workflow level rather than modifying the shared drift-check script.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
.github/workflows/vendored-sync.yaml |
New always-on workflow for vendored drift checking with retry logic. |
.github/workflows/label-correspondence.yaml |
Removes the drift-check job and keeps the paths: filter for the slower OAK-backed label correspondence job. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…NEXT_TASKS Three findings from the review of #196. Closes #199 — cancel-in-progress was unconditional, so two merges landing close together on main would cancel the earlier commit's run and leave it with no verdict. Scoped to pull_request, where superseding a branch run is genuinely waste. The "~4s" figure in both workflow comments came from the old job's timing before the retry wrapper; the job measures 7s in CI. Corrected, along with the 88s figure for label-correspondence, which now runs ~2min. NEXT_TASKS reconciled: #184 is fixed rather than open, the header still listed it alongside a stale "no open PRs" and a through-#182 watermark, and the section 2 write-up now records why #184's own suggested fix was not implementable and that the gap is fleet-wide (#198), not TraitMech-specific. Filed from the same review and deliberately not fixed here: #197 (running on every PR couples all PRs to CultureMech's availability — a real trade-off that wants a decision, not a reflex) and #198 (the sibling repos have the same four-file hole; one cross-Mech sweep, not three PRs). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ReviewAdversarial pass over my own diff. Four findings; three fixed in Checked and safeNo required-status-check breakage. Moving a job between workflows renames its check if the job name changes; it did not ( The guard demonstrably runs now — Fixed here
Filed, not fixed
Note on the retryWorth flagging for a reader: State
|
Two of the file's numbers did not reproduce, and a whole work thread was missing. - Header: no open PRs; merged through #210. Replaced the prose issue list with a table of all 11 open issues; recorded #184/#199/#200/#202/#204 as closed. - Section 4: the "predicates 85% / nodes 62%" figures do not reproduce under any metric. The repo's own dry-run scripts give 63% and 35%; point at them as ground truth instead of restating a number here. The "quality floor" claim was also too generous — `positively regulates` (37 edges) and `negatively regulates` (16) are exact RO labels sitting ungrounded while their paraphrases `promotes`/`inhibits` are already mapped. - Section 5: "1 of 220 traits done" was wrong arithmetic. 220 is what is still fragmented, re-measured today, across 220 of 477 files. - Section 7 (new): the CI/agent-workflow thread — #194, #196, #201, #206, #207, #210 — plus the seven small review issues it left open. - Section 8 (new): the paid Edison sweep's manifest says 353 ok, but research/ is gitignored and only 11 reports survive here. Resume detection is file-existence based, so a re-run would re-bill 342 completed calls. - Section 9 (new): the grounding backfill that follows from the section 4 correction, gated by the existing blocking label-correspondence check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two of the file's numbers did not reproduce, and a whole work thread was missing. - Header: no open PRs; merged through #210. Replaced the prose issue list with a table of all 11 open issues; recorded #184/#199/#200/#202/#204 as closed. - Section 4: the "predicates 85% / nodes 62%" figures do not reproduce under any metric. The repo's own dry-run scripts give 63% and 35%; point at them as ground truth instead of restating a number here. The "quality floor" claim was also too generous — `positively regulates` (37 edges) and `negatively regulates` (16) are exact RO labels sitting ungrounded while their paraphrases `promotes`/`inhibits` are already mapped. - Section 5: "1 of 220 traits done" was wrong arithmetic. 220 is what is still fragmented, re-measured today, across 220 of 477 files. - Section 7 (new): the CI/agent-workflow thread — #194, #196, #201, #206, #207, #210 — plus the seven small review issues it left open. - Section 8 (new): the paid Edison sweep's manifest says 353 ok, but research/ is gitignored and only 11 reports survive here. Resume detection is file-existence based, so a re-run would re-bill 342 completed calls. - Section 9 (new): the grounding backfill that follows from the section 4 correction, gated by the existing blocking label-correspondence check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Caught by actually checking the sibling repos instead of carrying the paragraph forward — which is the failure mode this file's own skill warns about, and I had reproduced it. MediaIngredientMech#160 and CommunityMech#280 are both closed, their PRs (#166, #302) merged, and both repos now carry the unfiltered vendored-sync.yaml that #196 pioneered here, complete with the #199 cancel-in-progress fix. CommunityMech#278 is closed too. So "every repo readable on 2026-08-01 has a four-file hole" is out of date. What may remain is narrower and lives in a different workflow: CultureMech is the hub and has no vendored-sync job at all, but its label-correspondence.yaml still has a paths filter globbing only src/culturemech/schema/**, and whether tests.yaml covers chem_formula.py and the three id_label tests by another route is unverified. #198 stays open scoped to that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The backlog had drifted a day and carried numbers that did not reproduce. Docs-only; the PR touches `NEXT_TASKS.md` and nothing else. Corrected: - Section 4's grounding coverage was recorded as "predicates 85% / nodes 62%". The repo's own scripts give 63% and 35%; the hardcoded percentages are replaced by the commands that produce them. - Section 5 presented four different measurements — 220 fragmented graphs, 219 baseline files, 1314 findings, 1264 stranded nodes — as though they were one. Now a table with a source per row. - Section 8 read "353 ok, 8 fail" as 8 unfinished traits. Every `fail:1` row is a retry that later succeeded; `fail − ok` is empty, so zero traits are outstanding. - Section 2 called the cross-Mech vendored-sync sweep pending. It landed: MediaIngredientMech#160 and CommunityMech#280/#278 are closed and both spokes carry the unfiltered workflow. Only the CultureMech `label-correspondence` half may remain. Added: section 7 for the CI/agent-workflow thread (#194, #196, #201, #206, #207, #210, #216), section 8 for the paid Edison sweep whose 353 reports are gitignored and absent from a fresh clone, section 9 for the grounding backfill. Filed along the way: #214 (residual reports drift with nothing to catch it), #217 (no workflow-conventions page), #218 (enforce the concurrency rule in pr-sanity), #220 (`audit-graphs` cannot see a graph splitting into two trait-bearing components — `morphology/dumbbell_shaped.yaml` is the live instance, and it is why 220 and 219 disagree). #215 was found and fixed the same way, in #216. The open-issue table was cross-checked against `gh issue list` before merge: 15 rows, exact match. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
#215 was the third bug of this shape after #199 and #196's review, and it caused a silent repo-wide outage: no PR got a Claude review for a day, because the workflow's own progress comment fired issue_comment into the group its pull_request run was holding. #217 proposes writing the lesson down; this enforces it, which is strictly stronger. pr_sanity.check_workflows already parses every workflow, so the guard sits beside NO_UNFILTERED_CI: if a workflow triggers on pull_request AND on something that resolves to the same PR without a push (issue_comment, pull_request_review, pull_request_review_comment, pull_request_target), any concurrency block that can cancel must discriminate — by group key, or by a cancel-in-progress expression that genuinely separates the events. The scoping is most of the work, since a lint that cries wolf gets disabled: - `push` and `schedule` are exempt. Their github.ref is a branch, never refs/pull/N/merge, and they carry no PR context. curation-history.yaml is exactly that shape and a naive rule fails it on day one. - Cancel expressions are judged on POLARITY, not on mentioning github.event_name. `== 'pull_request'` confines cancellation to push runs and is a fix; `== 'issue_comment'` confines it to comment runs and is #215 stated as a condition. `!= 'push'` separates nothing relevant. Every colliding trigger must be excluded, not just one. - The list and bare-string `on:` shorthands are normalised, having initially disabled the check entirely — a lint that quietly evaluates nothing being the exact failure this script exists to prevent. The same blind spot in NO_UNFILTERED_CI is fixed alongside. It is a heuristic over a template string. Verified where that matters: it fires on the pre-#216 claude-code-review.yml in both dict and list forms, and reports zero across all nine current workflows. 43 tests, both directions for every shape, including one that runs the check over this repo's real workflows so it cannot rot into a fixture-only guard. Closes #218. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fixes #184.
The problem, verified
scripts/check_vendored_sync.shcompares six files against the canonical hub. Exactly one was in thepaths:filter of the workflow the job lived in:trigger_paths?scripts/validate_id_label_correspondence.pyscripts/chem_formula.pytests/test_id_label_empty_adapter.pytests/test_id_label_unknown_prefix.pytests/test_id_label_plausibility.pysrc/traitmech/schema/mech_shared.yamlNeither
scripts/check_vendored_sync.shnorscripts/.vendored_canon_refwas listed either, so editing the checker or silently reverting the pin did not fire the checker.This is not hypothetical. The workflow has not run since 2026-07-22; #182, #185 and #190 all merged without it. Most pointedly, #182 rewrote
check_vendored_sync.shand changed.vendored_canon_refand this job never ran on it — the PR that rebuilt the drift guard was not checked by the drift guard.Why the fix is "delete the filter", not "extend it"
Measured from the last real run, the two jobs sharing that filter are not comparable:
vendored-synclabel-correspondenceThe filter is correct for the 88s job and was inherited by the 4s one.
#184 suggested deriving the filter from the checker's own
FILESarray so the two cannot drift. That is not implementable — GitHub evaluatespaths:from static YAML before checkout, so it cannot read the repo. Any hand-maintained list must be kept in sync with the checker by hand, which is precisely the drift this guard exists to prevent.So
vendored-syncmoves to its own workflow with nopaths:filter. At 4s there is nothing to save by filtering, and there is no longer a list that can go out of sync.label-correspondencekeeps its filter, which was always right for it.Retry
The job now makes 6
raw.githubusercontentfetches on every PR, so a hub blip would fail unrelated work. Added 3 attempts with a 5s gap. Exit 2 (missing/empty pinned ref) short-circuits — a local precondition failure that retrying cannot fix.The retry is in the workflow, not in
check_vendored_sync.sh. That script is byte-identical to CommunityMech's copy and absent from CultureMech — it has no canonical copy in the hub to diff against (CommunityMech#278), so editing it here would create cross-repo drift that nothing currently detects.Verified
Retry logic exercised against stubs, because
if cmd; then …; fireturns 0 when the condition fails — a first draft usedstatus=$?after theifand the exit-2 short-circuit silently never fired:Also: both workflows parse;
vendored-syncresolves to no paths filter andlabel-correspondenceretains its four; the real checker passes 6/6 againstCultureBotAI/CultureMech@6be694f3;just qcand 152 tests green.Scope note
#184 says TraitMech is "the worst of the three spokes", which is true — but I checked, and CultureMech and CommunityMech also omit
chem_formula.pyand all threetests/test_id_label_*.py; theirsrc/<pkg>/schema/**glob only coversmech_shared.yaml. So every repo I could read has a four-file hole and TraitMech has a fifth. MIM's workflow returns 404 viagh api, so its filter is unverified. Follow-up filed for the sibling sweep.🤖 Generated with Claude Code