Repository navigation
fix: route layerdir through ENVIRON in bytesof(), not -v (#475) - #483
Merged
Merged
Conversation
The ambiguous-vocabulary-keyword collision report in rebuild-tsv.sh built layerdir in bash and passed it to awk via `-v layerdir=...`. awk's -v decodes backslash escapes in the value it receives -- the same mistake #424 fixed for JIT_BASE/tools_base/vocab_base and #437 fixed for GENERIC_WORDS_FILE/GENERIC_WORDS_FILES, left untouched at this one other site. On a backslash-bearing CLAUDE_PROJECT_DIR, bytesof()'s getline against the mangled path silently failed to open, so every entry in that layer reported "unmeasured" instead of its real byte count. layerdir now travels through ENVIRON instead, matching the route GENERIC_WORDS_FILES already uses in this same file. Co-Authored-By: Claude Sonnet 5 <[email protected]>
#475) Item 1 (layerdir/bytesof()) is the bug this branch's previous commit fixed. Item 2 (GENERIC_WORDS_FILE/wf in the generic-word classifier's BEGIN block) was already fixed before this branch started -- it reads ENVIRON["GENERIC_WORDS_FILES"] at HEAD, not -v wf=.... Caught by self-review (Explore spawn) of this branch's own diff: the fragment still described item 1 as open after the fix landed. Co-Authored-By: Claude Sonnet 5 <[email protected]>
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
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.
Closes #475
What was broken
scripts/rebuild-tsv.sh's ambiguous-vocabulary-keyword collision report built
layerdir(derived from CLAUDE_PROJECT_DIR/VOCAB_BASE) and passed it to awk via-v layerdir="$layerdir". awk's-vdecodes backslash escapes in the value it receives, so on a backslash-bearing CLAUDE_PROJECT_DIR (a Windows-shaped checkout path) the path arrived mangled inside the awk program, everybytesof()getline against it failed to open, and every entry in that vocabulary layer was reported "unmeasured" instead of its real byte count. Same defect class #424 fixed for JIT_BASE/tools_base/vocab_base and #437 fixed for GENERIC_WORDS_FILE/GENERIC_WORDS_FILES, in this same file -- #475 was filed because this one site was missed by both earlier fixes.The fix
layerdirnow travels into the awk program viaENVIRON["layerdir"](exported as a plain bash variable in front of theawkinvocation) instead of-v layerdir=..., matching the existingGENERIC_WORDS_FILES/ENVIRON pattern already used a few hundred lines below in the same file. Thelayervariable (a cosmetic label built byjit_report_name(), which only ever returns[A-Za-z0-9._-]or a withheld placeholder) was deliberately left on-v-- it can never carry a backslash and is not part of this defect class.Test-first evidence
New case "475" added to tests/test-entry-bytes.sh, inside the existing per-awk-engine loop (awk/gawk/mawk). Builds a project root under a directory containing a literal backslash, writes two vocabulary entries sharing a keyword, forces the collision report, and asserts the collision still fires (positive control) while the stderr report does NOT contain "could not be measured" (negative control -- the literal text emitted on the bug).
RED: ran by hand against the pre-fix code (single git stash of scripts/rebuild-tsv.sh, popped immediately after) -- case 475 FAILED on awk and gawk, PASSED on mawk (expected engine variance; mawk's -v does not decode escapes the same way, matching this file's own documented #77/#78 variance shape).
GREEN: after restoring the fix, all three engines pass; full file reports PASS: 345 FAIL: 0.
Also ran:
python3 .oss/assemble_changelog.py --check(ok, 3 fragments), tests/test-line-citations.sh (exit 0),shellcheck -x scripts/rebuild-tsv.sh(only a pre-existing, unrelated SC1091 info note, present regardless of this diff). Did not runbash tests/run-all.sh-- CI is the authority across its 3-platform matrix; ran the targeted suite plus two cross-cutting guard scripts (lane_setup.py), both of which reported not-in-this-repo/uncovered for this bash project.Follow-up in this branch
trap.d/424.rebuild-tsv-v-escape.md was found stale during self-review -- still describing both its items (this one, and the already-fixed GENERIC_WORDS_FILE/wf site) as open. Removed in a second commit, since both are now resolved.
Adjacent, not fixed
A second
-v-passed value exists at scripts/rebuild-tsv.sh's module-prefix-mapping report:-v file="$filename", used only in aprintfinto a different TSV, never opened viagetline. Below the bar, not fixed:$filenameis abasename()result, and a literal backslash cannot appear in a filename component on a real Windows filesystem (backslash is the Windows path separator, forbidden inside a filename), unlikelayerdir, which is a full directory path where backslash legitimately appears as a separator -- so the defect class this issue fixes is not reachable at that site.docs_targets
README.md checked for
layerdir/bytesof/"Ambiguous vocabulary" -- no mention. Internal maintainer-tool reporting fix with no user-facing behavior change; no-change-needed.[AI-generated]