Skip to content

Make lane-end's synopsis say what a DUPLICATE retire does, read a state cell that breaks the phrase's grammar whole instead of narrowing it, and correct the TAKEOVER grammar table and two tests.yml header comments - #202

Merged
brettheap merged 15 commits into
mainfrom
fix/precision-90a-96-108-126
Oct 19, 2026

Conversation

@brettheap

@brettheap brettheap commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Three small precision fixes that lane openRepoTools-4 took over from lane openRepoTools-3 (TAKEOVER comments on each issue), and a note that #227 settled the fourth. The commits, oldest first: d77504c the proving tests, 1a22dd7 lane-end (#96 and #90 part a together), 3296573 the docs row, 42a1085 the tests.yml comments (superseded by #227, see below), 2c5b3f2 and 748e0ef the first fix round (the review's character-count finding, then the hygiene rule on wc), the merge of origin/main with #227, the merge with #212, then the second fix round (03a60f9 the tests for text that is not UTF-8 and for a failing wc, f2d559d the validity check, ffcca3c the validity check taken from an iconv -c round trip) and the merge with #216, #199 and #201. Brett Heap lifted the closeout direction for these follow-ups.

#96 — lane-end's synopsis said a DUPLICATE retire "ends nothing"

Wrong: the top-level USAGE entry read retire a live FORK (pid or uuid) or a DUPLICATE (pid); ends nothing. True of a FORK; false of a DUPLICATE HOLDER, whose retire TERMs the bg-pty-host pair (#39). --help prints that synopsis.
Fix: lane-end lines 30-34. The entry is split in two: a FORK "writes nothing and kills nothing"; a DUPLICATE "TERMs a background holder's pair, only reports any other; the row is not written", matching the OPTIONS paragraph below it.
Reconcile on landing: open PR #205 rewrites the duplicate-holder retire from "TERMs the pair" to "the bg-pty-host parent and every child". The synopsis here says "pair". The two PRs do not conflict textually, so git will not flag it: whichever of #202 and #205 lands second should reconcile the word.
Test: tests/test_precision_90a_96.py::test_the_synopsis_says_what_a_duplicate_holder_retire_does and ::test_the_synopsis_keeps_the_fork_and_the_duplicate_on_their_own_lines read the USAGE block of lane-end --help entry by entry, because the OPTIONS paragraph already says TERMs and a whole-help match would pass on main.

Closes #96

#108 — the TAKEOVER grammar table omitted the lane: form

Wrong: the table documented only TAKEOVER ← <stale claim URL>; the prose at the stale-claim paragraph (and the code) also write TAKEOVER ← lane:<dispossessed lane>.
Fix: docs/README-lanes.md line 678, one row, issue or PR, worded like its neighbours (a kind and the verb with its payload). Docs only, so no test.

Closes #108

#126 — nothing left to change here: #227 settled it

The branch first corrected two tests.yml header comments (item 2 and item 3 of the issue; commit 42a1085). Brett Heap's #227 ("run the test suite on Linux only") then rewrote that header and removed tests-windows, parse-macos and tests-macos, so the merge of origin/main takes main's tests.yml whole and this PR no longer touches it. Checked against main's new text:

  • item 3 (the checkout invariant omitted tests-no-submodule): main now reads "submodules: true, tests-no-submodule excepted: it checks out submodules: false on purpose". Accurate.
  • item 2 (workflow_dispatch was not named among tests-macos's homes): tests-macos is gone. The new on: comment says "push (main only) and the nightly schedule run those same three jobs", and workflow_dispatch runs them too. That is a smaller omission in ci: run the test suite on Linux only #227's own text, left alone here.
  • item 4 (AGENTS.md listed the pull_request jobs wrongly): ci: run the test suite on Linux only #227 rewrote the rule; it now names guard-launch-mode, tests and tests-no-submodule and the four triggers, which is accurate.
  • item 1 (a hygiene test pinning labeled and tests-macos's if:): not added, and moot, since no job reads the ready label now and the if: is gone. ci: run the test suite on Linux only #227 deleted the macOS parse hygiene test, and added no replacement for the trigger text.

AGENTS.md and tests/test_repo_hygiene.py are untouched by this PR.

Refs #126 (items 2, 3: settled by #227, no change here)

#90 part (a) — cell_for_scan skipped O1's grammar

Wrong: lane-end's narrowed scan of a register state cell checked the STATE word, the UTC shape and the part count, but not ratified decision O1's other rules for the phrase's line (at most 240 characters, no |, no CR/LF). A cell whose third part broke one was narrowed to its state word, so an open LANDING in that prose was never seen and the lane ENDED. That is the no-object-log fallback, the one path that still decides a merge hold from prose, and Rule 6 stops being enforced for it.
Fix: lane-end cell_for_scan and its comment (lines 1726-1789). The cap, | and CR/LF are checked beside the existing part-count test, and a cell that fails any of them is scanned whole, like a diary: fail-closed. Both call sites (this checkout's row and the published row on origin/main) share the function.
The cap is 240 characters, and ${#x} is bytes under a C locale, so cell_for_scan counts characters itself, the same in every locale: past 240, and only if the text is valid UTF-8, the count is redone with the continuation bytes taken out (tr -d '\200-\277' | wc -c | tr -d ' ', the padding strip test_no_shipped_bash_reaches_for_gnu_only_utilities_unaccompanied asks for). Without that, a valid 240-character line of — and → read as past the cap under LC_ALL=C and a lane merely waiting for a LANDING was held (the review's first finding). Text that is not valid UTF-8 keeps the byte count, which can only over-count: stray continuation bytes follow nothing and are no characters, and taking them out undercounted them, so a hand-edited non-UTF-8 cell of 241 bytes was accepted as 240 (the review's second finding). Validity is taken from an iconv -c -f UTF-8 -t UTF-8 round trip (the text is UTF-8 when it comes back unchanged), not from iconv's exit status: the bash:3.2 image's musl iconv exits 0 on invalid input, which the first version of the check trusted and so accepted all three cases there. A missing iconv or a failing wc reads as a line over the cap.
set-row-state does not count characters in every locale either: lanes-edit.sh's row_state_check counts bytes under LC_ALL=C, and a 240-character line of —/→ is refused there as "720 characters". That is filed as #234 and not touched here.

Tests (all but the controls fail on main, see below), each run under LC_ALL=C and a UTF-8 locale: tests/test_precision_90a_96.py

  • test_a_line_over_the_cap_cannot_hide_an_open_landing (a 241-character line ending LANDING #7)
  • test_a_line_with_a_pipe_or_a_carriage_return_cannot_hide_an_open_landing (two cases)
  • test_the_published_row_is_read_whole_when_it_is_not_the_phrase
  • test_a_wide_line_one_past_the_cap_cannot_hide_an_open_landing (241 characters of —/→)
  • test_a_line_that_is_not_utf8_cannot_hide_an_open_landing (three cases: 241 stray \x80 bytes, a 241-byte Latin-1 line with one °, and a valid 240-character line with one stray byte appended)
  • test_a_count_that_cannot_be_made_is_a_line_over_the_cap (a stub wc that fails on -c; the || cfs_len=241 fail-safe; C locale only, since a UTF-8 locale never recounts a 240-character line)
  • controls that must keep passing: test_a_line_exactly_at_the_cap_is_still_prose (240 characters naming LANDING #7 is a lane waiting, not a hold), test_a_wide_line_exactly_at_the_cap_is_still_prose (the same in —/→, about 680 bytes, which fails under LC_ALL=C with a byte count) and test_a_line_over_the_cap_that_holds_nothing_still_ends.

Each case runs a real lane-end in the lane-start sandbox with a stub lane-worktrees, so a cell wrongly read as clear is a lane that ENDS.

Refs #90 (part a). Part (b), lane-start's refused append, has an open question and is not touched.

Not touched, and nearby writers

None of the off-limits files is touched (the installer, tests/conftest.py, tests/run.sh, tests/test_repo_hygiene.py, AGENTS.md, specs/004-migrate-to-triad/**, lane-handoff, lane-worktrees, lane-end ~200-215 and ~1971-2143, docs/README-lanes.md past ~3830). README.md and .github/workflows/tests.yml are unchanged.
Other writers in nearby spots, which these hunks do not overlap: lane openRepoTools-5 edits docs/README-lanes.md ~597 and lane-end ~1107-1300 and ~1401-1449; another lane-4 writer edits lane-end ~711.

Verification

All local runs under the shared pytest lock, with every LANES_* and TMUX* unset, on 2026-10-18.

Second fix round, red to green (tests/test_precision_90a_96.py, 26 cases: each lane-end case runs under LC_ALL=C and under a UTF-8 locale):

  • Red: this head with lane-end put back to the first fix round's (27074e0: the tr recount with no validity check): 6 failed, 19 passed, 1 skipped. The six are test_a_line_that_is_not_utf8_cannot_hide_an_open_landing, its three inputs under both locales, each assert 0 == 2: lane-end exited 0 and ENDED the lane over the open LANDING. The skip is the failing-wc test under the UTF-8 locale, which never reaches wc.
  • Green: tests/test_precision_90a_96.py with tests/test_repo_hygiene.py (main f5c3bcd's hygiene rules, including the bench-root path rule): 214 passed, 1 skipped, the same skip; the same 214 and 1 again on 8ff52c2.
  • The failing-wc test (test_a_count_that_cannot_be_made_is_a_line_over_the_cap, C locale) pins the || cfs_len=241 fail-safe itself, red to green. A failing wc reads as a line over the cap either way, because without the || the count comes back empty and [ "" -le 240 ] is an error, which also reads as over the cap. That error is printed (lane-end: line 1771: [: : integer expression expected), so the test asserts "integer expression" not in result.stderr. Without the ||: 1 failed, 1 skipped ([C], at that assertion). With it, in the green run above: passed. And with a failing wc read as a SHORT line (|| cfs_len=0) it goes red as well: 1 failed, 1 skipped (assert 0 == 2, the lane ENDED over the LANDING).

bash 3.2: the extracted cell_for_scan ran on six cases (a valid 240-character line of —/→, a valid 241-character one, 241 stray \x80 bytes, a 241-byte Latin-1 line with one °, a valid 240-character line with one stray byte, an ASCII 240) under LC_ALL=C, POSIX and UTF-8, with bash 5.2 on the host and with bash 3.2.57 in the bash:3.2 image (musl iconv): all six correct everywhere, narrowed only for the two 240-character valid lines. The first version of the check, which trusted iconv's exit status, got all three invalid cases wrong in that image; that is why validity is an iconv -c round trip. With iconv off the PATH it fails closed (the valid wide line is held, the three invalid ones too).

CI: head ffcca3c, run 39086159056: tests, tests-no-submodule and guard-launch-mode green. Head 45e81c4 (the merge with main f5c3bcd), run 39094650878: tests, tests-no-submodule and guard-launch-mode green. Head 8ff52c2 (the pinned assertion), run 39112708025: see the checks.

Earlier rounds, for the record: main f87fd5a plus only the first test file gave 6 failed, 2 passed (the pipe and published-row cases fail as assert 0 == 2, the lane ENDED over the open LANDING); the first fix round's byte-counting lane-end against the extended tests gave 1 failed, 17 passed (test_a_wide_line_exactly_at_the_cap_is_still_prose[C], exit 2 where 0 is right: the review's false hold), and 18 passed once fixed.

CI's Linux tests job is the suite of record for everything else.

🤖 Generated with Claude Code

Summary by Sourcery

Harden lane-end state-cell interpretation and clarify its retire synopsis while completing the TAKEOVER grammar documentation.

Bug Fixes:

  • Correct the lane-end synopsis so FORK and DUPLICATE HOLDER retire behavior is described accurately.
  • Ensure malformed or overlong lane state cells are scanned as full prose, preventing hidden LANDING or CLAIMED markers from being missed.
  • Handle character counting consistently across locales and fail closed for invalid UTF-8 or unavailable length counts.

Enhancements:

  • Document the lane:<dispossessed lane> form of TAKEOVER state transitions.

Documentation:

  • Add the missing lane-based TAKEOVER form to the lane grammar table.

Tests:

  • Add precision tests covering retire synopsis output, malformed state cells, locale-sensitive character limits, invalid UTF-8, failing wc, and published rows.

Brett Heap and others added 4 commits October 13, 2026 18:22
…TE retire TERMs, and a line past the cap, or with a pipe or a CR, cannot hide an open LANDING (#96, #90 part a)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… cell whose line breaks O1's grammar whole instead of narrowing it (#96, #90 part a)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…able (#108)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…odule checkout and workflow_dispatch as a tests-macos home (#126 items 2, 3)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Oct 13, 2026

Copy link
Copy Markdown

Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 23 hours and 12 minutes by commenting @sourcery-ai review. Upgrade to get a review now.

@sourcery-ai

sourcery-ai Bot commented Oct 13, 2026

Copy link
Copy Markdown

Reviewer's Guide

This PR makes four precision corrections: it aligns lane-end help text with DUPLICATE retirement behavior, hardens malformed state-cell handling with fail-closed scanning and regression coverage, documents the missing lane TAKEOVER form, and updates tests workflow comments to reflect actual checkout and trigger behavior.

Flow diagram for fail-closed lane-end state-cell scanning

flowchart TD
    A[Read register state cell] --> B{Valid phrase shape?}
    B -->|Yes| C[Scan state word and phrase parts]
    B -->|No: over cap, pipe, CR/LF, or wrong part count| D[Scan whole cell]
    C --> E{Open LANDING detected?}
    D --> E
    E -->|Yes| F[Keep lane open]
    E -->|No| G[Allow lane to end]
Loading

File-Level Changes

Change Details Files
Clarify lane-end's retire synopsis for FORK versus DUPLICATE targets.
  • Split the help synopsis into separate FORK and DUPLICATE entries.
  • Describe DUPLICATE retirement as terminating the background holder pair while reporting other duplicates without writing the row.
  • Add help-output tests that inspect synopsis entries independently of the existing OPTIONS text.
lane-end
tests/test_precision_90a_96.py
Make malformed state cells fail closed instead of narrowing scans to the state word.
  • Require the O1 line-length, pipe, and CR/LF constraints before applying the narrowed state-cell parse.
  • Scan invalid cells in full for both the checkout row and the published origin/main row.
  • Add sandboxed lane-end tests for overlong, delimiter-containing, published, boundary-valid, and harmless malformed cells.
lane-end
tests/test_precision_90a_96.py
Document the lane-based TAKEOVER grammar form.
  • Add TAKEOVER ← lane:<dispossessed lane> to the issue-or-PR grammar table.
docs/README-lanes.md
Correct workflow header comments to match checkout and trigger behavior.
  • Identify both parse-macos and tests-no-submodule as intentional submodule-disabled checkouts.
  • Include workflow_dispatch among tests-macos's documented trigger sources and reflow the explanatory comments.
.github/workflows/tests.yml

Assessment against linked issues

Issue Objective Addressed Explanation
#96 Update lane-end’s top-level USAGE synopsis so it distinguishes FORK retirement, which writes and kills nothing, from DUPLICATE HOLDER retirement, which TERMs the holder pair. ✅
#108 Update the canonical TAKEOVER grammar table in docs/README-lanes.md to document TAKEOVER ← lane:<dispossessed lane> alongside the existing stale-claim-URL form. ✅

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Brett Heap and others added 3 commits October 13, 2026 20:07
…a valid wide line is not held under a C locale, and run the boundary tests under C and UTF-8 (#90 part a)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ene rule spells it (#90 part a)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ich already makes #126 items 2 and 3 accurate

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Brett Heap and others added 6 commits October 18, 2026 01:18
…-8, and reads a count it cannot make as a line over the cap (#90 part a)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…he byte count for any other text (#90 part a)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…n iconv's exit status, which musl's does not set (#90 part a)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… rule)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…e-end's test must not print an integer-expression error (#90 part a)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant