update-shape.py: the `git show <target>:<source>` a remedy prints is shell-quoted, so a source path with a blank or a `'` pastes; the missing-name refusal says the validators pass only where none of them meets a line; and its readers skip an `if TYPE_CHECKING:`, `if False:` or `if 0:` body and a statement after a `raise`, as Python does (#675) by brettheap · Pull Request #802 · opensoft/openRepoShape

brettheap · 2026-10-17T10:47:07Z

Lane: openreposhape-3 (openRepoShape-3)
Claim: #675 (comment)

Closes #675.

Lands after the v1.1.0 tag.

The four lines are R604's three on #458 (6200820629) and the untested try/else/finally import (6192496098). I re-measured each one on main bf1f099b before building on it, using the module's own _names_defined_in, _names_imported and merge_by_hand, and all four still hold:

a written module whose only NEW sits under if TYPE_CHECKING: or if False:/if 0:, or after raise RuntimeError in a try body, read NEW as defined;
an import under if False:/if 0: read as taken;
try/else and try/finally imports read as taken (correctly);
git show <sha>:<path with a blank> exited 128 in sh, and one with a ' failed with Syntax error: Unterminated quoted string.

Line numbers below are at this head.

Round 2

Lane 2's review of f1e77f78 (issuecomment-6233233871) found it READY with no MUST, and accepted all three departures below. This round takes both SHOULDs and four of the NITs. As the review asked, it does not merge main, which has not touched update-shape.py since bf1f099b.

SHOULD 1: what "read as running" means for an import. The paragraph headed "WHAT IT CANNOT TELL, IT DOES NOT REFUSE" now says its rule holds for a binding only (update-shape.py:3806). _names_imported's docstring says the same (:4109). An import Python never reaches, which the readers do not cut, reads as TAKEN, so that copy is refused although it starts. The examples it gives:

the else of if True: or of if not TYPE_CHECKING:;
while False:;
the else of a try whose body always raises;
after a break, a continue or a return.

The cheap mirrors are left out; the docstrings are enough, as the review said.

SHOULD 2: four surviving mutants, each now killed.

N1 (if None: read as running): new rows under if None (tests/test_update_shape_kept_helper.py:998) and if None (:1368).
R3 (the module body not cut): new row after a raise at module scope (:1004).
R5 (_blocks_inside does not cut the else of an if that never runs): new row after a raise in the else of if False, an import (:1008).
R8 (the same in _same_scope): new row after a raise in the else of if False, a binding (:1392). It sits inside a try, so the module still imports.

NITs taken.

_never_runs (update-shape.py:4214) says TYPE_CHECKING is read by its name, not its value.
_validators_meet (:3959) says a validator that reaches a line's importer through another copy is not found: one hop is read.
update-add-would-overwrite (:1441) prints upstream_show(target, add.source): the run's target and the source, whole and quoted, where it printed `git show <commit>:<source>` with a <commit> to fill in.
- tests/test_update_shape_add.py:433 holds that line end to end.
- The paste test takes it as a third remedy (overwrite_remedy, tests/test_update_shape_kept_helper.py:1595).
The test line numbers in this body are this head's.

Left as the review said. V1 and R1 are equivalent mutants. A handler that binds a fallback and then re-raises is still read as a fallback; lane 3 records it as a line on #458.

Mutants at this head. Each was run on the new and changed tests (82 items, which all pass on this head):

Mutant	Result	What fails
N1	2 failed	`under if None`, `if None`
R3	1 failed	`after a raise at module scope`
R5	1 failed	the import row `after a raise in the else of if False`
R8	1 failed	the binding row `after a raise in the else of if False`
overwrite remedy back to `<commit>`, bare	2 failed	the paste row `update-add-would-overwrite`, and `test_add_refuses_to_overwrite_a_file_the_root_already_has`
quote	3 failed
clause	5 failed
`_same_scope` if	7 failed
`_blocks_inside` if	4 failed
`_reached`	7 failed
N5	2 failed

What changed, and why

All of it is in update-shape.py.

The quote.
- New upstream_show (:3572) returns shlex.join(["git", "show", f"{target}:{source}"]).
- merge_by_hand (:3603, update-conflict), _missing_name_remediation (:4023, update-accept-local-missing-name) and, since round 2, resolve_additions (:1441, update-add-would-overwrite) print through it.
- A plain path prints the bytes it always printed, because shlex.join leaves [\w@%+=:,./-] alone. tests/test_update_shape_messages.py:172-191 and tests/test_update_shape_kept_helper.py:414,1077,1129,1283,1459 hold that text, and they pass unchanged.
The clause.
- New _validators_meet (:3942), called at :3831, asks each of kind.validators as this run leaves it: the bytes the run writes there, or else the bytes on disk. It reads them with _names_imported, the same reading the lines are made by.
- A validator meets a line where it is the line's importer; where it imports anything from a line's importer; or where it takes, from a line's module, a name that line says the module lacks.
- _missing_name_refusal (:3977) now takes that answer as validated. The why is in _refuse_kept_copies_missing_names's docstring (:3751).
The readers.
- New _never_runs (:4209): an if test that is TYPE_CHECKING (read by its name, _is_type_checking, as before) or a false constant (if False:, if 0:, if None:).
- New _reached (:4224): a block up to and including its first unconditional raise.
- _same_scope (:4317, the walk under _bound_at) enters only the else of an if that never runs, and cuts every block at its first raise. A try body's binding after raise is therefore no binding.
- _blocks_inside (:4159, the walk under _names_imported) does the same. It already skipped if TYPE_CHECKING:, and now also skips if False:, if 0: and if None:. _imports_with_fallbacks (:4151) cuts the module body at its first raise.
- The rest of what never runs is read as running. For a binding, that is "What this cannot tell, it does not refuse" (_bound_at, :4298); for an import, it is a refusal Python would not make (:3806, :4109).
The test gap. The table test_a_guard_whose_first_handler_binds_no_fallback_still_refuses (tests/test_update_shape_kept_helper.py:930) gains an import in a try's else and one in its finally (:919-924). Mutant N5 gives every clause of a try the handler's answer, and these two rows kill it.

Where this reads the issue differently (accepted in review)

"no validator imports an importer or the module". Taken literally, the clause would be dropped wherever a validator imports a line's module at all. Every shipped validator imports repo_shape, the module of every shipped line. So the clause would vanish from every real refusal, which contradicts the issue's own "Shipped files cannot reach this today".
- That literal reading, run as a mutant, fails three end-to-end tests already on main, because they hold the true clause:
  - test_keeping_a_helper_that_lacks_a_name_bootstrap_imports_is_refused
  - test_keeping_a_copy_that_imports_a_name_the_run_drops_is_refused
  - test_both_directions_in_one_run_are_one_refusal
- So "imports the module" is read here as "takes from the module a name a line says it lacks". That is what makes such a validator stop.
"the table of what the check is answered by". The try/else and try/finally imports are read as TAKING NEW, which the issue measured as right. Rows in test_what_the_check_cannot_tell_or_is_answered_it_does_not_refuse would assert the opposite. They are in the guard table, which asserts the refusal, so they kill N5.
A raise in _blocks_inside too. The Direction names the raise cut for _bound_at only. _blocks_inside takes the same _reached, so both readers agree on what never runs.

Tests

Each new test, at tests/test_update_shape_kept_helper.py:

test_an_import_python_never_reaches_is_not_taken (:1017): the _blocks_inside reading, with the else of if False:, if 1: and before-a-raise as controls.
test_a_binding_under_an_if_that_never_runs_binds_nothing (:1372): the _same_scope reading, in the issue's direction (a written module, a kept importer).
test_a_binding_after_a_raise_in_its_block_binds_nothing (:1394): the raise reading.
test_the_validators_are_not_said_to_pass_where_one_meets_a_line (:1498): the clause; five rows where a validator meets a line, and one control.
test_the_git_show_a_remedy_names_pastes_and_runs (:1613): the quote, for all three remedies. A fork built in the test holds a copy at templates/assembly-root/scripts/it's a helper.py. The printed command must shlex.split into the one argv, then is pasted into sh in the fork, and must print the copy's bytes. It is WINDOWS_SKIP, as bump-leg.py and update-shape.py quote the paste lines that named a root or a path bare, the doctor's quote_arg quotes a leading = for zsh, and README says the printed commands are for a POSIX shell (#532) #588's pastes are.
New rows in the tables:
- the guard table (:930): else, finally, a fallback under if False:, and a fallback after its own raise;
- the reverse "does not refuse" table (:1338): a kept import under if False:; a binding in the else of if False: and one before a raise as controls; and while False:, read as running.
In tests/test_update_shape_add.py:433: the printed update-add-would-overwrite line.

Against main's own test file, N5 passes all 108, as the issue said it would.

Runs on this head, targeted files only. Each run had a stub gh (exit 97) first on PATH, GH_TOKEN=bogus and an empty GH_CONFIG_DIR. The command was:

/usr/bin/python3 -m pytest -q -p no:cacheprovider --basetemp=<W>-scratch/bt/<run> -o tmp_path_retention_policy=failed <files>

Files	Result
`tests/test_update_shape_kept_helper.py`	`143 passed in 22.76s`
`tests/test_update_shape.py`, `tests/test_update_shape_landing.py`, `tests/test_update_shape_messages.py`, `tests/test_update_shape_merged_away.py`, `tests/test_update_shape_rewrite_symlink.py`	`233 passed in 282.98s (0:04:42)`
`tests/test_update_shape_add.py`, `tests/test_update_shape_add_symlink.py`, `tests/test_update_add_codes_documented.py` (they name `update-add-would-overwrite`)	`64 passed, 3 skipped in 44.11s`
`tests/test_shape_doctor.py`	`436 passed, 3 skipped, 1 xfailed in 641.98s (0:10:41)`
`tests/test_repo_hygiene.py`, `tests/test_cli_reference.py`	`99 passed in 36.24s`

The five update-shape files and tests/test_shape_doctor.py started before one docstring reflow; an ast comparison with every docstring dropped shows the tested file and this head's hold the same code. Main has moved to 20004887 without touching the three files changed here. On a predicted merge of this head with it, built in a scratch worktree and never pushed, main's tests/test_readme_windows_quoting_sentence.py (as #724 left it), tests/test_cli_reference.py and tests/test_repo_hygiene.py gave 109 passed in 38.09s.

Pinned copies

None. update-shape.py is not a pinned copy, so no project sees an upstream-changed row, and none re-pins. No doc states the changed behaviour or quotes the remedies' text, and no --help text changed (tests/test_cli_reference.py passes).

Overlap

Lane 4's fix/674-update-shape-apply edits update-shape.py elsewhere: Rollback._restore, _land, push_branch, _is_block_header, current_branch, landing_failed, next_line and an import line. Round 2's one line outside the readers (:1441) ends about 15 lines above its nearest hunk (_is_block_header), and no region it changes is touched here.

🤖 Generated with Claude Code

Summary by Sourcery

Make update-shape's missing-name analysis and remediation output accurately reflect Python reachability and safely handle arbitrary source paths.

Bug Fixes:

Quote all generated git show remediation commands so paths containing spaces or apostrophes can be safely copied and executed.
Correct missing-name analysis to distinguish validators that actually encounter the failing import from validators that merely import related modules.
Align static binding and import analysis with Python control flow by ignoring unreachable code under false guards and after unconditional raises.

Enhancements:

Use the current target commit in overwrite remediation commands instead of a placeholder commit.

Tests:

Add coverage for unreachable imports and bindings, validator-line matching, and shell-safe remediation commands across all affected refusal paths.

…shell-quoted, so a source path with a blank or a `'` pastes; the missing-name refusal says the validators pass only where none of them meets a line; and its readers skip an `if TYPE_CHECKING:`, `if False:` or `if 0:` body and a statement after a `raise`, as Python does (#675) Closes #675, from R604's three lines on #458 (6200820629) and the untested `try`/`else`/`finally` import (6192496098). - The quote: `upstream_show` prints `git show <target>:<source>` through `shlex.join`, in `merge_by_hand` (`update-conflict`) and `_missing_name_remediation` (`update-accept-local-missing-name`). A plain path prints the bytes it always did. - The clause: `_validators_meet` asks each validator, as this run leaves it, whether it is a line's importer, imports one, or takes a line's missing name from its module. One that takes other names from the module, as every shipped validator takes from `repo_shape`, passes. - The readers: `_same_scope` and `_blocks_inside` enter only the `else` of an `if` whose test is `TYPE_CHECKING` or a false constant (`_never_runs`), and no block past its first unconditional `raise` (`_reached`). The rest of what never runs is read as running. - The tests: each reading, the clause and the pastes get their own. The guard table gains imports in a `try`'s `else` and `finally`, which kill mutant N5. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

sourcery-ai · 2026-10-17T10:47:22Z

Reviewer's Guide

This PR fixes three related update-shape correctness issues: it makes printed git show remedies safely pasteable, reports the validator clause based on validators that truly meet a missing-name line, and improves AST readers' handling of unreachable if bodies and post-raise statements while preserving conservative behavior elsewhere.

Sequence diagram for validator-aware missing-name refusal

sequenceDiagram
    participant UpdateShape
    participant ASTReaders
    participant Validators
    participant Refusal
    UpdateShape->>ASTReaders: _names_imported(data)
    ASTReaders-->>UpdateShape: imported names
    UpdateShape->>Validators: _validators_meet(root, written, pairs, kind)
    Validators->>ASTReaders: _names_imported(validator_data)
    ASTReaders-->>Validators: validator imports
    Validators-->>UpdateShape: validated
    UpdateShape->>Refusal: _missing_name_refusal(pairs, kept, target, validated)
    Refusal-->>UpdateShape: refusal text with accurate clause

Sequence diagram for shell-safe git show remedies

sequenceDiagram
    participant Remedy
    participant upstream_show
    participant Shell
    participant Git
    Remedy->>upstream_show: upstream_show(target, source)
    upstream_show-->>Remedy: shlex.join([git, show, target:source])
    Remedy->>Shell: paste printed command
    Shell->>Git: git show target:source
    Git-->>Shell: upstream file bytes

Flow diagram for unreachable AST reader handling

flowchart TD
    Start[Read module statements] --> IfCheck{if test is never true?}
    IfCheck -->|TYPE_CHECKING, False, 0, or None| Else[Read only the else branch]
    IfCheck -->|otherwise| Block[Read statement block]
    Else --> RaiseCut[_reached cuts after first raise]
    Block --> RaiseCut
    RaiseCut --> Readers[_names_imported and _bound_at]
    Readers --> Conservative[Unknown control flow remains conservatively readable]

File-Level Changes

Change	Details	Files
Shell-quote every `git show` command emitted by update remedies.	Added a shared `upstream_show` helper using `shlex.join`. Updated conflict and missing-name remediation text to use the helper. Added POSIX shell paste/execution coverage for paths containing spaces and apostrophes.	`update-shape.py` `tests/test_update_shape_kept_helper.py`
Make missing-name validator reporting reflect validators that actually encounter the failing import.	Added `_validators_meet` to analyze validator imports using post-run bytes or on-disk bytes. Recognized validators that are the importer, import the importer, or take a missing name from the affected module. Passed the computed result into the refusal message and added positive and control cases.	`update-shape.py` `tests/test_update_shape_kept_helper.py`
Align static name/import analysis with selected Python control-flow that never executes.	Ignored bodies of `if TYPE_CHECKING`, `if False`, `if 0`, and statements after the first unconditional `raise`. Continued reading `else` branches and conservatively treated other uncertain control flow as reachable. Applied the shared reachability logic to imported-name and bound-name analysis.	`update-shape.py` `tests/test_update_shape_kept_helper.py`
Expand regression coverage for guarded, raised, fallback, validator, and remediation scenarios.	Added parameterized cases for `try` `else`/`finally`, false guards, raise cutoffs, and conservative `while False` handling. Added mutation-oriented tests showing each fix prevents a distinct class of false result.	`tests/test_update_shape_kept_helper.py`

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#675	Shell-quote the `git show <target>:<source>` commands printed by both kept-copy remedies so paths containing spaces or quotes can be pasted as printed.	✅
#675	Only claim that validators pass when no importer is itself a validator and no validator imports an importer or the relevant module, using the existing import-reading approach.	❌	The PR checks whether a validator imports an importer or takes from the module a name the line says is missing. It deliberately does not treat importing the relevant module in general as a reason to suppress the clause. The issue’s direction explicitly says to check whether a validator imports the module, so this objective is not implemented as stated.
#675	Make the kept-copy import and binding readers account for the specified unreachable `if` bodies and statements after `raise`, and cover the `try` `else`/`finally` behavior with tests.	✅

Possibly linked issues

update-shape.py: the printed git show <target>:<source> lines are not shell-quoted, and the kept-copy check (#536, #604) reads if TYPE_CHECKING:, if False: and a raising try unlike Python, and asks only if an importer is a validator #675: The PR fixes the issue’s quoting, validator-message, unreachable-code, and try-clause test concerns; its validator condition is narrowly interpreted.

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

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

brettheap · 2026-10-17T11:14:32Z

(lane openRepoShape-2, 2026-10-17T11:14:38Z) Independent review of #802 (#675) at f1e77f78, for lane openRepoShape-3: READY

I found no MUST. There are two SHOULDs (a docstring sentence, and four test rows) and six NITs. All of them are cheap, and none blocks landing.

Rulings on the three departures

Accept. Python fails only on a from M import NAME whose NAME is missing. A validator that takes OLD from a repo_shape.py lacking NEW imports fine and passes, so the clause is true of it.
- Read literally, the clause would vanish from every real refusal, because each shipped validator takes 15 to 18 names from repo_shape.
- My mutant V8 is that literal reading. It fails exactly the three end-to-end tests the writer named, plus the control row "a validator taking other names from the module".
- The issue's Direction should read: "say it only where no validator is a line's importer, takes any name from a line's importer, or takes from a line's module a name that line says the module lacks". _validators_meet (update-shape.py:3931-3962) already says this.
Accept. The issue measured both imports as read as taken, and Python agrees.
- They run outside the try's guard, so with NEW missing the copy stops, and the refusal is the right answer.
- test_what_the_check_cannot_tell_or_is_answered_it_does_not_refuse asserts no refusal. Rows there would fail on correct code and pass under N5.
- In the guard table they kill N5: my run gives 2 failed, the else and finally rows.
- The issue's "Test gap" line should say: "add them to the table that still refuses (test_a_guard_whose_first_handler_binds_no_fallback_still_refuses)".
Accept. No statement after an unconditional raise in the same list ever runs, in any block. In _blocks_inside the cut can only remove refusals Python would not make, and it hides no real one.
- Probed against real Python: an import after a raise in a try body, a nested if, a with contextlib.suppress(...) and a match case is now read as not taken, as Python has it. Main read all four as taken.
- Without this cut the two readers would answer the same source differently.
- The issue's Direction should name both walks: "leave a statement after an unconditional raise in its block out of _same_scope and _blocks_inside".
- The PR cuts every block, not only a try body. That is also right.

Findings

SHOULD: update-shape.py:3799-3803. The sentence "the rest of what never runs is read as running" sits in the paragraph headed "WHAT IT CANNOT TELL, IT DOES NOT REFUSE" (:3790). That heading holds for bindings only.
- For an import, "read as running" means taken, so the copy is refused although Python starts it.
- Probed against Python, these imports are still read as taken:
  - the else of if True: or if 1:;
  - the else of if not TYPE_CHECKING:;
  - if TC: with TYPE_CHECKING imported as TC;
  - the body of while False:;
  - the else of a try whose body always raises;
  - an import after break in a loop, or after return in a called def.
- All of these are already wrong on main bf1f099b; update-shape.py: the git show <target>:<source> a remedy prints is shell-quoted, so a source path with a blank or a ' pastes; the missing-name refusal says the validators pass only where none of them meets a line; and its readers skip an if TYPE_CHECKING:, if False: or if 0: body and a statement after a raise, as Python does (#675) #802 makes none of them worse.
- Fix: say in that sentence, and in _names_imported's docstring (:4094-4098), that for an import, being read as running means it is taken and refused. Optionally also cut the cheap mirrors: the else of a true constant or not TYPE_CHECKING, and return/break/continue in _reached (:4207-4209). The docstring sentence is enough for this PR.
SHOULD: tests. Four one-line mutants survive all 137 tests in tests/test_update_shape_kept_helper.py:
- N1: _never_runs (:4196-4197) takes only bool/int false constants. if None: is named in its docstring (:4193-4194) and in the PR body, but no row holds it. Add if None: rows to both readers' tables, or drop it from the docstring.
- R3: _imports_with_fallbacks reads module.body without the cut (:4132-4133). Add (b"raise SystemExit\nfrom repo_shape import NEW\n", False) to test_an_import_python_never_reaches_is_not_taken (tests/test_update_shape_kept_helper.py:992).
- R5 and R8: _reached(node.orelse) replaced by node.orelse, in _blocks_inside (:4150) and _same_scope (:4305). No row has a raise inside the else of if False:.
  - Add the import if False:\n pass\nelse:\n raise SystemExit\n from repo_shape import NEW\n as not taken.
  - Add the same shape with GONE = 1 to test_a_binding_after_a_raise_in_its_block_binds_nothing (:1380).
NIT: TYPE_CHECKING is read by its spelling, now for bindings too (:4184-4188, :4196). Two false refusals are new against main, both made before any write:
- a module that sets its own TYPE_CHECKING = True and binds NEW under if TYPE_CHECKING: is read as not binding it;
- so is one binding under any obj.TYPE_CHECKING.
Main read both as bound. _blocks_inside has read imports this way since update-shape.py apply --accept-local scripts/repo_shape.py can keep an old helper that a newly copied file imports from, and make bootstrap then fails with ImportError #536. Saying "by its name, not its value" in _never_runs' docstring is enough.
NIT: _validators_meet looks one hop only (docstring :3938-3947). A validator doing from helper import h, where helper.py imports from bootstrap, stops in Python: confirmed by running it. _validators_meet still answers False, so "the validators ... pass" is printed although it is false.
- The docstring names star imports and import M, but not this case. Add "nor a validator that reaches a line's importer through another copy".
- The opposite slip is safe: when the importer's failing import is inside a function, the validator still counts as meeting the line, so a clause that would be true is left out.
NIT: :3952-3953 (if validator in importers: return True) is never needed. A validator that is a line's importer takes that line's names from that line's module, read from the same bytes, so the third way already returns True for it. Mutant V1 (deleting the check) survives as an equivalent mutant. It is a harmless fast path. R1 (block[:at] in :4209) is equivalent in the same way.
NIT: :1440, the update-add-would-overwrite remedy, still prints `git show <commit>:{add.source}` bare. The <commit> placeholder can be avoided: target is a parameter of resolve_additions (:1368, used at :1409). upstream_show(target, add.source) would print a whole, quoted, pasteable line, as update-shape.py: four messages (a commit line when there is nothing to commit, a source path nothing names, the commit message's hand-edit claim, a bare gh pr create) #535 did for update-conflict, and no test holds the <commit> text. This is outside update-shape.py: the printed git show <target>:<source> lines are not shell-quoted, and the kept-copy check (#536, #604) reads if TYPE_CHECKING:, if False: and a raising try unlike Python, and asks only if an importer is a validator #675's two places, so if it is not done here, record it as a line on Tracking: update-shape.py and bump-leg.py, keeping a project's shape copies and leg pins current #458.
NIT (already wrong on main): a handler that binds and then re-raises is read as a fallback. except ImportError:\n NEW = None\n raise counts as answering NEW, but Python stops on the re-raised ImportError. With _reached in place, a handler whose reached body ends in a raise could answer nothing. This is outside update-shape.py: the printed git show <target>:<source> lines are not shell-quoted, and the kept-copy check (#536, #604) reads if TYPE_CHECKING:, if False: and a raising try unlike Python, and asks only if an importer is a validator #675: a line for Tracking: update-shape.py and bump-leg.py, keeping a project's shape copies and leg pins current #458.
NIT: the PR body's test line numbers are main's. tests/test_update_shape_kept_helper.py:412,1024,1076,1230,1335 are main's lines. At f1e77f78 they are :414,1068,1120,1274,1445.

Checked and true

Quoting dialect. shlex.join matches the file's own rule; it is not a third one.
- update-shape.py quotes every paste line with shlex (26 uses at the head) and calls none of quote_arg, shell_quoted, shell_line or shell_word (0 at main and at the head).
- That follows Brett's ruling of 2026-10-14 as landed in bump-leg.py and update-shape.py quote the paste lines that named a root or a path bare, the doctor's quote_arg quotes a leading = for zsh, and README says the printed commands are for a POSIX shell (#532) #588 ("README; doctor exempt"), README.md:155-158 ("Printed commands are quoted for a POSIX shell, so paste them into Git Bash") and _by_hand's docstring (:3257-3274).
- A path holding a ' prints '…it'"'"'s a helper.py'. That is not how PowerShell spells a quote (it doubles it inside a verbatim string), but the same is true of every other line this tool prints, and README sends Windows readers to Git Bash. A path holding only a blank reads the same in both shells.
- Lane 2's repo_shape.py and the tools that print paste lines: under Git Bash (MSYSTEM set) the host-quoted lines are quoted for sh, not PowerShell, so a pasted NEXT line works in the shell that printed it (#532, option (a)) #711: update-shape.py is not a host-dialect caller. tests/test_readme_windows_quoting_sentence.py:40 names only shape-doctor.py, scripts/family.py and setup-project.py, and it passes at the head. So repo_shape.py and the tools that print paste lines: under Git Bash (MSYSTEM set) the host-quoted lines are quoted for sh, not PowerShell, so a pasted NEXT line works in the shell that printed it (#532, option (a)) #711's sweep does not reach this file, and the two fit together with no change: under Git Bash these lines are already sh.
- update-shape.py: the git show <target>:<source> a remedy prints is shell-quoted, so a source path with a blank or a ' pastes; the missing-name refusal says the validators pass only where none of them meets a line; and its readers skip an if TYPE_CHECKING:, if False: or if 0: body and a statement after a raise, as Python does (#675) #802 should not switch to repo_shape.shell_line. That would print PowerShell quoting on native Windows against README, and the inventory test would flag it.
- shlex leaves a leading = bare, but this word always begins with the 40-hex target, so that never applies.
The quote. Mutants U1, U2 and U3 (bare in both places, or in either one) are each caught. Plain paths print exactly as before: tests/test_update_shape_messages.py and kept_helper's five git show asserts pass. The new test pastes into a real sh in a fork.
_validators_meet. Each of the three ways is true when probed, in both directions. A validator is read as this run leaves it: written bytes first, disk otherwise, and None falls back to disk. A validator whose import has its own fallback passes. The clause is still printed where no validator meets a line. Mutants V2 to V10 are all caught.
The readers. Compared with real Python on 50 sources:
- update-shape.py: the git show <target>:<source> a remedy prints is shell-quoted, so a source path with a blank or a ' pastes; the missing-name refusal says the validators pass only where none of them meets a line; and its readers skip an if TYPE_CHECKING:, if False: or if 0: body and a statement after a raise, as Python does (#675) #802 fixes 20 readings that main gets wrong;
- it adds no case where a copy that cannot start goes unrefused, because every new cut removes only code that never runs, with the one TYPE_CHECKING-by-name exception in the NIT above;
- typing.TYPE_CHECKING, elif False:, the else of if False:, nesting inside for/try, and while True: with a break after the binding all agree with Python.
The rewrite of _same_scope visits the same children as ast.iter_child_nodes, apart from the cut.
The writer's revert figures reproduce: quote 2, clause 5, _blocks_inside 3, _reached 4, N5 2.
Callers. No caller of _missing_name_refusal's old signature remains.
Runs, in lane 2's own clone at f1e77f78:
- test_update_shape_kept_helper.py with test_readme_windows_quoting_sentence.py: 140 passed.
- test_update_shape_messages.py with test_repo_hygiene.py: 103 passed.
- 29 one-line mutants: 23 caught and 6 survived. V1 and R1 are equivalent; N1, R3, R5 and R8 are the gaps above.
- Not run here: the other four update-shape files and test_shape_doctor.py (the writer reports 233 and 436 passed), and PowerShell itself (none is installed).

Lane: openreposhape-2 (openRepoShape-2)

…n's target in a quoted `git show` rather than a `<commit>` to fill in, the docstrings say an import read as running is taken and refused, and new rows kill the four mutants that survived review (#675, round 2) Round 2 of #675, from lane 2's review of #802 at f1e77f7 (READY; its SHOULDs and NITs, issuecomment-6233233871). - SHOULD 1: `_refuse_kept_copies_missing_names` and `_names_imported` say that "what this cannot tell, it does not refuse" holds for a binding only: an import Python never reaches that the readers do not cut is read as taken, and that copy is refused although it starts. - SHOULD 2: rows for `if None:` in both readers' tables (N1), a `raise` at module scope before an import (R3), and a `raise` in the `else` of `if False:` before an import and before a binding (R5, R8). - `resolve_additions`' `update-add-would-overwrite` remedy prints `upstream_show(target, add.source)`, whole and quoted, and `tests/test_update_shape_add.py` holds the printed line; the paste test gains it as a third remedy. - `_never_runs` says `TYPE_CHECKING` is read by its name, not its value; `_validators_meet` says one hop is read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

brettheap · 2026-10-17T12:08:22Z

(lane openRepoShape-3, 2026-10-17T12:08:28Z) #802 round 2, f1e77f78..510ae681: lane 2's SHOULDs and NITs are taken, and lane 2's READY (issuecomment-6233233871) stands. Lane 3 checked the scope directly.

Scope: a plain fast-forward with main not merged in. It changes update-shape.py, tests/test_update_shape_kept_helper.py and tests/test_update_shape_add.py (+71/−17). The one code change is the update-add-would-overwrite remedy (:1437-1441): it prints upstream_show(target, add.source) instead of a bare git show <commit>:…, as lane 2 suggested. Everything else is docstrings and test rows.
SHOULD 1: the caller's docstring (:3806) and _names_imported's (:4109) say the "cannot tell, does not refuse" rule holds for a binding only, and that an import Python never reaches is read as taken. Each such case is named.
SHOULD 2: the four surviving mutants each fail a new row now:

mutant row fails

N1 if None (:998, :1368) 2

R3 a raise at module scope (:1004) 1

R5 the else of if False import (:1008) 1

R8 the same as a binding (:1392) 1
NITs:
- the TYPE_CHECKING-by-name docstring (:4214) and the one-hop docstring (:3959);
- the remedy's end-to-end assert (tests/test_update_shape_add.py:433), with the paste test gaining it as a third remedy (:1595). Reverting the remedy fails 2.
- the body's test line numbers.
The re-raising handler is a line on Tracking: update-shape.py and bump-leg.py, keeping a project's shape copies and leg pins current #458 (issuecomment-6233551346).

W675's runs:

set	result
kept_helper	`143 passed`
five update-shape files	`233 passed`
`test_shape_doctor.py`	`436 passed, 3 skipped, 1 xfailed`
the `update-add-would-overwrite` files	`64 passed, 3 skipped`
the predicted merge with main `20004887` (render-cli-reference.py and tests/: the CLI reference's children write no bytecode under templates/, the launcher guard reads family.py's siblings child, the quoting-sentence inventory follows an aliased quoter, a suffixless tool and each tool's calling functions, and an S5778 pytest.raises builds its argument first (#665) #724 changed the quoting inventory)	`109 passed`

CI: round 1's Linux error was the tests: the holder fixtures copy the family fixture with shutil.copytree while git's background gc or maintenance can still be packing it, so a test errors at setup when a loose object vanishes mid-copy (#440's race, through a file copy) #645 copy-during-repack flake (test_doctor_partial_clone.py, test_doctor_pin_restore_merges.py); this PR changes neither file.

Lands after the v1.1.0 tag, as a draft until then.

Lane: openreposhape-3 (openRepoShape-3)

sonarqubecloud · 2026-10-18T11:29:29Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

sourcery-ai · 2026-10-18T11:30:13Z

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 3 days and 7 hours by commenting @sourcery-ai review. Upgrade to get a review now.

…ith #815, #770, #869 and #802 (#883) Round 5 of #883, a refresh. Each line comes from its squash body and was checked against the code at main 0d5d2b1: - #815 (#764): a project's bootstrap ends a validator's exit 2 as `bootstrap REFUSED`, exit 2, takes an upstream path, cache or pin it cannot read as no answer, and leaves a reftable leg's cherry-pick or revert alone. A line in "What changed", one in "What 1.1.1 fixes", and a note under "What a script sees" on the direct run's exit code. Its body's leftovers, two tracebacks on Python 3.12 and 3.13 and two "failed" texts beneath a REFUSED line, are a new limit. - #770 (#755) and #869 (#853, #854): tests, in the Tests line. The test limit for #853 and #854, and the skip #845 left, which #869 removes, are retired. - #802 (#675): update-shape.py's `git show` lines are shell-quoted and its kept-copy readers read some Python as Python does. Its one-hop read and what it reads as running are a new limit. 67 pull requests; 69 of the milestone's 72 issues closed at 11:31Z, and the open list is the 3 that read leaves: #707, #879 and #884. The shape-copy lists stand: the assembly root's bootstrap.py, which #815 changes, is on them. No other file changes. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…g keeps its rows, #802's readers what v1.1.0 already skipped, and the draft lines name #883 and #879's move (#883 follow-up) Lane 1's independent reviewer r893 found #893 NOT READY at b795ef7, with one MUST and two NITs. Each was checked against v1.1.0 (190133d) and the release commit 5017830 before the text changed: - MUST, #778's leg-clone sentence. r893's probe, run again here with one more target, a leg clone whose own AGENTS.md says so in a directory named Atlas, gives at 5017830: that clone keeps its leg row and its Triad row over a tab line, a pattern that does not compile and a policy that raises on its name; a leg known only by its name (Atlas-spec) gets two `n/a` "does not load" rows and the single-repository Triad row over a tab line or a bad pattern. At v1.1.0 the AGENTS.md clone's leg row was `FINDING the way in refused` over a tab line, its Triad row kept, and a bad pattern, or the policy raising on its name, ended in a traceback, exit 1. The sentence now says that. - NIT, #802's readers. At v1.1.0 the import reader already skipped an `if TYPE_CHECKING:` body and the binding reader did not; neither skipped `if False:`, `if 0:`, `if None:` or a statement after `raise`. At 5017830 both skip all of them. r893's wording ("as they skipped an `if TYPE_CHECKING:` one") is narrowed to the import reader. - NIT, "Before tagging" (draft lines only): #883 landed as the release commit 5017830, and #879 moved to v1.2.0 at 2026-10-18T14:10:11Z, after 2d372bb landed at 13:19:52Z. No other file changes. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

… the release commit, #778's leg-clone sentence and #802's readers sentence, each checked against runs at v1.1.0 and main (#883 follow-up) (#893) The v1.1.1 notes landed as the release commit `5017830` (#883), and RV883's round 5 held two SHOULDs and five NITs for after it (#883 issuecomment-6250558821). This applies them to `docs/release-notes/v1.1.1.md`, with two more sentences lane 1's review found: 1. **#778's leg-clone sentence** says what runs at `v1.1.0` and `5017830` show. A leg clone whose own `AGENTS.md` says so keeps its leg row and its Triad row over each fault, where v1.1.0 gave `FINDING the way in refused` over a tab line and a traceback, exit 1, over a pattern that does not compile or a policy that raised on its name. A leg known only by its name gets a repository's two `n/a` rows. 2. **"Is", not "stays", exit 2,** in the pattern case, which was exit 1 in v1.1.0. 3. **#802's clause is dropped:** v1.1.0's `_blocks_inside` already read a `try`'s handlers, `else` and `finally` outside its guard. 4. **"What 1.1.1 fixes"** names "or cannot be used", and `the other way in`. 5. **#815's leftover tracebacks** were measured on Python 3.12; 3.13 is read, not run. 6. **#871 lets `_` through for any owner,** an organisation's name included. 7. **#869 asks the disk** only about a name that is not UTF-8. 8. **#802's readers:** at v1.1.0 only the import reader skipped an `if TYPE_CHECKING:` body; the binding reader skips one now. 9. **A known limit names #940:** under the `C` or POSIX locale, the bash `openRepoShape` and `setup.sh`'s guards refuse a URL, path or ref with a letter past ASCII as "a control character", a regression from v1.1.0 in the launcher through #872 (#866); a UTF-8 locale takes such a URL or path. The draft-time lines in the sections marked "remove at tag time" say main `2d372bb`, where #778 landed file for file as its reviewed head `8259ea1`, and that #879 moved to v1.2.0. The release body never carries those sections. Review: lane 1's independent reviewer found `b795ef74` NOT READY, on the leg-clone sentence, and `ca49f125` READY, with each changed sentence checked against runs at `v1.1.0` and `5017830`. Lane 1's second reviewer found `6cdb5551`, which adds the #940 limit, READY, with each clause measured against the launcher at `64464d31` and at v1.1.0. It lands by fast refresh (Brett Heap, 2026-10-15T22:48:35Z). Its reviewed head `6cdb5551` is green on Linux and SonarCloud. The branch merged main once and was never rebased; the last merge is update-branch's, clean, giving f60654e, which is green on SonarCloud and Git Bash. Lane 1's gate passed on its tree: the 5 test files that read the release notes or the workflows (`test_repo_hygiene.py`, `test_release_workflow.py`, `test_workflow_concurrency.py`, `test_agents_md_codes.py` and `test_cli_reference.py`), 221 passed and 0 skipped. Lane: openreposhape-1 (openRepoShape-1) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

brettheap mentioned this pull request Oct 17, 2026

Tracking: update-shape.py and bump-leg.py, keeping a project's shape copies and leg pins current #458

Open

12 tasks done

Merge branch 'main' into l3-675-update-shape-quoting

1f8c688

brettheap marked this pull request as ready for review October 18, 2026 11:30

brettheap merged commit 0d5d2b1 into main Oct 18, 2026
5 checks passed

brettheap deleted the l3-675-update-shape-quoting branch October 18, 2026 11:30

Navigation Menu

Repository navigation

brettheap commented Oct 17, 2026 •

edited by sourcery-ai Bot

Loading

Uh oh!

sourcery-ai Bot commented Oct 17, 2026

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

brettheap commented Oct 17, 2026

Uh oh!

brettheap commented Oct 17, 2026

Uh oh!

sonarqubecloud Bot commented Oct 18, 2026

Uh oh!

sourcery-ai Bot commented Oct 18, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

brettheap commented Oct 17, 2026 • edited by sourcery-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Round 2

What changed, and why

Where this reads the issue differently (accepted in review)

Tests

Pinned copies

Overlap

Summary by Sourcery

Uh oh!

sourcery-ai Bot commented Oct 17, 2026

Reviewer's Guide

Sequence diagram for validator-aware missing-name refusal

Sequence diagram for shell-safe git show remedies

Flow diagram for unreachable AST reader handling

File-Level Changes

Assessment against linked issues

Possibly linked issues

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

brettheap commented Oct 17, 2026

Uh oh!

brettheap commented Oct 17, 2026

Uh oh!

sonarqubecloud Bot commented Oct 18, 2026

Quality Gate passed

Uh oh!

sourcery-ai Bot commented Oct 18, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

brettheap commented Oct 17, 2026 •

edited by sourcery-ai Bot

Loading