Repository navigation
Conversation
… `files:` is no block header, as it is none to the reader, so `apply` no longer writes the pin's rows under it or refuses a manifest for a `shape:` key written "twice" (#1220) `_is_block_header` cut each line with `str.strip()` and `str.lstrip()` and no argument, which also cut U+00A0 and U+3000. The reader strips `_YAML_WHITE`, a space and a tab, so to it `<U+00A0>shape:` is the key `<U+00A0>shape`, a key of its own, and `parse_yaml` gives the file both. The finder now strips `_YAML_WHITE`, imported from `repo_shape` beside `_split_key` and `_strip_comment`, and its docstring says so. Measured at origin/main with `check` and `apply` over such a file, each run with a local `--upstream`: a manifest whose only `shape:` line is blanked was rewritten under that line and refused `update-validators-red` after the write, where it is now refused `update-manifest-no-shape` by the rewrite's own count; a blanked key above the real header was taken for the header, so a pin was written wrong and rolled back and a manifest was refused for a `shape:` key "twice", and both are now rewritten. A pin whose only `files:` line is blanked is refused `update-pin-no-files` by the reader before the rewrite is reached, and still is. The two other bare `.strip()` calls the issue names are left: see the pull request. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Reviewer's GuideThe PR makes Sequence diagram for whitespace-aware shape header detectionsequenceDiagram
participant Tool as update-shape.py
participant Reader as repo_shape
participant Validator as Validators
Tool->>Reader: line.lstrip(_YAML_WHITE)
Tool->>Reader: _strip_comment(line.strip(_YAML_WHITE))
Tool->>Reader: _split_key(...)
alt key matches shape or files
Tool->>Validator: rewrite using recognized block header
else U+00A0 or U+3000 precedes header
Tool->>Validator: refuse as missing or unrecognized header
end
Flow diagram for reader-consistent header recognitionflowchart TD
A[Input line] --> B[Trim only space and tab with _YAML_WHITE]
B --> C[_strip_comment]
C --> D[_split_key]
D --> E{Matches shape: or files:?}
E -->|Yes| F[Recognize block header]
E -->|No| G[Do not rewrite as a block header]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
(lane openRepoShape-3, 2026-10-23T21:34:45Z) RV1231 independent review of #1231 (#1220) at The fix is right, with no regression from main: MUST
NIT (same round)
Asked rulings
Measured with RV1231's own probe: Tests at the head, one at a time, all green:
Mutants on the 26 new rows, each restored and checked with
Merges: merge-tree with origin/main Sonar and CI:
Body and commit:
Lane: openreposhape-3 (openRepoShape-3) |
…k is one assert, the docstring says the old rewrite lost the pin's `files` key, and a tab in front of `files:` is a row that pins the tab half of the column guard (#1220) The four composite `assert a and b` fixture checks are two asserts each, each keeping its "fixture: ..." message (SonarCloud python:S9073). The docstring said main's result "did not read back". It did: `rewrite_fault` reads only the commit and the tree, so it returned None, and the result lost its `files` key, which validate-pins refused after the write. A `tab-in-front` row (a tab, then `files:`) is the tab half of `_is_block_header`'s column guard. Dropping that half survived every test before; it is killed by this row. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
|
(lane openRepoShape-3, 2026-10-23T21:52:12Z) RV1231b independent delta review of #1231's fix round ( Every MUST and NIT of RV1231 is answered:
Tests at
Probe, RV1231b's own, of
Merges: the open PRs were listed live (51). Only #1166 ( CI on Body and commit:
NIT (body only, no push): "written as an escape in the file, which stays ASCII". The row is ASCII, but the file holds two U+2014 from main at Lane: openreposhape-3 (openRepoShape-3) |



What changed
A v1.2.1 follow-up from lane 3's independent reviews (#1220). It lands under Brett Heap's v1.2.1 merge word (2026-10-23T13:10:26Z) once reviewed and green.
update-shape.py's_is_block_header, which finds the pin'sfiles:and the manifest'sshape:header, cut each line withstr.strip()andstr.lstrip()and no argument. With no argument they also cut U+00A0 and U+3000. The reader's blanks are a space and a tab (_YAML_WHITE, used by_significant_textand_split_key), so to it a line that opens with U+00A0 or U+3000 and then readsshape:is the key<U+00A0>shape, a key of its own. This tool took it for the header.update-shape.py:_YAML_WHITEis imported fromrepo_shape, one more private name beside_split_keyand_strip_comment._is_block_headerstrips withline.strip(_YAML_WHITE)and measures the blanks in front withline.lstrip(_YAML_WHITE). The docstring sentence that said the text "is.strip()ped first, as_tokenisestrips it" was not true of_tokenise; it now says the text is stripped of_YAML_WHITEas_significant_textdoes, and why. The import comment says the same in one sentence. No command, flag, code or message changed.tests/test_update_shape_apply_lines_and_headers.pyandtests/test_update_shape_header_comments.py: the rows the issue names, and the end-to-end runs it asks for (below).Fix round (RV1231)
assert a and bfixture checks intests/test_update_shape_apply_lines_and_headers.py(SonarCloud python:S9073) are two asserts each, each keeping its "fixture: ..." message.test_a_files_key_that_opens_with_a_blank_the_reader_keeps_is_carried_overno longer says main's result "did not read back". It did:rewrite_faultreads only the commit and the tree, so it returned None; the result lost itsfileskey, and validate-pins refused it after the write.tab-in-frontrow (a tab, thenfiles:) joins the header rows, so the tab half of_is_block_header's column guard is pinned (mutant M5 below). It is written as an escape in the file, which adds no character above U+007F.Measured first
At origin/main
50ce8ab15, withcheckandapplyrun through the real tool on theAtlasproject the partial-rollback tests scaffold, each run with a local--upstreamunder a stub gh (exit 97), no network.checkapplyat mainapplyherefiles:line has U+00A0 or U+3000 in frontupdate-pin-no-files, exit 2shape:line has U+00A0 or U+3000 in frontapply(as for a manifest with noshapekey at all)update-validators-red("after the rewrite, scripts/validate-manifest.py refused"), rolled backupdate-manifest-no-shape("project.yaml has noshape:block"), rolled back: what a manifest with noshape:line gets<blank>files:above the realfiles:applyupdate-validators-red(shape-pin-no-files): the rows went under the stray line andbreakdropped the real header; rolled back<blank>shape:above the realshape:applyupdate-manifest-unrecognised, "holds theshape:key twice, at lines 80 and 81", which is false of a file the reader reads as two keys; rolled backshape:line has U+00A0 after the colon (shape: <U+00A0>)apply, though both validators refuse the file (yaml-unparsable, "line 81: unexpected indentation": the reader givesshapethe one blank as its value, and the block under it is indented under a scalar)update-validators-red, the validators naming "line 81: unexpected indentation"; rolled backupdate-manifest-no-shape("project.yaml has noshape:block"), rolled backSo the answer to "do
checkandapplyreach the rewrite on such a file at all" is: for a pin with only the blanked line, no, the reader's ownupdate-pin-no-filesstops both commands first, and the header finder is reached only by a direct call (RV1188's_repincase). For the other four inputs they do. The unrecognised-header refusal the issue wants is the manifest'supdate-manifest-no-shape, now given; a pin that holds the blanked line beside the realfiles:is now rewritten as the reader reads it, where it was written wrong and rolled back.Design
_is_block_headerasks the reader's question:_split_key(_strip_comment(line.strip(_YAML_WHITE))) == (key, ""). A trailing U+00A0 or U+3000 is covered by the same call (shape: <blank>is a key with a value to the reader, and with the blank right after the colon the line has no key at all), so the unit rows pin both ends. Thelstripchange is the Direction's and keeps the function in one vocabulary; no row can tell it from the barelstrip()(see the mutants).The two other bare
.strip()calls: both leftRead against the reader, and measured, with the same probe.
rewrite_manifest'sline.strip()beforestartswith("commit:")andstartswith("tree_sha256:")(inside theshape:block). The same family: a stray<blank>commit:inside the block beside the realcommit:is counted, soapplyrefusesupdate-manifest-unrecognised, "has 2commitline(s)", rolled back, for a manifest the reader and both validators accept (two keys). It is a false refusal by name with the tree untouched, not a wrong write. Left because the issue lists it as not asked here. A follow-up would give itstrip(_YAML_WHITE)and rows of its own; a manifest holding only the stray line, with no realcommit:, was not measured._shape_block_bounds' end test,line.strip() and not line.startswith((" ", "\t")). A column-0 line of only U+00A0 or U+3000 reads blank to the barestrip(), so the block runs past it. The reader refuses the whole file for such a line (expected key: value, naming the line's text), so the run endsupdate-validators-red, rolled back, with or without the change, and the only thing the end test decides is the scope of a rewrite no validator will accept. Left: changing it changes no outcome a person can see._header_as_it_was'stail.strip()is the same call a third time, and unaffected: it only ever sees a real header, so its tail is a comment or a run of spaces and tabs.Out of scope
Nothing else in
update-shape.py.with_scalarinscripts/shape_materialize.pyalso measures a line's indentation with a barelstrip(); the one call that can hand it a blanked line isrewrite_manifest's, left above, so a follow-up starts there.Tests
Each run is one pytest at a time under a stub
gh(exit 97),GH_TOKEN=bogus, an emptyGH_CONFIG_DIR,GIT_SSH_COMMAND=false,GIT_CONFIG_GLOBAL=/dev/nullandEMAIL=tests@openreposhape.invalid:/usr/bin/python3 -m pytest -q -p no:cacheprovider --basetemp=<scratch> -o tmp_path_retention_policy=failed tests/<file>.New rows: 27 (the 5 header rows, the 8 manifest
damagerows, the 2 pin and 4 manifest carry-over rows, and 8 end-to-end runs). The selector I ran at main takes 26 of them and the two old rows oftest_a_files_header_with_trailing_blanks_only_is_written_bare, so it selects 28: on origin/main'supdate-shape.py24 of the 28 fail and 4 pass, the two old rows and the twoupdate-pin-no-filesend-to-end runs, which hold whatcheckandapplydo before the rewrite is reached and so cannot fail before. With this change all 28 pass. The 27th row,tab-in-front, passes at main as well as here: main already holds the tab half of the guard, so this row is no fix row but the pin of that half, which mutant M5 below drops.Run again after the fix round, at this head. Run at the first head and not again, because the fix round leaves
update-shape.pybyte for byte as it was and changes no test they run: tests/test_update_shape.py (90 passed in 193.71s), tests/test_pin_rewrite_layout.py (115 passed), tests/test_yaml_reader_spaces.py (513 passed), tests/test_bom_rewrites.py (12 passed), tests/test_bom_tool_io.py (126 passed), tests/test_agents_md_codes.py (47 passed) and tests/test_cli_reference.py (23 passed).tests/test_shape_doctor.pywas not run: this change does not touchshape-doctor.pyorscripts/repo_shape.py. No new bareis_file,is_dir,exists,is_symlinkoros.pathguard, and no new refusal code or message.Mutants
Each is a change to
update-shape.py, the new rows run (M5 over both test files), then the file is put back and checked withcmpagainst the committed bytes (cmp okeach time).line.strip(_YAML_WHITE)back to the bareline.strip()_YAML_WHITE, the back is the barerstrip()damagerows and the two trailing-blankfiles:rows)_is_block_header(theor <a tab> in line[:blanks]test)test_a_line_that_is_no_files_header_to_the_reader_is_still_counted_none[tab-in-front]fails and nothing else does. M5 survived every test before this row_YAML_WHITE, the front is the barelstrip()line.lstrip(_YAML_WHITE)back to the bareline.lstrip()lstriponly measures the blanks in front for the tab test, and a line it would treat differently starts with U+00A0 or U+3000, where thestrip(_YAML_WHITE)that follows already makes it no header. No row can tell them apart; it is kept because the issue's Direction names it and the function then speaks one vocabularyMerges
git merge-tree --write-treeof this head (aca00dfd9) with origin/main (0ed3e57f8) and with each open PR that touchesupdate-shape.pyor the two test files, listed live withgh pr listbefore the push (48 open; #1231 itself is the only one that touches the test files): PR 1166 (5102b4294) and PR 1133 (a7bfee1bd). All three are clean, exit 0, with no conflict lines.Closes #1220
Lane: openreposhape-3 (openRepoShape-3)
🤖 Generated with Claude Code