chore: harden crucible_check.py against env drift, add scope_check.py - #2620
Merged
Conversation
…icate census (#2547) Languages sliced by Mode D (_slice_by_keywords: shell/ruby/lua/elixir/livecode/ matlab) or Mode E (_slice_by_terminator: sql/sqlite/plpgsql/...) have no real same-file call graph. Mode D bundles any top-level loose code before the first real scope into a synthetic "__global_context__" satellite; Mode E names every top-level statement generically from its keyword ("SELECT_Statement", "CREATE_Statement", ...) rather than a real captured identifier. Neither is a genuine callable function, so the intra-file orphan census's "does this name appear anywhere else in the file" check was structurally guaranteed to flag them -- confirmed against the #1096 control corpus: shell a.sh/b.sh reported 4 orphans for 3 real functions (the extra was __global_context__), and sqlite main.sql reported orphaned_logic=7 for a file with zero real callable names at all (every satellite was a synthetic Mode-E bucket). The existing exclusion set (Unknown_Sat/Anonymous_Block/Main/Declarative_Block) already recognized this class of problem but was incomplete: it missed __global_context__ entirely, didn't strip the _[Truncated]/_[Unterminated] suffixes the slicer appends to those same names, and never covered Mode E's <KEYWORD>_Statement shape. It also only guarded the orphan branch, not the duplicate-detection branch reachable by the same synthetic names. Replace it with _is_synthetic_satellite_name(), which recognizes the full family (suffix-stripped exact names + the Statement-name pattern) and gates both branches. This directly fixes the orphan->api conversion (Contextual Baseline Fix, galaxyscope.py ~2145) inflating a popular file's api count with non-function shapes. Re-verified against the real corpus post-fix: a.sh/b.sh api 4->3, main.sh orphans 2->1, main.sql orphans 7->0, a/b/c.sql's bogus api 2/4/3->0/0/0 (SQL statements were never real API surface -- Mode E never captures a real identifier for any SQL statement type, so the correct count is zero, not one per CREATE INDEX as initially assumed). Golden master fixtures regenerated (tests/tools/update_golden_master.py, both full-precision and zero-dependency modes) -- the ~2200 diffs are the expected, intentional consequence of this fix across every shell/sql/lua/livecode/ruby file in the language-crucible corpus with either shape. crucible_check.py and the full test suite (7168 tests) pass clean after regeneration. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…, fix ci-push-checklist Motivated by PR #2518 (jcl args continuation-line fix): verifying that fix was correctly scoped took far longer than it should have, chasing environment issues that had nothing to do with the actual code change. This bundles what that investigation found into reusable tooling so the next fix doesn't re-pay the same cost. crucible_check.py: - Prefers a `uv`-managed Python matching CI's own pin (parsed from .github/workflows/golden-crucible.yml) when building a venv, instead of whatever interpreter happens to be running the script -- a version mismatch can resolve different releases of unpinned optional deps (networkx/pandas/etc.), producing numeric drift unrelated to the change under test. - Warns if the local language-crucible corpus isn't on the pinned tag, and if its own absolute path contains an IGNORED_DIRECTORIES name (e.g. "tmp") as any path component -- both produce thousands of phantom diff lines with no error message otherwise. - Verifies (and retries) that a venv's editable install actually resolves to the intended repo_root before returning it, via a new _venv_env() that strips PYTHONPATH from every subprocess invoking a specific venv's python. An inherited PYTHONPATH pointing at a real gitgalaxy/ package (trivially easy to have set for unrelated reasons) silently shadows the venv's own editable-install finder with no error at all -- confirmed to fully explain a chain of false leads (a wheel-cache-collision theory, a setuptools_scm worktree-root theory, a python -c cwd-shadowing theory) while building this fix, before the actual cause turned up. See the module's own docstring for the full writeup. - ensure_venv() now accepts an explicit repo_root, so a second venv against a different worktree can reuse the exact same build logic. scope_check.py (new): isolates a change's real-corpus impact to the language(s) it should touch, independent of whether the committed golden master fixture is even current. Scans the working tree and a comparison ref (default origin/main) fresh in separate venvs, diffs them directly, and buckets every difference by language -- `--expect <lang>` fails loudly if anything outside that set changed. This is the exact by-hand methodology used to verify PR #2518's fix was jcl-only, now a single command instead of ~15 ad hoc one-liners. ci-push-checklist skill: added time estimates per section, a step -1 pointing at the self-scan DB/architecture brief for orientation, and corrected section 3's corpus-cloning guidance -- the prior "never clone a fresh copy, metadata/footprint shifts topology" claim was wrong about the mechanism (verified by direct repro); the two real causes are corpus pin drift and unsafe path naming, both now caught automatically. Also documents scope_check.py and points section 6's post-conflict-resolution step at it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
squid-protocol
added a commit
that referenced
this pull request
Aug 31, 2026
… + ci-push-checklist (#2621) Two friction points from fixing #2547 that cost real time/tokens, distinct from #2620's crucible_check.py env-drift hardening (a different issue, #2518): 1. No documented technique for empirically inspecting what detector.py actually extracted for a given file -- root-causing the #2547 bug required reading _slice_by_keywords/ _slice_by_terminator cold before realizing `galaxyscope --debug` + grepping the WORKER-TRACE log line is far cheaper. Added as its own CLAUDE.md section, cross-linked from the ci-push-checklist skill's "Local & Unit Validation" step. 2. CLAUDE.md's Differential Scan section told a reader to regenerate golden masters via `python tests/tools/update_golden_master.py` directly -- technically correct (it's the sanctioned mechanism) but misleading: it silently updates only whichever ONE fixture matches whatever's importable in the current shell, with none of crucible_check.py's venv/PATH automation. The better path (`crucible_check.py --update --yes`, which handles both modes through properly-scoped venvs) was already documented 10 lines below in the same section, just never cross-referenced -- reading the first instruction and stopping there (as I did) cost a full manual venv-wrangling cycle. Reordered so the automated entry point is what a reader hits first, with update_golden_master.py explained as the underlying single-mode script it wraps. Also noted (ci-push-checklist): Claude Code's Auto Mode classifier blocks blessing a golden master by default, even via --yes on a no-op clean tree -- worth expecting up front rather than hitting it as a surprise mid-task. Docs only, no code/fixture changes. Co-authored-by: Joe Esquibel <squid-protocol@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Efficiency/reliability tooling for the "fix an issue → verify → ship" workflow, motivated directly by how much longer PR #2518 (jcl args continuation-line fix) took than it should have — most of the extra time was chasing environment issues that had nothing to do with the actual code change. This bundles what that investigation found into reusable tooling.
crucible_check.pyuv-managed Python matching CI's own pin (parsed from the workflow file itself) when building a venv, instead of whatever interpreter happens to run the script.language-cruciblecorpus isn't on the pinned tag, or if its own absolute path contains anIGNORED_DIRECTORIESname (e.g.tmp) anywhere in its ancestry — both silently produce thousands of phantom diff lines.repo_root, via a new_venv_env()that stripsPYTHONPATHfrom every subprocess invoking a specific venv's python. This turned out to be the real bug behind a chain of false leads while buildingscope_check.py— an inheritedPYTHONPATHpointing at a realgitgalaxy/package silently shadowed a venv's own editable-install finder with no error at all. See the module's own docstring for the honest, warts-and-all account (I chased a wheel-cache theory and a setuptools_scm worktree-root theory first, both wrong).ensure_venv()now takes an explicitrepo_root, so a second venv against a different worktree can reuse the exact same build/verify logic.scope_check.py(new)Isolates a change's real-corpus impact to the language(s) it should touch, independent of whether the committed golden-master fixture is even current (useful mid-investigation, or once
mainhas moved past your branch point). Scans the working tree and a comparison ref (defaultorigin/main) fresh in two separate venvs, diffs them directly, and buckets every difference by language:This is the by-hand methodology from PR #2518's verification (~15 ad hoc
python3 -cone-liners), now one command.ci-push-checklistskilldocs/gitgalaxy_architecture_brief.md/ the self-scan DB for orientation before reading code cold (ties back to CLAUDE.md's existing guidance).crucible_check.py, and the section explains the actual mechanism instead of the folklore.scope_check.pyand pointed section 6's post-conflict-resolution step at it, since a clean regen run alone doesn't catch a bad conflict resolution.Verification
ruff_audit.py --ci/mypy_audit.py --ci: clean.crucible_check.py --mode zero: PASS (this branch has zero engine changes, as expected).scope_check.py --mode zero(default, vsorigin/main):Total differences vs origin/main: 0— clean, no false warnings, no false retries.scope_check.pyexercised against a real historical ref with genuine accumulated changes (~300 commits, ~9k diff lines across nearly every language): correctly bucketed everything by language and correctly failed--expect jclsince far more than jcl had actually changed — confirming the tool reports real signal, not noise, either direction.🤖 Generated with Claude Code