fix(jcl): recognize PARM= on continuation lines, bound Mode A args search - #2518
Merged
Conversation
…arch (#2482, #2483) The `args` regex only ever saw PARM= when it sat on the EXEC statement's own physical line, missing ~16-18 real corpus occurrences where it lands on a `//` continuation line instead (sometimes two hops away, e.g. CICSTS56.jcl's CICS step). Extended the regex with a bounded (<=8 hops), ReDoS-safe continuation-crossing group. Separately, Mode A's generic args-count derivation searched the whole greedy block for jcl (unlike cobol/fortran/dockerfile, which already got a bounded window via #1973), letting a multi-line PARM='...' string -- or worse, a later unrelated step's own PARM= -- get misattributed to the wrong step (confirmed real: ZOSCSEC.jcl's BPXIT step read args=7 off its own multi-line PARM= string). Added jcl's own comma continuation marker to _MODE_A_ARGS_CONTINUATION_MARKER so it shares the same window-bounding fix. Verified both fixes are correctly scoped via an isolated diff (this branch's fresh galaxyscope scan vs. unmodified main's, same corpus snapshot pinned to language-crucible v1.2.0, full-precision and zero-dependency modes): 269 differences, all confined to jcl/ paths, zero ripple into any other language. Golden master fixtures intentionally left unblessed -- a control run confirmed unmodified main already diverges from the committed fixtures by 1000+ unrelated entries against the correctly pinned v1.2.0 corpus (pre-existing drift, not introduced by this change), so re-blessing locally right now would bake in unrelated noise alongside this fix's legitimate diff. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
…line fix Regenerated both tests/golden_master_audit.json and tests/golden_master_zero_dep_audit.json via update_golden_master.py (Python 3.11 + CI's exact dependency versions, language-crucible pinned to v1.2.0) to reflect the args-recall/over-count fix from the previous commit. 269 differences in each fixture, 100% confined to jcl/ paths -- confirmed matches CI's own crucible-audit diff on PR #2518 exactly. Note for posterity: an earlier local regeneration attempt spuriously showed 1282 differences spanning many unrelated languages (a "Doc Umbrella" field flattening to 0.0 broadly). Root cause: the isolated language-crucible clone used for that attempt lived under a path containing a literal "tmp" directory component, and guidestar_lens.py's _calculate_documentation_coverage skips any directory whose FULL path (any ancestor component, not just the immediate dir name) matches IGNORED_DIRECTORIES -- which includes "tmp". Not a real environment/dependency mismatch, not jcl-related, and not touched by this commit; re-cloning the corpus to a tmp-free path reproduced CI's exact 269-diff result on the first try. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tinuation # Conflicts: # tests/golden_master_audit.json # tests/golden_master_zero_dep_audit.json
This was referenced Aug 31, 2026
squid-protocol
added a commit
that referenced
this pull request
Aug 31, 2026
…#2620) * fix(detector): exclude synthetic slicer bucket names from orphan/duplicate 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> * chore: harden crucible_check.py against env drift, add scope_check.py, 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> --------- Co-authored-by: Joe Esquibel <squid-protocol@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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
Fixes #2482 and #2483 — both are the same underlying JCL
args(PARM=) gap/over-count, addressed together since they share one mechanism.argsregex only matchedPARM=when it sat on theEXECstatement's own physical line. Real corpus JCL routinely continuesPARM=onto a//continuation line — sometimes two hops away (CICSTS56.jcl'sCICSstep:EXECline ends in a comma, then aCOND=(...)continuation line also ends in a comma, and only the third line carriesPARM=). Extended the regex with a bounded (≤8 hops), single-line-per-repetition continuation-crossing group — same-linePARM=(the common case) is untouched.jcl, unlikecobol/fortran/dockerfilewhich already got a bounded search window via Mode A (_slice_by_labels) generic args-count search is unbounded, can attribute unrelated text to a function's parameter count #1973. This let a multi-linePARM='...'string — or worse, a later, unrelated step's ownPARM=— get misattributed to the wrong step. Confirmed real:ZOSCSEC.jcl'sBPXITstep readargs=7off an unbounded sweep of its own multi-linePARM='SH chmod ...'string (documented indocs/language_status/jcl.md). Addedjcl's own comma continuation marker to_MODE_A_ARGS_CONTINUATION_MARKERso it shares the same window-bounding fix as the other three languages.The nested-paren nuance in some real
PARM=(...)lists (e.g.PARM=(...,'AMODE(31)',...)) is a separate, pre-existing limitation (regex can't balance nested parens in one bounded pass) — untouched by this PR, same as before.Verification
tests/extraction/languages/test_jcl_strict.py/test_jcl.py: added regression tests for both issues (continuation-line recall at 1 and 2 hops, multi-linePARM=(...)values, same-line regression, no-bleed-into-later-step, and a dedicated ReDoS sweep for the new bounded hop group), plus a pipeline-level test exercisingStructuralExtractordirectly to prove the window-bound actually prevents cross-step bleed and over-counting (not just the regex in isolation). Removed a now-stalexfailmarker whose "cannot parse JCL line continuations" reason no longer applies.ruff_audit.py --ci/mypy_audit.py --ci: clean, no new findings.language-cruciblesibling checkout was on a stalev1.0-era snapshot rather than the actually-pinnedv1.2.0— cloned an isolatedv1.2.0copy to avoid disturbing another in-progress session's shared corpus checkout): rangalaxyscopedirectly with this branch's own.crucible_venvsagainst unmodifiedmain's, on the identicalv1.2.0corpus, and diffed the two fresh outputs against each other directly (bypassing the committed fixture). Result, in both full-precision and zero-dependency mode: 269 differences, all confined tojcl/paths, zero ripple into any other language — e.g.ZOSCSEC.jcl'sBPXITstep'sInput Parametersdrops from the documented bogus7to the correct1.Note on golden master fixtures (intentionally NOT re-blessed here)
A control run (
main's own fresh scan of the correctly-pinnedv1.2.0corpus vs. the committedtests/golden_master_audit.json) already shows 1000+ differences on unmodifiedmain— pre-existing drift unrelated to this PR (spans unrelated languages/fields, e.g. aDoc Umbrellafield flattening to0.0across many languages, andcumulative_riskranking swaps). I can't rule out this is a local-venv dependency-version artifact (my ad hocpip install -e .+ extra-deps venv vs. CI's exact pinned versions) rather than a realmain-branch regression, and either way it's out of scope for this PR. Re-runningupdate_golden_master.pylocally right now would bake that unrelated drift into this PR's diff alongside the legitimate jcl-only change above, so I've left the fixtures untouched and am relying on CI'scrucible-auditcheck (which runs in the exact pinned environment) as the authoritative signal for whether/what to re-bless.Test plan
test_jcl_strict.py+test_jcl.pypass, including new regression testsruff_audit.py --ci/mypy_audit.py --cicleanmainon the correctly-pinned corpus: change confined tojcl/, both full-precision and zero-dependency modescrucible-audit/tri-comparison-auditCI checks on this PR and address any real (non-pre-existing) drift they flag🤖 Generated with Claude Code