Skip to content

adopt-project.py, shape-doctor.py: a plan's answer against a rule is recorded and an empty .gitmodules asks nothing, so docs/ answered code is not MISPLACED after adoption verified, nor a .gitmodules emptied by git rm asked on every run (#655) - #721

Merged
brettheap merged 3 commits into
mainfrom
fix/655-adopt-doctor-plan-overrides
Oct 16, 2026

Conversation

@brettheap

@brettheap brettheap commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Lane: openreposhape-1 (openRepoShape-1)
Claim: #655 (comment)

What

This PR answers #655: two P2 lines of #457, L1-56 and L1-57, and the two riders on #617's answered_paths: rows, L1-58 and L1-59.

  • L1-57: execute records a plan's answer against a rule. This is decision (A) of lane 1's triage, which Brett Heap approved on 2026-10-16T11:21:17Z.
  • L1-56: the doctor stops asking about an empty .gitmodules.
    • A .gitmodules that holds no byte registers no submodule. The question ambiguous-gitmodules asks of it ("which leg owns the submodules it registers") has no subject, so no answer could be wrong.
    • The doctor no longer asks it. The leg's entry names the file under empty_gitmodules, and the row says the policy asks nothing of 1 `.gitmodules` that hold(s) no byte and so register(s) no submodule (#655): code/.gitmodules.
    • A .gitmodules holding anything, even one blank line or a comment, stays the question it was.
  • L1-58: a directory's row without its / is said.
    • The row still answers nothing under the directory: _covering reads it as a file's row, as check reads a plan entry.
    • Where the leg the row names holds a directory at that path, the leg's entry names it under unslashed, and the row says 1 row(s) of project.yaml's `answered_paths:` name a directory without the `/` a directory's row ends with, so each answers nothing under it (#655): spec/examples.
    • It is never a finding, and it never keeps the row off ok.
  • L1-59: what a directory's row answers is said where the row is read. The comment execute writes above the block now says: "A directory's row answers for every file under it, one added after the adoption too: the doctor does not hold a file to source_commit:." The docstring of the project's validate-manifest.py says the same, ending "nothing holds a file to source_commit."
  • AGENTS.md. The one line on the record now says answered_paths: holds a row "per other path the policy asked about or put elsewhere". The file stays at 492 of 492 lines.

Why (repro on main a524542; the same on 35133f2)

The source holds README.md, src/app/util.py, docs/index.md, a golden run under examples/ and a submodule at vendor/rules. The plan answered examples/ as spec, vendor/ and .gitmodules as code, and docs/ (rule spec-governance, spec) as code, each with a resolution:. Every probe ran with a stub gh first on PATH, GH_TOKEN set to a bogus value, an empty GH_CONFIG_DIR, GIT_CONFIG_GLOBAL=/dev/null and GIT_ALLOW_PROTOCOL=file.

  • L1-57. check exited 0, and execute exited 0 with adoption verified. answered_paths: held examples/ only. The doctor's first run exited 1 with MISPLACED (1 path): code/docs/: spec by rule spec-governance.
  • L1-56. git rm vendor/rules in the code leg left code/.gitmodules at 0 bytes, tracked. Three runs of the doctor in a row each listed code/.gitmodules under review_required. A hand-written answered_paths: row for .gitmodules changed nothing.
  • L1-58. With the examples/ row edited to examples, validate-manifest.py said manifest ok. The doctor asked spec/examples/ again and said nothing about the row.
  • L1-59. A file staged under spec/examples/later/ was answered by the examples/ row, by design. The block's comment said only that each row answers "the question it would ask again of that path in that leg".

On this branch, the same script gives:

  • L1-57: the doctor's first run exits 0, COMPLIANT, with placement ok, and both spec/examples/ and code/docs/ under answered_paths;
  • L1-56: review_required is empty on every run, and the clause above is said;
  • L1-58: the question is still asked and the unslashed clause is said;
  • L1-59: the comment above says what a directory's row answers.

How

  • adopt-project.py.
    • answered_path_record runs the policy's walk over the source's paths as before, gitlinks left out. It keeps each entry _answered_legs reads as answered spec or code (not .gitmodules) that covers a path whose walked leg differs from that answer. The walked leg is read by _walked_leg, from the walk entry that covers the path. An answer is recorded against a rule only where it also differs from the leg plan showed, read from the same walk with the gitlinks in and ask_about_submodules and ask_about_refused_paths applied, so an entry left as plan wrote it is never recorded.
    • The new _covering_names(path) lists every name _covering can match for a path: each directory above it, with its /, and the path itself.
    • ANSWER_RECORD_PREAMBLE, the module docstring and two comments in execute say what the record now holds.
  • shape-doctor.py.
    • answered_questions is called twice in audit_leg: once for the questions, as adopt-project.py, shape-doctor.py, validate-manifest.py: execute records the plan's answers under answered_paths: and a moved submodule names its row, so an answered examples/ is not asked again and a deliberate move names no plan (#525) #617 did, and once for the walked paths in the wrong leg.
    • Its rule is unchanged. A path is answered when every file under it is covered by exactly one row, and that row names this leg.
    • It now looks each file up by _covering_names rather than holding every path against every file. That is the same answer, without the quadratic cost (see Tests).
    • answers_clause names the two kinds of answer apart.
    • empty_gitmodules takes a .gitmodules that is a regular file of 0 bytes on disk (lstat, as leg_tracked_files reads a size) out of the questions. git refuses to index a symlinked .gitmodules ("invalid path"), and where the disk has none git reads the index's copy, so neither counts as empty.
    • unslashed_rows names the rows for this leg whose path is a directory of the leg's classified files.
    • Two keys on a leg's entry, empty_gitmodules and unslashed, are present only when non-empty, so every other report is unchanged.
    • The module docstring has two new paragraphs.
  • templates/assembly-root/scripts/validate-manifest.py: docstring only.

Why L1-56 stops asking instead of letting a record answer

Why L1-58 is said and not read as the directory's row

  • One spelling means one thing. check reads examples as a file's entry, and calls it plan-empty-entry beside a plan-uncovered for each file under it.
  • A lenient reading would let examples and examples/a.json both pass the validator, which sees no overlap between them, and then both cover a.json in the doctor.
  • The validator cannot say it, because it runs where the legs are not checked out. The doctor reads the leg, so it says it.

Tests

revert fails
R0: main's source (all four non-test files) 13 (12 new and #617's changed case)
R1: execute records the policy's questions only (#617's filter) 7
R2: the doctor reads no answer for a path in the wrong leg 3
R3: a row for either leg answers 3 (1 new, 2 of #617's)
R4: any one answered file answers the path 3 (1 new, 2 of #617's)
R5: an empty .gitmodules is still asked 2
R6: "empty" means whitespace only 1
R7: any .gitmodules asks nothing 3 (2 new, 1 of #617's)
R8: the empty .gitmodules is not said 1
R9: a slashless directory row is not said 2
R10: every slashless row is named, directory or not 2
R11: a slashless row is named in either leg 1
R12: the block's comment as on main 2
R13: the validator template as on main 1
R14: AGENTS.md as on main 1
R15: an answer against a rule worded as a question's 1
R16: answered_questions' body as #617 wrote it, both calls kept 0 (behaviour-preserving; see the probe)
R17: execute records every entry kept in a leg 16
R18: execute records no answer against a root rule 1
R19: an answer judged against the gitlink-free walk alone (r655's F1) 1

Every one of the 22 new tests fails under at least one of R1 to R19.

  • The cost probe. Main's answered_questions body was compared with this branch's on synthetic legs, at a load average of about 50. Half the paths were questions and half were in the wrong leg, and every other one was answered.
files / paths main, questions only main's body, both calls this branch, both calls
5,000 / 200 0.22 s 0.54 s 0.02 s
20,000 / 1,000 4.55 s 9.72 s 0.07 s
60,000 / 3,000 38.50 s 74.56 s 0.37 s

The answers were identical in every row.

  • answered_path_record now reads every file of the source through _answered_legs, not only the questions. On the same probe it took 0.97 s, 14.57 s and 123.96 s for 10,000, 40,000 and 120,000 files with 400, 2,000 and 6,000 entries. Main took 0.47 s, 7.89 s and 68.19 s.
  • At each of those sizes, _coverage_findings, which check and execute already run over the same paths, took longer: 1.31 s, 25.71 s and 212.70 s.
  • Both modes at the head 0e37d02 (dirty 0), with a stub gh first on PATH, a bogus GH_TOKEN and an empty GH_CONFIG_DIR. The files: tests/test_doctor_plan_overrides.py, tests/test_doctor_recorded_answers.py, tests/test_manifest_validator.py and tests/test_agents_md_codes.py as one group, and tests/test_shape_doctor.py as another.
group git-filter-repo on PATH git-filter-repo hidden
the four files 198 passed, EXIT=0 192 passed, 6 skipped, EXIT=0
tests/test_shape_doctor.py 405 passed, 3 skipped, EXIT=0 401 passed, 7 skipped, EXIT=0

The rest is covered by CI's full Linux suite on this PR and by the earlier runs at fc00656, which differs from 0e37d02 only in answered_path_record and the new test. Those runs covered 137 test files in six groups per mode: every file matching adopt-project|adopt_project|\bADOPT\b|shape-doctor|shape_doctor|bump-leg|bump_leg|validate-manifest|validate_manifest, plus tests/test_shape_doctor.py, tests/test_scaffold_leg_path_collisions.py, tests/test_repo_hygiene.py, tests/test_agents_md_codes.py, tests/test_cli_reference.py and the three files above. With git-filter-repo on PATH, 8,842 passed and 17 skipped; with it hidden, 8,496 passed and 363 skipped. All twelve groups exited 0.

Windows

CI is Linux only since #597, so the Windows newline emulation was not run. The new tests write .gitmodules with write_text (an empty string stays 0 bytes on any platform) and read every file a helper wrote back with read_text(); the one read_bytes() compare is of the empty .gitmodules that git rm itself leaves. Every path they create is legal on NTFS.

Not in this PR

  • adopt-project.py plan and check are unchanged. An answer against a rule still passes check with no finding: that was option (B), which the triage did not take.
  • _answered_legs is unchanged (see Known limits).
  • README.md and docs/handbook.html are not touched.

Known limits

This PR answers four lines of #457, L1-56, L1-57, L1-58 and L1-59, and #457 itself stays open.

Closes #655

🤖 Generated with Claude Code

Summary by Sourcery

Preserve intentional adoption-plan overrides in recorded answers and make the shape doctor accurately handle empty .gitmodules files and directory answer rows.

New Features:

  • Record adoption plan answers that place paths against their policy rules so the doctor treats those paths as intentional rather than misplaced.
  • Report empty .gitmodules files, slashless directory answer rows, and directory-row coverage explicitly in doctor output.

Bug Fixes:

  • Prevent empty tracked .gitmodules files left after submodule removal from being repeatedly reported as unresolved questions.
  • Prevent paths intentionally answered into another leg from being reported as misplaced during shape audits.

Enhancements:

  • Optimize recorded-answer coverage checks by looking up only path names that can cover each file.
  • Clarify manifest, validator, project guidance, and generated record comments for rule overrides and directory answer semantics.

Documentation:

  • Update user-facing manifest guidance to document answers against rules, directory coverage, and slashless directory rows.

Tests:

  • Add comprehensive coverage for rule overrides, empty .gitmodules, malformed directory rows, later files under answered directories, and end-to-end adoption behavior.

…against a rule under `answered_paths:` and an empty `.gitmodules` asks nothing, so `docs/` answered `code` is no longer MISPLACED on the doctor's first run after `adoption verified`, and a `.gitmodules` emptied by `git rm` is no longer asked on every run (#655)

L1-57: `execute` now writes an `answered_paths:` row for each entry the
plan keeps in a leg that covers a path the policy's walk does not put in
that leg: a question, as #617 recorded, or a path a rule calls for the
other leg or the root (decision A of lane 1's triage, approved
2026-10-16T11:21:17Z). The doctor holds a walked path in the wrong leg to
the record as it holds a question, file by file and only in the leg the
row names, lists it under `answered_paths` with the rule it answered
against, and says so in its own clause.

L1-56: a `.gitmodules` that holds no byte registers no submodule, so the
doctor no longer asks `ambiguous-gitmodules` of it; the leg's entry names
it under `empty_gitmodules` and the row says so. A file holding anything
stays the question it was.

L1-58: a row naming a directory of its leg without its `/` answers nothing
under it, as before, and the doctor now says so (the leg's `unslashed`
entry and a clause), never as a finding.

L1-59: the block `execute` writes and the validator template's docstring
say that a directory's row answers files added under it after the
adoption too.

`answered_questions` now looks each file up by the names `_covering` can
match (`_covering_names`), so calling it for both lists costs a leg's
files times their depth. AGENTS.md's record line names the new rows and
stays at 492 lines.

Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Oct 16, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR expands answered_paths from policy questions to all plan decisions that place a path outside its policy leg, teaches the doctor to honor those records and ignore truly empty .gitmodules files, adds diagnostics and documentation for malformed directory rows and directory coverage, and supplies broad regression and end-to-end tests.

Sequence diagram for recorded plan overrides during adoption and audit

sequenceDiagram
    participant Execute as adopt-project.py execute
    participant Policy as PathPolicy.walk
    participant Record as project.yaml answered_paths
    participant Doctor as shape-doctor.py

    Execute->>Policy: walk(files)
    Policy-->>Execute: called path legs
    Execute->>Execute: answered_path_record
    Execute->>Record: write paths answered into a different leg
    Doctor->>Policy: walk(leg files)
    Policy-->>Doctor: questions and misplaced paths
    Doctor->>Record: answered_questions
    Record-->>Doctor: recorded leg and classified_as
    Doctor-->>Doctor: settle recorded overrides
    Doctor-->>Execute: COMPLIANT when no findings remain
Loading

Flow diagram for empty gitmodules and recorded path handling

flowchart TD
    Start[Doctor audits leg] --> Gitmodules{.gitmodules is a regular zero-byte file?}
    Gitmodules -->|Yes| Ignore[Remove from questions]
    Gitmodules -->|No| Questions[Keep normal submodule question]
    Ignore --> Paths[Evaluate policy questions and misplaced paths]
    Questions --> Paths
    Paths --> Record{answered_paths row covers every file?}
    Record -->|Yes, row names this leg| Settled[Report answered path]
    Record -->|No| Finding[Keep question or MISPLACED finding]
    Settled --> Diagnostics[Emit answer clauses]
    Finding --> Diagnostics
    Diagnostics --> Report[Include empty_gitmodules, answered_paths, or unslashed details when applicable]
Loading

File-Level Changes

Change Details Files
Record and honor plan decisions that place paths contrary to policy rules.
  • Extend adoption records to include entries kept in a leg different from the policy walk, while excluding rule-consistent, root, drop, and .gitmodules cases.
  • Have the doctor apply recorded answers only within the named leg and file-by-file, preserving the original rule classification and resolution in reporting.
  • Add coverage-based lookup for recorded answers to avoid the previous path-by-file quadratic scan.
adopt-project.py
shape-doctor.py
tests/test_doctor_recorded_answers.py
tests/test_doctor_plan_overrides.py
Treat empty .gitmodules files as having no submodule question.
  • Ignore regular zero-byte .gitmodules files when collecting review questions, while continuing to ask about blank-line, comment, symlink, and non-regular cases.
  • Report omitted empty files explicitly in the leg detail and doctor reason.
shape-doctor.py
tests/test_doctor_plan_overrides.py
Diagnose malformed or broad recorded-path entries without changing their effective coverage semantics.
  • Report slashless rows that name directories as non-answering entries without making them findings.
  • Document that directory rows cover files added after adoption and do not constrain source_commit.
  • Add corresponding manifest comments and validator documentation.
shape-doctor.py
adopt-project.py
templates/assembly-root/scripts/validate-manifest.py
tests/test_doctor_plan_overrides.py
Update project guidance and regression coverage for the expanded record behavior.
  • Clarify answered_paths semantics in AGENTS.md.
  • Add unit and end-to-end tests covering rule overrides, empty .gitmodules, slashless rows, later-added files, and documentation.
AGENTS.md
tests/test_doctor_plan_overrides.py
tests/test_doctor_recorded_answers.py

Assessment against linked issues

Issue Objective Addressed Explanation
#655 Record plan answers that place a path in a leg different from the policy’s rule, and have the doctor honor that record so the path is not reported as misplaced. ✅
#655 Stop the doctor from repeatedly asking about a tracked, zero-byte .gitmodules file left after removing a leg’s only submodule. ✅
#655 Make answered_paths: directory-row behavior clear: identify slashless rows that name an existing directory but answer nothing beneath it, and explain that a directory row also answers files added later. ✅

Possibly linked issues


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

…also differs from the leg `plan` showed, so an entry left as `plan` wrote it, beside a gitlink that kept `plan` from folding its directory, no longer gets an `answered_paths:` row nobody gave (#655)

r655's F1. `plan` walks the tree with its gitlinks, and a gitlink is one
more verdict in its directory's extension majority, so it can keep `plan`
from folding a directory the gitlink-free walk folds. `plan` then proposed
`engine/notes.md` as `spec` by its own extension, and `execute` recorded
that untouched entry as an answer against the folded `code`: a plan with
no override wrote a different `project.yaml` than main.

`answered_path_record` now also reads the leg `plan` showed (the walk with
gitlinks, asked again by `ask_about_submodules` and
`ask_about_refused_paths`), and records an answer against a rule only
where it differs from both. A question of the gitlink-free walk is
recorded as before. The fix and its test are r655's, verified in its
clone (3dfa567e); the test joins tests/test_doctor_plan_overrides.py.

Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Oct 16, 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 4 hours and 25 minutes by commenting @sourcery-ai review. Upgrade to get a review now.

@sonarqubecloud

Copy link
Copy Markdown

@brettheap
brettheap merged commit 80ecb3a into main Oct 16, 2026
5 checks passed
@brettheap
brettheap deleted the fix/655-adopt-doctor-plan-overrides branch October 16, 2026 19:25
brettheap pushed a commit that referenced this pull request Oct 17, 2026
main gained 25 pull requests since de92614: #634, #699, #671, #719,
#701, #717, #700, #702, #722, #720, #725, #721, #728, #705, #727, #718,
#744, #743, #749, #740, #737, #734, #742, #751 and #736. No conflict:
AGENTS.md, adopt-project.py and docs/cli.md merged as text, and
README.md and docs/handbook.html are unchanged by the merge. README.md
is 1473 lines, its cap; AGENTS.md 494, its cap; every handbook marker is
current; `render-cli-reference.py --check` is ok.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap pushed a commit that referenced this pull request Oct 17, 2026
…es every reason THIS checkout cannot answer, and the notes are current to main bf1f099 with the 25 pull requests since de92614 (#563, #683)

- W683 and RV-f's N4 on #742: README's exit-3 list (Keeping a project's
  shape current, :1208-1220) and its handbook twin (`doctor`) named only a
  pin whose commit this checkout does not carry. They now say, as
  shape-doctor.py's EXIT CODES does: a pin's commit it lacks, a partial
  clone missing what is compared (#683), a read that fails
  (`update-upstream-unreadable`), a crashed validator of its own (#278,
  #410), or its own naming policy not loading (#678). +1 line, with
  tests/readme-cap.d/683.txt; README.md is 1474, its cap (1422 + 2 + 5 +
  12 + 22 + 10 + 1). The two `Keeping a project's shape current` markers
  move: `doctor` text and marker, `current` marker only.
- The notes, "What changed since v1.0.1", one entry per pull request from
  `git log --first-parent de92614..bf1f099`, each from its squash commit and
  the issues GitHub says it closes:
  - adopt-project.py: #699, #719, #749, #727, #717, #728 (with #621's
    hint), #743 + #720 + #718, #722, #721;
  - update-shape.py, bump-leg.py and a project's own scripts: #702 (with
    the pinned validators), #634;
  - scaffold, setup, family and the shared readers: #725, #671, #744;
  - shape-doctor.py: #742, #701, #737, #751, #736, #734, #700 + #705,
    #740;
  - install, CI and the docs: the exit-3 sentence above.
- Counts (99 pull requests since bfed143; 109 of 135 milestone issues
  closed at 10:28Z), the "current to main" sha and the open list (26) are
  at main bf1f099, still placeholders for tag time.

Left for round B: lane 5's eight lines (#630 issuecomment-6227129879),
whose pull requests #726, #747, #682, #724, #746, #745 and #738 are open
drafts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Oct 17, 2026
… verification row's colon, `run_validator`'s "costs no child an import" and the decoy test's `sitecustomize` claim say what main does (#666) (#730)

Four comments and docstrings said something main no longer does. They now say what the code beneath them does. A fix round after review added two more of the same kind (see "Fix round"). Comments and docstrings only: no behaviour, no `--help` text, no pinned copy, so no README, handbook or `docs/cli.md` change.

- **`adopt-project.py`, the module docstring.** It said the verification table reads `code: 0 of N source paths (seeded from template)`. The printed row has no colon (`  {leg:<6} {count:>5} of {N} source paths{note}`). It now says the table's `code` row reads `0 of N source paths (seeded from template)`, the words `AGENTS.md` and `README.md` use since #505 and the docstring of the verification function uses too. `git grep 'code: 0 of'` finds nothing now.
- **`shape-doctor.py`, `submodule_config`.** The last sentence of the docstring gave a reason that is true of the string and not decisive: "It is also what the row PRINTS, and a lone surrogate is a string no UTF-8 stream can write." The table goes through `ascii_text` and `--json` through `json.dumps`, so neither would raise on a lone surrogate. I dropped the sentence, the issue's first direction. The comparison with `origin_url`'s reading of a clone's remote is the reason, and the paragraph already gives it first. **This hunk carries the bytes of lane 3's #700**, which landed as `55c6ba7d`: the two removed lines and the one added line (`    working clone.`) are the same. So the hunk is on main already and drops out of the squash (see "Evidence").
- **`shape-doctor.py`, `run_validator`.** "It costs no child an import" holds for an unedited copy only. A new paragraph says a root whose own copy of a validator gained an `import yaml` read `passes` while `site` was on, and now reads `crashed before reaching a verdict (exit 1): ModuleNotFoundError: No module named 'yaml'`, the crash its own CI gets, because `validate.yml` installs nothing. It calls that row a fair answer and says `-S` does not ask whether a copy was edited. No code change.
- **`tests/test_shape_doctor.py`, the decoy test's two comments** (the block above `DECOY_REPO_SHAPE` and the inline comment before `reached`). `run_validator` starts every child as `[sys.executable, "-S", script, ...]`, so no child imports a `sitecustomize`. The comments now say that, say that replace-not-prepend and the #306 review S1 case are held by `test_run_validator_gives_the_child_exactly_the_pythonpath_it_is_given`, and say that the decoy test holds the template row's real verdict under an exported `PYTHONPATH`.

The review of the first two commits approved with nits. Two new commits take S1 and N1: `7f2bb36b` makes both edits, and `426877bf` rewrites the last sentence of the N1 comment, which did not parse ("...a future removal of `-S`, which `test_run_validator_starts_every_child_with_no_site` ... is the test that holds"). It now ends "...a future removal of `-S`; `test_run_validator_starts_every_child_with_no_site` (in `tests/test_doctor_validator_site.py`) is the test that holds `-S`." Both are comments and docstrings only.

- **S1, a third stale sentence.** The docstring of `test_run_validator_gives_the_child_exactly_the_pythonpath_it_is_given` said that for the callers which pass no `pythonpath`, "only a probe like this one, or (b)'s decoy, can see that they have none". That is false under `-S`: a `run_validator` that keeps the variable for those callers passes the decoy test in all three cases and fails the probe test twice (the second row of the table below). It now says only a probe like this one can see that they have none, and that the decoy of (b) could before `-S` (#342). It is the same defect as the issue's, in a place the issue did not name. "Could" holds for the `manifest` and `pins` cases: with `-S` removed, 2 of the 3 decoy cases fail, and the `family` case never caught it.
- **N1, the inline comment overstated the loop.** The comment above `reached` said "What this loop still holds is that no row quotes the decoy or says it exited 86." Under `-S` the loop cannot fire. It now says so, says the loop is kept as a tripwire for a future removal of `-S`, and names `test_run_validator_starts_every_child_with_no_site` in `tests/test_doctor_validator_site.py` as the test that holds `-S`. That test is on main.
- **N2, this body.** The merge checks and the notes on #630 were stale (they cited `de926142` and a conflict that is gone). They are rerun and rewritten below.

**Only comments and docstrings changed.** `git diff 8062810 426877b` has 74 changed lines (44 added, 30 removed) in three files. A script checked each changed line against the file's comment tokens and docstring spans (both sides of the diff): 0 outside. A second check parsed each file at the base and at the head, blanked every docstring, and compared the ASTs: identical in all three files. For the two fix-round commits together, the AST of `tests/test_shape_doctor.py` with docstrings blanked is identical to the one at `a2113cff`, `git diff --check` is clean, and the longest added line is 75 columns (73 in the second commit).

**What the squash holds.** `git merge-tree --write-tree` of this head with origin/main `20c8873b` gives the tree `6ead2ffc`. `git diff 20c8873 6ead2ffc` is 71 changed lines in three files, and `shape-doctor.py` has 8 of them: the new `run_validator` paragraph and nothing else. `git diff` of the two trees has no hunk that mentions `submodule_config`, `surrogate`, `PRINTS` or `working clone`. On main, `submodule_config`'s docstring has no "PRINTS" sentence and ends in `working clone.`. The same two checks on that tree: 0 lines outside comments and docstrings, ASTs identical. On main `2d988200` with the head `426877bf` the squash is the same: 71 changed lines, 8 of them in `shape-doctor.py`, 0 outside comments and docstrings, ASTs identical, and no hunk that mentions `submodule_config`.

**`python3 scripts/render-cli-reference.py --check`**, at `426877bf`: `[ok] docs/cli.md is what the generator produces`. Only the first line of a module docstring is help text, and none of the first lines changed.

**The decoy comments, measured.** I did not take the issue's reading on trust. In a throwaway clone at the base I put each #306 regression into `run_validator` and ran the two tests (`test_an_exported_pythonpath_does_not_reach_a_template_validator`, three cases, and `test_run_validator_gives_the_child_exactly_the_pythonpath_it_is_given`, six):

| mutant | with `-S` (main) | with `-S` removed |
|---|---|---|
| the standard's `scripts/` put in FRONT of the operator's value | decoy test: 3 of 3 pass. Probe test: 1 of 6 fails | decoy test: 3 of 3 fail |
| the variable kept for the callers that pass none (S1) | decoy test: 3 of 3 pass. Probe test: 2 of 6 fail | decoy test: 2 of 3 fail |
| the operator's directory AHEAD of the standard's | decoy test: 3 of 3 fail. Probe test: 1 of 6 fails | not run |

So on main the decoy test catches neither #306 regression and the probe test catches both, as lane 4's R342 measured on #636's head. The third row is why the comment says the raising `repo_shape.py` "still bites": it kills an operator-first order. The review re-ran these mutants on `a2113cff` and got the same counts.

**The `run_validator` paragraph, measured.** A root whose own `validate-manifest.py` is `import yaml` plus a print, run through `run_validator` from this tree: exit 1 and `ModuleNotFoundError: No module named 'yaml'`. The same call with `-S` taken out of the child's argv: exit 0, `manifest ok`. `yaml` is importable from the machine's site-packages and not without `site`.

With the real `gh` unreachable (a stub first on PATH, a bogus token, an empty `GH_CONFIG_DIR`).

On `a2113cff`, before the fix round:

- `tests/test_cli_reference.py`, `tests/test_repo_hygiene.py`, `tests/test_agents_md_codes.py`, `tests/test_scaffold_leg_path_collisions.py`: 250 passed (57.5 s).
- `tests/test_doctor_gitmodules_not_utf8.py`, `tests/test_doctor_validator_rows.py`, `tests/test_doctor_validator_site.py`, `tests/test_doctor_validator_verdicts.py`, `tests/test_adopt_spec_only.py`, `tests/test_adopt_entry_paths.py`, `tests/test_adopt_submodules.py` (the files that drive `run_validator`, `submodule_config` or the seeded-leg row): 334 passed (28 min 18 s).
- `tests/test_shape_doctor.py`, in two halves run side by side (the first 204 test ids, then the last 204): 204 passed in 11 min 31 s, and 201 passed, 3 skipped in 4 min 58 s. That is all 408 tests. The 3 skips are the PowerShell-line tests, which need `powershell.exe` on a Windows host.
- The review ran all of `tests/test_shape_doctor.py` there: 405 passed, 3 skipped (512 s). On the predicted merge with main `2fac9790` it ran `test_shape_doctor.py`, `test_doctor_validator_site.py`, `test_doctor_validator_rows.py`, `test_doctor_validator_verdicts.py`, `test_doctor_gitmodules_not_utf8.py`, `test_repo_hygiene.py`, `test_cli_reference.py` and `test_agents_md_codes.py`: 632 passed, 3 skipped (760 s).

On `426877bf`, after the fix round, which changes comments and docstrings only, so the runs above stand:

- The two tests the fix round touches, `test_an_exported_pythonpath_does_not_reach_a_template_validator` (3 cases) and `test_run_validator_gives_the_child_exactly_the_pythonpath_it_is_given` (6 cases), run by node id: 9 passed (11.7 s). The same 9 also passed (16.1 s) at `7f2bb36b`.
- `tests/test_repo_hygiene.py`: 77 passed (1.9 s). It also passed (2.0 s) at `7f2bb36b`.

No test fails on the base, because nothing here changes behaviour. The AST comparison above is the proof in their place.

`git merge-tree --write-tree` of this head against origin/main and against every ref on origin that could meet it. The full run was on 2026-10-16, with the head `7f2bb36b` (the one before the last comment commit) and main `20c8873b`: the 41 branches (`fix/*`, `l3-*`, `wip/*`, `docs/*` and `release/v1.0`) and the 55 PR heads from #630 up. That is 96 refs. I ran #731's head `ab55c29f` through the same list. After `426877bf` I re-ran main, now `2d988200`, and the six heads that edit `tests/test_shape_doctor.py`, the one file that commit touches (#705, #737, #740, `l3-669-members-row`, `l3-680-doctor-signals` and `l3-697-crash-cure-next`), and #630: all clean.

- **origin/main: clean.** At `20c8873b`, with the head `7f2bb36b`, the predicted tree is `6ead2ffc`. At `2d988200` (three `adopt-project.py` commits later, with their tests and a change in `scripts/shape_materialize.py`), with the head `426877bf`, it is `7ee5caf4`, and the squash is the same three files (43 added, 28 removed). #731's head `ab55c29f` is clean with `2d988200` too. This head with #731's head: clean.
- **#630's head `6fd33a93`: clean.** It conflicted with main in `docs/handbook.html` when this body was first drafted, and no longer does, with or without this change.
- **#700 and #702 have landed** (`55c6ba7d` and `20c8873b`), and so has #701, and #699, #717 and #719 for `adopt-project.py`. Their heads still merge clean with this head.
- **Every ref that edits `shape-doctor.py`, `tests/test_shape_doctor.py` or `adopt-project.py` merges clean with this head**, except the two old heads named in the bullet after next. They include the heads of PRs not on main yet (#705, #721, #737 and #742, and lane 3's `l3-680`, `l3-683`, `l3-687`, `l3-691` and `l3-697`, which were not on origin when this body was first drafted). #673 has no branch or PR head on origin.
- **Four refs conflict with origin/main on their own**, and so with this head: `docs/release-notes-v1.0.0` and `release/v1.0`, and the pre-squash heads of the landed #637 and #641. Their conflicts do not come from this change.
- **Two landed pre-squash heads conflict with this head only because they are old.** #632 conflicts with main on its own. #636 merges clean with main, but its old head conflicts with this head in `run_validator`'s docstring, next to the text #636 added; main has that text, and this head merges clean with main.
- **Refs that conflict with main on their own are not made worse.** #682, #745, #747, `fix/648`, `fix/681` and `fix/704` conflict with main in `templates/family-root/scripts/validate-family.py`; #742 and `l3-683` in `shape-doctor.py`. Each merges clean with this head.
- **Every other open head and branch: clean.**

Nothing from the issue. Its four lines are done. The decoy test's `sitecustomize` machinery is dead code under `-S` and I left it, because the issue says comment only. The comment above `reached` now says it is kept as a tripwire.

- **The decoy test's `sitecustomize.py` and exit-86 trap never run.** `DECOY_SITECUSTOMIZE`, `os._exit(86)` and the `(exit 86)` and `DECOY` clauses in `reached` can only fire for a child started without `-S`. They are kept as a tripwire and the comment says so. The other choice is to remove them; `test_run_validator_starts_every_child_with_no_site` already holds `-S`. Small, tests only.

Review: rv666, lane 5's independent review, approved a2113cf with nits; 7f2bb36 and 426877b take its S1 and N1 (above), and codex found no major issue at a2113cf. CI (Brett Heap's ruling 27 and his fast-refresh ruling of 2026-10-15T22:48:35Z): H0 426877b passed tests, tests-git-bash and SonarCloud; the head 32722b3 is a clean update-branch merge of main bf1f099, with tests-git-bash and SonarCloud green; and the local gate passed on its predicted squash tree 55d5d3b over main dcf62ed: the shape_doctor, scaffold_leg_path_collisions, repo_hygiene, agents_md_codes, cli_reference sets, 686 passed, 3 skipped, 1 xfailed. Landed on Brett Heap's word of 2026-10-17T10:59:39Z, relayed by lane 3 on #473: "merge lane 5's approved v1.1.0 drafts".

Closes #666

Lane: openreposhape-5 (openRepoShape-5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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