Skip to content

adopt-project.py, shape_materialize.py: five printed sentences say what the run does: a kept root Makefile's block, the spec leg's own path, where execute stops, when a link target names an attribute, and no visibility for a bare repository (#906) - #964

Merged
brettheap merged 3 commits into
mainfrom
fix/906-printed-sentences
Oct 19, 2026

Conversation

@brettheap

@brettheap brettheap commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

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

A v1.2.0 draft, authored on the v1.1.1 release commit 50178305; this body is for head 369f2781, one fix round on 10f25e4b after lane 1's review r906, which found the first commit not ready. Lane 3 filed #906 from its triage of #457 (L2-72, L2-73, L1-91, L1-93 and L1-75): five printed sentences that say what the run does not do. It changes adopt-project.py, scripts/shape_materialize.py and five test files, one of them new (tests/test_adopt_printed_sentences.py).

What

  1. L2-72, the Makefile follow-up. follow_ups_for said "the root Makefile wires CONTRACTS_DIR" even where the root keeps a Makefile of the source's own. Then the shape's goes beside it as shape/Makefile with the block, and the root wires nothing. Now it says "the shape's Makefile wires ...; but the source keeps a Makefile of its own at the root, so that block waits in shape/Makefile until the two are merged, and the run adds none of it to the root's Makefile", as Pinned shape copies: one bump, so AGENTS-shape.md is true of every root, the scaffold's chain WARNING offers only its own sources and spells --pin-source, the root's not-used note spells it too, and THE LOCKSTEP RULE's bump runs (#610) #634 words it for AGENTS-shape.md. A new _root_keeps is the one reading, shared with _predict_collisions. Otherwise the line is as it was.
  2. L2-73, the spec leg's own path. contracts_from_code_leg takes the plan's follow-ups. Where one says a file must "read across the assembly root" (holds_a_read_across), check's mount the legs at X/ and Y/ and the split message's mounted back here at X/ and Y/ end ", so from the code leg the spec leg is ../spec/", plus " and the spec leg's contracts/ is ../spec/contracts/" where contracts/ is there too. With no read-across line they are as before.
  3. L1-91, where execute stops. The finding no longer says "would stop there". For a refused store or copy it says execute would stop "no later than committing the split, and sooner where a step before it meets the same rule: extracting or seeding a leg, or mounting the legs, where the rule reaches the .gitmodules it writes"; for a refused checkout, "at its first clone of the source" (STOPS_AT_THE_SPLIT, STOPS_AT_A_CLONE).
  4. L1-93, what a link target names. Two sentences said "a path names no attribute", which holds only for a link target with no space, tab or carriage return: git reads the target as one line of rules, and separates its fields on all three.
    • _kept_nested_attributes's docstring says so. The code was right already: it asks about every kept link.
    • scripts/shape_materialize.py's _attributes_link_follow_up, the printed sentence Tracking: adopt-project.py, converting an existing repository in place #457's L1-93 names, now says a checkout "reads only the link's own text, the path it leads to, as one line of rules, which names an attribute only where that text holds a space, a tab or a carriage return, so, for a link to a path, no line-ending rule reaches the pinned copies outside it". _append_eol_block's docstring and a comment in adopt-project.py say the same.
  5. L1-75, visibility. Under --local-remote-dir, WHAT WILL HAPPEN prints create <Project>-spec and <Project>-code with no (private), since execute makes bare repositories. Against GitHub the line is unchanged.

Why

Measured on 50178305, git 2.43, Linux:

  • L2-72: after a real execute --local-remote-dir of a source with its own root Makefile, make contracts-dir at the root printed No rule to make target. CONTRACTS_DIR was only in shape/Makefile.
  • L2-73: the issue says v1.1.0 printed ../spec/specs/. It did not: v1.0.1 did, and adopt-project.py: after the hand edit of legs.spec_path or legs.code_path it asks for, the read-across follow-up and the contracts/ rule's reason still name the plan-time paths #600 took the path out of the follow-up. The tests follow ../spec/specs/001-feature/spec.md from the code leg of a recursive clone and reach the file.
  • L1-91: with _refuse_what_check_finds patched out, core.safecrlf=true over a kept CRLF file stops execute at REFUSED committing the split; * working-tree-encoding=UTF-16LE with .gitattributes exempted, at REFUSED mounting the legs (failed to encode '.gitmodules'); a failing smudge filter, at REFUSED extracting the spec leg; and, with the code leg seeded (src/ and .github/ answered root) under core.autocrlf=true and core.safecrlf=true, at REFUSED seeding the code leg, where _seed_leg's own git add -A meets the rule before the split does.
  • L1-93, nested: a nested .github/.gitattributes link to config/attributes or docs/read me.md left the workflow copy LF; to *.yml eol=crlf, with a space, a tab or a carriage return between the fields, it wrote CRLF.
  • L1-93, root: clones of a source whose root .gitattributes links to config/attributes or docs/read me.md left a committed-LF notes.txt LF; links to * eol=crlf, with a space, a tab or a carriage return, wrote it CRLF. A test holds each.

Tests

tests/test_adopt_printed_sentences.py, 54 tests, a group for each item. OWN_MAKEFILE_FOLLOW_UP sits beside MAKEFILE_FOLLOW_UP in tests/test_adopt_contracts_dir.py; that file, test_adopt_leg_keys.py, test_adopt_reencoded_files.py and test_adopt_attributes_eol_block.py take the new text.

  • Without the fix: with 50178305's adopt-project.py and shape_materialize.py put back (a scratch copy of the tree), 38 of the 54 fail, and one test each in test_adopt_attributes_eol_block.py and test_adopt_reencoded_files.py. The 16 that pass pin what the fix leaves alone: the Makefile line where the root keeps none (2), where execute really stops (4), and what git does with a link target, nested (5) and at the root (5).
  • Against the first commit (10f25e4b): 8 fail: the three finding rows that say where execute stops for a store (the seeded-leg row among them), the docstring test, the two kept-Makefile line tests, test_the_follow_up_for_a_linked_attributes_file_lists_the_lines_it_needs, and test_a_refusal_says_where_execute_stops_and_the_way_out.

Run with /usr/bin/python3 -m pytest, a stub gh first on PATH, a bogus GH_TOKEN, an empty GH_CONFIG_DIR, GIT_CONFIG_GLOBAL=/dev/null and GIT_SSH_COMMAND=false, on head 369f2781, one run per file:

  • printed_sentences 54, attributes_eol_block 46, eol_block_through_links 13 (2 skipped), nested_attributes 79, contracts_dir 34, leg_keys 44, reencoded_files 53, agents_md_codes 47, cli_reference 23, repo_hygiene 78; adopt_e2e 27, adopt_plan 67 (1 skipped, SHAPE_MEDXEHR_CLONE unset), adopt_submodules 167; test_shape_doctor 445 (3 skipped, PowerShell, a Windows host only); test_scaffold_leg_path_collisions 104. All passed.
  • Not re-run since 10f25e4b, where they passed, and none reads a sentence this round changed: plan_yaml_quoting 518, and the 13-file batch of 1088 passed, 2 skipped (among them leg_paths, literal_names, entry_paths, leg_path_collisions, execute_refusals, c_locale_bytes, plan_escapes, stdout_encoding, shape_advisory).

Known limits and follow-ups

Closes #906

🤖 Generated with Claude Code

Summary by Sourcery

Make adoption messages accurately describe generated files, cross-leg paths, execution failure points, attribute-link semantics, and local bare-repository visibility.

New Features:

  • Clarify adoption output with the spec leg’s path, accurate Makefile ownership, precise refusal stopping points, and Git attribute-link behavior.
  • Suppress repository visibility labels when local remote directories create bare repositories.

Bug Fixes:

  • Correct printed guidance for sources that retain a root Makefile, read-across paths without a contracts directory, Git attribute links containing rule separators, and execution failures that occur before the split commit.

Enhancements:

  • Centralize root-path retention detection and use plan follow-ups to determine when cross-leg paths should be displayed.

Tests:

  • Add comprehensive coverage for the five corrected printed-sentence behaviors, including end-to-end adoption, refusal, path, attribute, and local repository cases.

…n does (the kept root Makefile's block waits in `shape/Makefile`, the spec leg's own path is printed beside the mounts, `execute` is said to stop where it does, a link target names an attribute only with a space or tab, no visibility under `--local-remote-dir`); not finished: the fail-without-fix run, the remaining test files, README line 810 (#906)

Lane 3's triage of #457 at 5017830 found five lines that said what the run
does not do. All five are changed and measured; the verification is not
finished (see the end).

L2-72. `follow_ups_for` said "the root Makefile wires `CONTRACTS_DIR`" where
the source keeps its own root Makefile. Then the shape's, with the block, is
written to `shape/Makefile`, and `make contracts-dir` at the root answers `No
rule to make target 'contracts-dir'` (measured on an `execute
--local-remote-dir` of a source with a Makefile; the block is only in
`shape/Makefile`). Where the root keeps the Makefile it now says "the shape's
Makefile wires ...; but the source keeps a Makefile of its own at the root, so
that block waits in `shape/Makefile` until the two are merged, and the run
adds none of it to the root's Makefile before then", as #634 words it for the
pinned AGENTS-shape.md. Otherwise the line is byte for byte what it was. The
test is `_root_keeps`, the reading `_predict_collisions` already used to move
the shape's copy, now one function for both.

L2-73. `contracts_from_code_leg` gave a ready path only where the spec leg
holds `contracts/`, so a code file that names `specs/` got the key-form line
and no path. v1.1.0 printed none either: the `../spec/specs/` was v1.0.1's,
before #600 took the path out of the follow-up. It now takes the plan's
follow-ups too, and where one says a file must `read across the assembly
root` (`holds_a_read_across`, found in both lines `plan` writes for it) the
two sentences that print the mounts end ", so from the code leg the spec leg
is `../spec/`", and, where the spec leg holds `contracts/`, " and the spec
leg's `contracts/` is `../spec/contracts/`". With no read-across line the
sentence is what it was. Measured on a source naming `specs/` only, for
`spec`/`code`, `legs/spec`/`legs/code` and `a/b/spec`/`a/code`, and by
following the path from the code leg of a recursive clone to the spec file.

L1-91. `_refused_finding` said `execute` "would stop there, after the leg
repositories exist", `there` being the step git refuses. Measured with
`_refuse_what_check_finds` patched out, over a `--local-remote-dir`: a
refused `git add` (`core.safecrlf=true` over a kept CRLF file) stops
`execute` at `REFUSED committing the split`; `* working-tree-encoding=UTF-16LE`
with `.gitattributes` exempted stops it at `REFUSED mounting the legs`, `failed
to encode '.gitmodules'`; a smudge filter that fails stops it at `REFUSED
extracting the spec leg`, neither leg pushed. The finding says "no later than
committing the split, and sooner where a step before it meets the same rule:
extracting a leg, or mounting the legs, where the rule reaches the
`.gitmodules` it writes" for a refused store, and "at its first clone of the
source" for a refused checkout.

L1-93. The `_kept_nested_attributes` docstring said "a target that is a path
names no attribute". Measured with git 2.43: a kept `.github/.gitattributes`
linking to `config/attributes` (and to `docs/read me.md`) left the workflow
copy LF; linking to `*.yml eol=crlf`, or `*.yml` TAB `eol=crlf`, wrote it
CRLF. It now says it of a target with no space or tab only. The printed
sentence in scripts/shape_materialize.py (`a path names no attribute`) is not
changed here.

L1-75. Under `--local-remote-dir`, WHAT WILL HAPPEN printed `create
<Project>-spec and <Project>-code (private)`. It prints no visibility there
(`execute` makes both with `git init --bare`); against GitHub the plan's
visibility is printed as before.

Tests. tests/test_adopt_printed_sentences.py is new (46 tests, all pass).
tests/test_adopt_leg_keys.py, tests/test_adopt_contracts_dir.py (which also
holds OWN_MAKEFILE_FOLLOW_UP beside MAKEFILE_FOLLOW_UP) and
tests/test_adopt_reencoded_files.py move with the sentences they assert. Run
and green with the stubbed-gh environment: test_adopt_leg_keys,
test_adopt_contracts_dir, test_adopt_reencoded_files, test_adopt_leg_paths,
test_adopt_literal_names, test_adopt_plan, test_adopt_entry_paths,
test_adopt_leg_path_collisions, test_adopt_execute_refusals,
test_adopt_c_locale_bytes, test_adopt_plan_escapes, test_stdout_encoding and
test_shape_advisory together: 1088 passed, 2 skipped.

Not finished: the run of the new tests against the base adopt-project.py
(fail-without-fix); test_adopt_nested_attributes,
test_adopt_attributes_eol_block, test_adopt_eol_block_through_links,
test_adopt_e2e, test_adopt_plan_yaml_quoting, test_adopt_submodules,
test_shape_doctor, test_scaffold_leg_path_collisions, test_cli_reference and
test_repo_hygiene; and README.md line 810, which says the root Makefile
exports `CONTRACTS_DIR` without the kept-Makefile case.

Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@brettheap brettheap added this to the v1.2.0 milestone Oct 18, 2026
@sourcery-ai

sourcery-ai Bot commented Oct 18, 2026

Copy link
Copy Markdown

Reviewer's Guide

Updates adopt-project.py’s printed guidance and documentation to reflect actual Makefile placement, cross-leg navigation, Git refusal stop points, nested attribute parsing, and local bare-repository visibility, with broad regression and end-to-end coverage in a new printed-sentences test module.

Sequence diagram for local adoption guidance and execution stop points

sequenceDiagram
    participant User
    participant Adopt as adopt-project.py
    participant Plan
    participant Git
    User->>Adopt: plan
    Adopt->>Plan: _plan_follow_ups
    Adopt->>Adopt: contracts_from_code_leg
    Adopt->>Adopt: _print_what_will_happen
    Adopt-->>User: Print leg paths and create repositories
    User->>Adopt: execute
    Adopt->>Git: clone source
    alt checkout refusal
        Git-->>Adopt: refusal
        Adopt-->>User: execute stops at its first clone of the source
    else extraction or mount refusal
        Adopt->>Git: extract or mount legs
        Git-->>Adopt: refusal
        Adopt-->>User: execute stops before committing the split
    else no refusal
        Adopt->>Git: commit split
        Git-->>Adopt: success
    end
Loading

Flow diagram for adopt-project guidance generation

flowchart TD
    Plan[Plan follow-ups and leg paths] --> Makefile[follow_ups_for]
    Plan --> Mounts[contracts_from_code_leg]
    Plan --> Refusals[_reencoded_files_findings]
    Plan --> Visibility[_print_what_will_happen]
    Makefile --> RootCheck[_root_keeps]
    RootCheck --> MakefileText[Makefile placement guidance]
    Mounts --> ReadAcross[holds_a_read_across]
    ReadAcross --> MountText[Spec-leg path guidance]
    Refusals --> StopText[execute stop-point guidance]
    Visibility --> LocalText[Local bare-repository visibility]
    MakefileText --> Output[Printed adoption guidance]
    MountText --> Output
    StopText --> Output
    LocalText --> Output
Loading

File-Level Changes

Change Details Files
Make adoption output accurately describe Makefile placement and cross-leg paths.
  • Detect when the source keeps its own root Makefile and explain that the generated block remains in shape/Makefile until merge.
  • Derive and print the spec leg’s path for read-across follow-ups, optionally alongside its contracts path.
  • Share root-preservation logic between collision prediction and follow-up generation.
adopt-project.py
tests/test_adopt_contracts_dir.py
tests/test_adopt_leg_keys.py
tests/test_adopt_printed_sentences.py
Clarify execution stopping points for Git re-encoding refusals.
  • Add explicit stop-location metadata for checkout, store, and copy refusals.
  • Report that execution may stop during extraction or mounting, rather than implying it always stops at the refused final step.
  • Add unit and execution-level coverage for clone, mount, and split-commit scenarios.
adopt-project.py
tests/test_adopt_reencoded_files.py
tests/test_adopt_printed_sentences.py
Correct adoption visibility output for local bare repositories.
  • Suppress plan visibility in WHAT WILL HAPPEN when --local-remote-dir creates bare repositories.
  • Retain visibility output for GitHub-backed repository creation.
  • Add coverage across private, public, and internal plan visibility values.
adopt-project.py
tests/test_adopt_printed_sentences.py
Make nested attributes documentation match Git’s parsing behavior.
  • Document that link targets without whitespace are patterns with no attributes, while whitespace separates patterns from attributes.
  • Add regression coverage for path targets, space-separated rules, and tab-separated rules.
adopt-project.py
tests/test_adopt_printed_sentences.py

Assessment against linked issues

Issue Objective Addressed Explanation
#906 Make the Makefile follow-up describe where the contracts block actually waits when the source keeps its own root Makefile. ✅
#906 Print the spec leg’s path from the code leg beside the mounts whenever the plan has a read-across follow-up, while retaining the contracts-specific path where applicable. ✅
#906 Correct the printed finding about where execution stops, narrow the link-target docstring claim, and omit visibility from WHAT WILL HAPPEN for local bare repositories. ✅

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

@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-1, 2026-10-18T18:31:12Z) Independent review of #964 at 10f25e4b, by lane 1's reviewer r906 (opus), who is not its writer: NOT READY. Of the five items, L2-72, L2-73 and L1-75 are met, and L1-91 and L1-93 are not yet true. Lane 1 fixes all of them here.

  • MUST-1: the L1-91 sentence leaves out a step where execute stops. With the code leg seeded under core.autocrlf=true and core.safecrlf=true, execute stops at REFUSED seeding the code leg., which "extracting a leg, or mounting the legs" does not name. Fix: "extracting or seeding a leg, or mounting the legs", with a STOPS row for a seeded leg.
  • MUST-2: the sentence Tracking: adopt-project.py, converting an existing repository in place #457's L1-93 names is the printed one, at scripts/shape_materialize.py:1004 (from adopt-project.py check: a kept or moved file other than .gitattributes whose blob git add would re-encode passes, and execute ends adopt-lost or dies mounting the legs after both legs are pushed #654), with its docstring at :870. The branch leaves it as it was. A root .gitattributes link whose target holds a separator writes CRLF, so "no line-ending rule reaches the pinned copies outside it" is false there. Fix: those two places and tests/test_adopt_attributes_eol_block.py:399 together.
  • SHOULD-1: git's attribute parser also splits on a carriage return. A nested link to *.yml\reol=crlf, with no space and no tab, wrote CRLF (git 2.43). So "no space or tab" becomes "no space, tab or carriage return", with a row for it.
  • Met, with mutants:
    • L2-73's path covers a prefix of the text that is not a prefix of a segment, case, a parent segment, and mounts that are not text.
    • L2-72 and L1-75.
    • 16 of 18 mutants are killed; the 2 that survive are equivalent.
  • Tests: 325 passed over the changed files and the docs and codes files.
  • Follow-ups, filed with the round:
    • a kept lower-case makefile or GNUmakefile still gets the old line;
    • README.md:810, whose section is bound to the handbook, with corrected wording.

Lane 1's writer f906 makes the round, then r906 re-checks the new head.

Lane: openreposhape-1 (openRepoShape-1)

…seeding a leg too, a link target names an attribute with a carriage return as with a space or tab, and a root `.gitattributes` link's follow-up says no rule reaches the pinned copies only for a link to a path (#906)

r906's review of #964 found three of the sentences the first commit wrote or left false in cases it measured. Each is fixed here, with the measurement under a test.

- `STOPS_AT_THE_SPLIT` names seeding: "extracting or seeding a leg, or mounting the legs". With the code leg seeded (`src/` and `.github/` answered root) and `core.autocrlf=true` with `core.safecrlf=true`, `execute` stops at `REFUSED seeding the code leg`: `_seed_leg` runs its own `git add -A`, which meets the rule before the split does. The STOPS row `a-seeded-leg-meets-the-same-config` holds it, and its finding test fails against the first commit's `adopt-project.py`.
- `_kept_nested_attributes`'s docstring says a link target with no space, tab or carriage return names no attribute, and one with any of them is read as rules, since git separates a line's fields on all three. Measured with git 2.43: `*.yml` CR `eol=crlf` wrote `validate.yml` CRLF, as a space or a tab does. `test_a_link_target_names_an_attribute_only_with_a_separator` has the row `a-carriage-return-between-them`, and the docstring test follows the words.
- `shape_materialize.py`'s `_attributes_link_follow_up` printed "a path names no attribute", which holds only for a link to a path, and so did `_append_eol_block`'s docstring and the comment in `adopt-project.py` above the nested-file finding. Measured in clones with git 2.43: a root `.gitattributes` link to `config/attributes` or `docs/read me.md` left a committed-LF `notes.txt` LF, and one to `* eol=crlf`, with a space, a tab or a carriage return between the fields, wrote it CRLF (`test_a_root_link_is_read_as_rules_only_with_a_separator`). The follow-up now says the link's text is read as one line of rules, which names an attribute only where it holds a space, a tab or a carriage return, so that, for a link to a path, no line-ending rule reaches the pinned copies; `tests/test_adopt_attributes_eol_block.py` asserts it.
- The Makefile follow-up no longer ends "before then", which suggested the run acts when the two Makefiles are merged.
- `test_a_kept_makefile_wires_nothing_where_the_split_message_says_so` asserts that `make -f shape/Makefile contracts-dir` prints the clone's own `legs/spec/contracts`, not any directory.
- The `STOPS_AT_*` comment no longer says "neither leg yet pushed", which r906 found false where the spec leg is seeded, or that `git add` is the split's last step: the push of the split branch follows.

Not changed: a kept lower-case `makefile` or `GNUmakefile`, and `README.md:810`, whose section is bound to the handbook. Both are follow-ups.

Lane: openreposhape-1 (openRepoShape-1)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@brettheap brettheap changed the title adopt-project.py: the Makefile follow-up says a kept root Makefile's block waits in shape/Makefile, the mount lines print the spec leg's own path, plan-file-reencoded says where execute stops, and --local-remote-dir prints no visibility (#906) adopt-project.py, shape_materialize.py: five printed sentences say what the run does: a kept root Makefile's block, the spec leg's own path, where execute stops, when a link target names an attribute, and no visibility for a bare repository (#906) Oct 18, 2026
@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-1, 2026-10-18T19:00:21Z) Re-check of #964 at 369f2781, by r906 (opus), who is not its writer: READY.

#964 takes its turn once its tests are green at 369f2781, gated over main as it then stands. An explicit squash message replaces the first commit's WIP text.

Lane: openreposhape-1 (openRepoShape-1)

@sonarqubecloud

Copy link
Copy Markdown

@brettheap
brettheap marked this pull request as ready for review October 19, 2026 12:26
@sourcery-ai

sourcery-ai Bot commented Oct 19, 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 2 days and 7 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@brettheap
brettheap merged commit b0614d5 into main Oct 19, 2026
5 checks passed
@brettheap
brettheap deleted the fix/906-printed-sentences branch October 19, 2026 12:27
brettheap added a commit that referenced this pull request Oct 20, 2026
…makefile` or `GNUmakefile` GNU make reads first, and where a case-blind disk writes the shape's; README says which Makefile exports `CONTRACTS_DIR` (#977) (#1046)

## What

Two follow-ups to #906 that lane 1's review of #964 (r906) found and
ruled out of that pull request. Neither is a regression: both were as
untrue on `main` before #906 as after it.

**(a) The Makefile follow-up names the root file GNU make reads first.**
The line is in `plan`'s `follow_ups:`, in `check`'s output and in the
split commit's message, which is also the pull request's body. GNU make
reads the first file it finds of `GNUmakefile`, `makefile` and
`Makefile`, and only that one. #906 asked only whether the source keeps
`Makefile`, comparing names byte for byte (`_root_keeps`). So a source
that keeps `makefile` or `GNUmakefile` got "the root Makefile wires
`CONTRACTS_DIR` ...". But the clone's root then holds the shape's
`Makefile`, with the block, and the source's file, which `make` reads
instead, so `make contracts-dir` answered `No rule to make target
'contracts-dir'` (r906's probe, both names).

`_makefile_follow_up` now writes the line:

- It names the source's file that GNU make reads at the root, wherever
  that file is not the `Makefile` #906's words already name. It also
  says what it takes for `make` there to export `CONTRACTS_DIR`.
- On a disk that ignores case, a kept `makefile` IS `Makefile`.
  `place` (`Path.exists()`) finds the name taken and writes the shape's
  Makefile as `shape/Makefile`. The line says so in `CASE_ASIDE`'s
  words. `_root_keeps` gains `fold`, which asks the question the way
  `_KeptTree` asks it for `check`'s disk that ignores case.
- A source that keeps only `Makefile`, or none of the three names and no
  other case of `Makefile` that a rule keeps, gets the line it got before,
  byte for byte.
- Each kept name is printed with `_spelled`, and the three names GNU make
  looks for with `_and_listed`. No `shell_quoted` call is added.

| The root keeps | Before | After |
| --- | --- | --- |
| none | "the root Makefile wires `CONTRACTS_DIR` ..." | unchanged |
| `Makefile` | #906's line: "the shape's Makefile wires ...; but the source keeps a Makefile of its own at the root, so that block waits in `shape/Makefile` until the two are merged, and the run adds none of it to the root's Makefile" | unchanged |
| `makefile` | "the root Makefile wires ..." | "the shape's Makefile wires ...; it is written as the root's `Makefile`, or as `shape/Makefile` once case is ignored, as a macOS or Windows disk does, where the source's `makefile` is that path, until the two are merged; and GNU make at the root reads the source's `makefile`, ..." |
| `GNUmakefile` | "the root Makefile wires ..." | "the shape's Makefile wires ...; it is written as the root's `Makefile`, but GNU make at the root reads the source's `GNUmakefile`, ..." |
| `makefile` and `GNUmakefile` | "the root Makefile wires ..." | as for `makefile`, and GNU make reads the source's `GNUmakefile` |
| `Makefile` and `makefile` | #906's line | #906's line, then "; and GNU make at the root reads the source's `makefile`, ..." |
| `Makefile` and `GNUmakefile`, or all three | #906's line | #906's line, then "; and GNU make at the root reads the source's `GNUmakefile`, ..." |
| `MAKEFILE`, kept in the root by a `--path-policy` rule (with the shipped rules `plan` asks about it, and a person's `root` answer keeps the line `plan` wrote: "the root Makefile wires ...") | "the root Makefile wires ..." | "the shape's Makefile wires ...; it is written as the root's `Makefile`, or as `shape/Makefile` once case is ignored, ..., where the source's `MAKEFILE` is that path, until the two are merged" |

The clause that names the file, in full:

```text
GNU make at the root reads the source's `makefile`, the first of
`GNUmakefile`, `makefile` and `Makefile` it finds, so `make` there exports
`CONTRACTS_DIR` only once `makefile` does, or includes a Makefile that does
```

**(b) README.md, section "Adopting an existing repository".**

- Before: "the root Makefile it materializes exports `CONTRACTS_DIR`,
  pointing at the `contracts/` of the spec leg where the plan mounts it
  (`$(CURDIR)/spec/contracts` by default)."
- After: "the shape's Makefile exports `CONTRACTS_DIR`, pointing at the
  `contracts/` of the spec leg where the plan mounts it
  (`$(CURDIR)/spec/contracts` by default); it is the root's Makefile, or,
  where the source keeps a `Makefile` of its own, `shape/Makefile`, which
  holds the export until a person merges the two."

This is r906's wording, as its re-check corrected it, with one change:
`Makefile` is in backticks in "a `Makefile` of its own". A source that
keeps `makefile` has the shape's Makefile written as the root's
`Makefile` on a disk that keeps case, so the plain word would have
claimed `shape/Makefile` for it.

The section is bound to the handbook's `adopt` section. The handbook
says nothing of `CONTRACTS_DIR`, and its sentence that a shape file
whose name the source already holds, `Makefile` among them, is written
under `shape/` stays true. So only the section's marker moves, to the
section's new blob, computed with `readme_segments` and
`readme_fingerprint`.

The tail of the paragraph is reflowed, and README.md grows by one line,
to 1479, which is the cap. No `tests/readme-cap.d/` file is needed while
no other README pull request lands first.

## Measured

On a disk that keeps case (Linux, GNU Make 4.3, git 2.43), each row ran
a real `execute --local-remote-dir` and made a recursive clone of the
split. Each file the source keeps has its own `which` target, so `make
which` at the clone's root prints the name of the file GNU make read.

| The root keeps | `make which` prints | the split message names | `make contracts-dir` at the root | the block is in (`make -f <it> contracts-dir` prints the spec leg's `contracts/`) |
| --- | --- | --- | --- | --- |
| `makefile` | `makefile` | `makefile` | no rule | `Makefile` |
| `GNUmakefile` | `GNUmakefile` | `GNUmakefile` | no rule | `Makefile` |
| `makefile` and `GNUmakefile` | `GNUmakefile` | `GNUmakefile` | no rule | `Makefile` |
| `Makefile` | `Makefile` | #906's line, the source's own `Makefile` | no rule | `shape/Makefile` |
| `Makefile` and `makefile` | `makefile` | `makefile` | no rule | `shape/Makefile` |
| `Makefile` and `GNUmakefile` | `GNUmakefile` | `GNUmakefile` | no rule | `shape/Makefile` |
| all three | `GNUmakefile` | `GNUmakefile` | no rule | `shape/Makefile` |

The disk that ignores case is not measured: there is none here. What
the line says of it rests on `place`'s `Path.exists()`, and is tested
against `_KeptTree(..., fold=True)`, `check`'s model of that disk.

## Tests

- `tests/test_adopt_make_reads_first.py`, new: 33 tests.
  - The line word for word, for every row above, and for `makefile`
    and `GNUmakefile` sent to legs.
  - The line against `_KeptTree` on both disks.
  - `plan` and `check` for each row but none and `Makefile`, which
    `test_adopt_contracts_dir` and `test_adopt_printed_sentences` already
    plan and check.
  - A real `execute` for each row except none, with the `make`
    measurements above.
  - README's sentence, and the handbook marker matching the section as
    it now stands.
- Fail on base: on `e2f9fc42` (main when the branch was made), 23 fail
  and 10 pass. The 10 are the handbook marker test, which was current
  on base, and rows whose line does not change: none and `Makefile`
  alone; `makefile` and `GNUmakefile` sent to legs; and the
  `Makefile`-plus-another rows of the disk test, which asks only where
  the copy goes.
- Gate on this head, named files only, one basetemp, a stub `gh` first
  on PATH, `GH_TOKEN=bogus`, `GIT_SSH_COMMAND=false`,
  `GIT_CONFIG_GLOBAL=/dev/null`:
  - The new file, `test_adopt_contracts_dir`, `test_adopt_printed_sentences`,
    `test_adopt_leg_keys`, `test_adopt_plan`, `test_adopt_e2e`,
    `test_repo_hygiene`, `test_readme_sections`, `test_agents_md_codes`,
    `test_cli_reference`, `test_readme_windows_quoting_sentence` and
    `test_scaffold_leg_path_collisions`: 652 passed, 1 skipped (the
    MedxEHR test, no clone here).
  - Every other adopt test file that names `Makefile`, `makefile` or
    `CONTRACTS_DIR`: `attributes_eol_block`, `verify`,
    `ignored_shape_files`, `leg_path_collisions`, `submodules`: 368
    passed. `root_manifest`, `root_symlinks`, `symlink_target_bytes`,
    `root_shape_paths`: 403 passed.
  - `test_shape_doctor.py` in 4 shards: 445 passed, 3 skipped (the
    PowerShell line, Windows only).
  - After the last test-only edit (helpers imported from
    `test_adopt_printed_sentences`): the new file and
    `test_adopt_printed_sentences`, 87 passed (33 and 54).

Mutants of the change, each restored. Each one is killed:

| Mutant | Failed |
| --- | --- |
| GNU make's order with `makefile` before `GNUmakefile` | 18 of 33. The end-to-end rows `makefile`+`GNUmakefile` and all three fail on `make which` against the split message itself. |
| the case-folding check off | 9 |
| the joining word always "; and" | 3 |
| "the shape's" not chosen when only `GNUmakefile` or `makefile` is kept | 4 |
| GNU make's first file never found | 19 |
| `_root_keeps` folds case by default | 13, #906's `the-root-keeps-it` row among them |
| README's old sentence | 3, `test_the_handbook_follows_the_readmes_outline` among them |
| the handbook marker left stale | 2 |

## Merge with main

`git merge-tree --write-tree` of this head with `main` at `dc7cd49b` is
clean (tree `ae9abfcb`). Of this branch's files, only
`adopt-project.py` moved on main: #1030 (#900) changed `_spelled`, and
#1038 (#994) changed a hint and a docstring. `_spelled` changes only a
backslash, a double quote or a backtick inside a name. The names this
line spells are plain letters, so its words do not change. README.md,
`docs/handbook.html` and `tests/readme-cap.d/` did not move on main, so
the marker stays current and the cap still holds.

On the merge, built in a `git clone --shared` checkout: the new file,
`test_adopt_printed_sentences`, `test_repo_hygiene` and
`test_readme_sections`, 263 passed.

## Not in this PR (follow-ups for lane 1 to file)

- **NEXT's `make bootstrap`, measured.** For a source that keeps
  `makefile` or `GNUmakefile`, `execute`'s NEXT block prints `make
  bootstrap`. `make -n bootstrap` in the clone exits 2 with `No rule to
  make target 'bootstrap'`. For `Makefile` and `GNUmakefile`, its NOTE
  says the shape's make targets "arrive only once the follow-up merges
  the two". But GNU make at the root reads `GNUmakefile`, not the merged
  `Makefile` (inferred; before the merge, `make -n bootstrap` there
  exits 2, as the NOTE says).
- **`collision_follow_up`'s sentence.** In `shape_materialize.py`,
  "The shape's make targets ... arrive only with that merge" is likely
  untrue where the source also keeps `GNUmakefile` or `makefile`, which
  GNU make reads before the merged `Makefile`. Inferred, not measured.
- **`AGENTS-shape.md` in the pinned template.** "Where the source kept a
  Makefile of its own, that block waits in `shape/Makefile`" has the
  same gap for a kept `makefile`. It is pinned bytes, so it is left.
- **Disks that ignore case.**
  - `_predict_collisions` stays byte for byte, so on such a disk a plan
    for a kept `makefile` lists no "merge shape/Makefile into Makefile"
    follow-up. `execute`'s own collision report lists it.
  - A split made on a disk that keeps case commits both `Makefile` and
    `makefile`, two names that collide in a clone on a disk that
    ignores case.
  - Spellings such as `gnumakefile` are not asked about.
  - None of this is measured.

**Review round 0, body only** (lane 1's reviewer r1046's partial review, issuecomment-6288714370): the `MAKEFILE` row and the byte-for-byte sentence now say that a `--path-policy` rule, not a person's answer, is what keeps `MAKEFILE` in the root. Follow-ups are built from the rules' answers alone, so a `MAKEFILE` a person answers `root` keeps "the root Makefile wires ...", which a disk that ignores case makes untrue, as on base.

**Review round 1** (`c121ed90`, test file only): the comment above `ANSWERED` says a `--path-policy` rule, not a person's answer, reaches the follow-ups (r1046's SHOULD-1). The new test file is ASCII: its section signs and em dashes are written as escapes, with two lines rewrapped: the assert's string is unchanged, the `re` pattern matches the same text (its raw string now holds the escape), and one docstring changes only where its lines break (NIT-2). `tests/test_adopt_make_reads_first.py` 33, `tests/test_adopt_printed_sentences.py` 54 and `tests/test_repo_hygiene.py` 100 passed.

It lands by fast refresh (Brett Heap, 2026-10-15T22:48:35Z). Its reviewed head `c121ed90` is green on Linux and SonarCloud. The branch merged main once and was never rebased; the last merge is update-branch's, clean, giving db92198, which is green on SonarCloud and Git Bash. Lane 1's gate passed on its tree: the 13 test files that run `adopt-project.py`'s Makefile follow-ups and `make`, read README and the handbook, the doctor and the docs, 1,097 passed and 4 skipped.

Closes #977

Lane: openreposhape-1 (openRepoShape-1)
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