Repository navigation
Read the clause (c) TAIL case on a checkout at a short fixed path, and drive lane-start's row stamp at three checkout lengths, so the case no longer answers by the length of the run root (#180) - #215
Open
brettheap wants to merge 5 commits into
Conversation
… is a measured design and not a run root's accident The stamp's tail (Amendment 11(c)'s dir and window) is what cut_to_line takes first under ratified decision O1's cap; its length is the checkout path's, not the run root's. A real lane-start, started twice --no-launch under a $HOME at least as long as tests/run.sh's, with the checkout at a short fixed path (tail intact), at 100 characters (window lost, #180's shape) and at 160 (the path dropped whole, never truncated); the head, the 240 cap and the log's full facts are asserted in all three. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… its answer no longer depends on the run root (#180) Under $HOME/projects the stamp's line outgrew the 230 the cut keeps once the run root was tests/run.sh's, and the window ref was cut: red locally, green under CI's plain pytest. The tail is now read on a lane whose checkout is /tmp/ort-a11t.XXXXXX/repoA11t (29 characters anywhere); repoA11b-1's second start keeps its exit, no-launch and head assertions, and cleanup removes the one fixture outside $SANDBOX. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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 22 hours and 56 minutes by commenting |
Reviewer's GuideThe PR fixes flaky clause (c) TAIL coverage by decoupling the full-tail assertion from variable File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
…ases by what the cut takes SonarCloud counted the register seed as a 14-line copy of test_lane_start_claude_current.py's (4.7% new-code duplication against a 3% gate); it is now one table written in a loop, the register text byte-identical. The 100-character case was labelled #180's shape, but #180's row read `... no launch; dir ...`, the path dropped whole: the ids are now window-cut-dir-kept and path-dropped-whole-as-in-180, and the docstring says which is which. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…kout in the sandbox wherever its TMPDIR is short An inherited A11T_ROOT would have been removed by cleanup had the suite died before the clause (c) case. The checkout now goes under the sandbox's own $TMPDIR when that is at most 40 characters, and under /tmp only when it is not, with an assertion that it is at most 65 characters, so the line stays under about 210 wherever the root is. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tamp test's docstring Since c19150f the clause (c) case's checkout goes under the sandbox's own TMPDIR when that is short and under /tmp when it is not, so its length is bounded at 65 characters rather than fixed. Co-Authored-By: Claude Opus 5.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
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 #180
What was wrong
tests/test_lane_helpers.sh's clause (c) TAIL case asserted that repoA11b-1's row stamp endsdir $HOME/projects/repoA11b; window a11start:3 @42.lane-start's stamp is ONE line under ratified decision O1's 240-character cap (Amendment 13(a)).cut_to_linecuts it at 230 and drops the last partial word. The tail is the checkout path, so the case only passed while$HOMEwas short. Under plain pytest's/tmpit was green. Undertests/run.sh's per-run root (TMPDIRabout 71 characters, so$HOMEabout 91) the line passed 230, and the cut landed on the;right after the path, so the path went whole: the row read… window renamed, no launch; dir .... On macOS's runner the case was at the edge.Reproduced outside the shell suite with a real
lane-start, started twice--no-launch, under a 92-character$HOME:$HOME(107 characters):… window renamed, no launch; dir <107 chars>; .... The window is gone and the head is intact.$HOMEwith the checkout passed as--dir /tmp/ort-tail.XXXXXXXX/repoZ(28 characters):… no launch; dir /tmp/ort-tail.…/repoZ; window testsess:0 @1, 197 characters in all.Why (A), the test-side fix, and not (B)
The cut is behaving as designed. Nothing in the stamp is broken under long paths.
dirandwindowlast on purpose.lane-start's Amendment 13(a) comment above the stamp (:3668-3677) orders the tail launch first, thendirandwindow, so "a line long enough to be cut loses what a reader can look up rather than what they cannot". Both facts are sub-fields of the same act'sSTARTED/RESUMEDline in the lane's log, whichlane-dirreads and the new test asserts in full. Both are also columns of Amendment 11's listing. No reader parsesdirout of the state cell (checked with grep acrosslane-start,lane-end,lanes-edit.sh,lane,lanes,lane-rename,lane-worktrees,link-estates).launching claude --name <lane> --resume <uuid>) is past 230 beforedireven starts. So "the stamp drops dir/window under a long line" is today's normal behaviour, not an edge case of long paths.dir. Today a cut that lands inside the path drops it whole (dir ...), so the row never carries a truncated path that reads as a real one. (B)'s "truncate the dir, keep the facts" would put an elided path there. That changes the stamp format of a governed artefact, which is for the protocol to decide, not for a test fix.dir $HOME/…, which cannot fit under the cap atrun.sh's length whatever the stamp does.cut_to_lineis pinned.test_the_three_row_writers_carry_the_same_one_line_cut_byte_for_bytepins it byte for byte acrosslane-start,lane-endandlane-handoff.So the case reads the tail on a checkout whose path has the same length wherever the suite runs. No product code changes.
tests/run.shis untouched: lane 1's #195 edits it.What changed
tests/test_lane_helpers.sh(edited in place, never at the end; ranges are at this head):A11T_ROOT=""besideSANDBOX=, before the traps, socleanupnever acts on an inherited value.cleanup():101-103: removes$A11T_ROOT. It runs on EXIT, INT and TERM. Open PR A silent tmux is no longer read as a dead window, the companion verdict is read off the lane's live window from any window, and the guard's cure for a lane named before Rule 4 says what it cannot fill in (#73) #222 (at2946a41) also adds a line tocleanup()here, so the two conflict in this one hunk oftests/test_lane_helpers.sh(~:101). The resolution is to keep both lines, and it falls to whichever PR lands second.:5351-5356., no launchand head assertions. These do not depend on the root. One description now reads "…and its stamp says what actually happened" because its antecedent moved.repoA11t-1, whose checkout ismktemp -d <base>/ort-a11t.XXXXXX+/repoA11t.<base>is the sandbox's own$TMPDIRwhen that is at most 40 characters (plain pytest on Linux, where the checkout then lives inside$SANDBOX), and/tmpwhen it is not (run.sh, macOS). So the checkout is at most 65 characters, an assertion holds it there, and the line stays under about 210 wherever the root is. It runs in its own fake windowa11tail:5 @43, which nothing else in the suite uses.ort-a11t.*behind, and it is in/tmp, outsiderun.sh's per-run root. Any other exit is cleaned up bycleanup.--dir, then started again, and the second start reads the checkout back from the lane's log, the same shape as before.repoA11b-1keeps$HOME/projects/repoA11bas its recorded directory, because the later DIRECTORY PRECEDENCE dry-run (:5668) reads it back.tests/test_lane_start_row_stamp.py(new): three parametrized cases, each a reallane-startstarted twice--no-launchin a sandbox whose$HOMEis at leastrun.sh's length. The checkout is at an exact length under a/tmpmkdtemp, so the answer does not depend on pytest's own basetemp. The workspace seed is a data table written in a loop, which also clears SonarCloud's duplication gate againsttests/test_lane_start_claude_current.py:short-fixed-checkout: the tail keepsdirandwindow.window-cut-dir-kept(100 characters):dirsurvives and the window ref is gone.path-dropped-whole-as-in-180(160 characters): the path is dropped whole (no launch; dir ...) and never truncated. This is what tests/run.sh: a long run root truncates the row stamp so test_lane_helpers.sh's clause (c) TAIL case fails locally while CI's plain pytest passes #180's case read underrun.sh's root.All three also assert the head, the 240 cap, and the lane log's full
dir …; window ….Tests
flock -w 1800 ${TMPDIR:-/tmp}/openrepotools-pytest.lock, lock taken 20:40:57Z, at the fix round's tree):python3 -m pytest tests/test_lane_start_row_stamp.py -q: 3 passed in 17.69s (short-fixed-checkout,window-cut-dir-kept,path-dropped-whole-as-in-180).TMPDIR, the same shape asrun.sh's root:f87fd5a: 8 passed, 1 failed, and the one failure is the TAIL case. This is tests/run.sh: a long run root truncates the row stamp so test_lane_helpers.sh's clause (c) TAIL case fails locally while CI's plain pytest passes #180 reproduced.TMPDIR: 12 passed, 0 failed.TMPDIR=/tmp: 12 passed, 0 failed./tmp/ort-a11t.*was left after any of those runs.085a234(before this fix round): the Linuxtestsjob passed, 1422 passed and 9 skipped. Two sibling PRs on the same base that add no pytest cases show 1419 passed and 9 skipped, so the difference is the three new cases. The shell suite's wrapper passed too. Every other job passed, excepttests-macos, which waits for thereadylabel.085a234failed its quality gate on 4.7% new-code duplication: the register seed,:164-177, againsttests/test_lane_start_claude_current.py:198-211. The seed is restructured at this head.🤖 Generated with Claude Code
Summary by Sourcery
Stabilize lane row-stamp coverage by testing checkout paths at controlled lengths instead of relying on the surrounding run root.
Bug Fixes:
Enhancements:
Tests: