Skip to content

shape-doctor.py: the pins row keeps its gate where update-shape.py check says REFUSAL AHEAD beside a merged-away row, so it no longer says that the apply check prints drops the row when that apply is refused, and the shape currency counts name every state check counts (#676) - #751

Merged
brettheap merged 3 commits into
mainfrom
l3-676-pins-merged-away
Oct 17, 2026

Conversation

@brettheap

@brettheap brettheap commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

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

Closes #676.

Stacked on #742 (#683); lands after it. This branch starts at #742's head 3027937d, so until #742 lands this PR's diff shows #742's commits too. Two reasons:

Re-measured on main 66b230ed (with #742, which does not touch either row)

The issue's two lines still hold. The root is tests/test_update_shape_merged_away.py's adopted root, with shape/Makefile merged away and AGENTS-shape.md kept as a committed symlink to the same bytes.

  • update-shape.py check says merged-away and copy-is-link, and REFUSAL AHEAD: \apply` will refuse while these are unresolved — AGENTS-shape.md`.
  • pins row: NEXT update-shape.py check. Its reason ends "…reports it `merged-away`, and the `apply` it prints drops its pin row in the re-pin commit: the step the `shape currency` row names too". No refusal_ahead in the detail.
  • shape currency row: 9 unchanged / 0 upstream-changed / 0 locally-modified / 0 both / 0 upstream-added / 1 merged-away … : shape/Makefile, AGENTS-shape.md. That is 10 counted over 11 rows, with the link's path under no count.
  • I used AGENTS-shape.md rather than the issue's .gitignore because git does not follow an in-tree .gitignore or .gitattributes kept as a symlink. A copy nothing reads at run time keeps every gate's answer the same.

What changed and why

shape-doctor.py

Tested

The tests are in tests/test_update_shape_merged_away.py, which owns #502's merged-away root and its helpers.

Command, run in three groups at once (stub gh first on PATH, GH_TOKEN=bogus, empty GH_CONFIG_DIR):

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

Results, all on b7312258:

  • tests/test_shape_doctor.py: 428 passed, 3 skipped, 1 xfailed in 356.07s (0:05:56).
  • tests/test_update_shape_merged_away.py, tests/test_update_shape_hardlink.py, tests/test_doctor_partial_clone.py, tests/test_doctor_gitlink_repairs.py and tests/test_doctor_pin_restore_merges.py, together: 98 passed in 135.01s (0:02:15).
  • tests/test_doctor_next_is_a_command.py, tests/test_doctor_legs_members_next.py, tests/test_pin_source_env_collisions.py, tests/test_shape_pin_files_not_a_list.py, tests/test_update_shape_next_quoting.py, tests/test_repo_hygiene.py and tests/test_cli_reference.py, together: 496 passed in 254.38s (0:04:14).

Pinned copies

None. shape-doctor.py is not copied into projects.

Docs

No doc states the pins row's merged-away sentence or the shape currency row's list of counted states, so README.md, AGENTS.md, the handbook and docs/cli.md are unchanged.

Lands after

🤖 Generated with Claude Code

Summary by Sourcery

Make shape diagnosis refuse safely when upstream data is unavailable and accurately reflect update-shape refusal behavior and state counts.

New Features:

  • Detect partial upstream clones that cannot safely answer shape comparisons and provide a refetch command for users to resolve them.
  • Preserve pin-gate findings when update-shape reports REFUSAL AHEAD conditions that prevent the proposed apply from removing merged-away rows.

Bug Fixes:

  • Distinguish unreadable upstream files from files that are absent, preventing failed reads from being misclassified as upstream changes or both-sided changes.
  • Ensure pin findings do not claim that a refused apply will drop rows.

Enhancements:

  • Report every non-zero update-shape state in shape currency counts, keeping the output aligned with the states check classifies.
  • Recognize git promisor remotes and partial-clone configuration consistently with Git's behavior.

Tests:

  • Add coverage for partial-clone detection, non-fetching behavior, unreadable upstreams, promisor configuration, REFUSAL AHEAD handling, hard-linked pins, and comprehensive state counts.

Brett Heap and others added 3 commits October 16, 2026 17:51
…standard, the `shape currency` row asks without fetching whether the clone holds what the comparison reads, and is CANNOT ANSWER with a fetch for a person to run where it does not, and a read that fails is refused, never a copy called `both` (#683)

Measured again on main de92614 with git 2.43: run from a
`--filter=blob:none` clone of the standard, the doctor made five lazy
fetches, one under each `git show` of a copy the standard changed since
the pin (missing objects at the pin 1255 -> 1250), and with the clone's
remote out of reach it said `DRIFTED (5 both)` of copies nobody edited;
`update-shape.py check` printed the same five `both` rows.

shape-doctor.py:
- `partial_clone_row`: where `partial_clone` holds, `objects_lacking`
  asks `git rev-list --objects --no-walk --missing=print` of the pinned
  commit and of HEAD, before `prepare` or any `ls-tree` reads them. Where
  either lacks an object, the row is CANNOT ANSWER (`environment`, exit
  3, error `shape-doctor-partial-clone`), and its NEXT is
  `git -C <shape> fetch --refetch --no-filter <promisor remote>`.
- `promisor_remotes`: git 2.43's own reading of a promisor remote.
  `promisor = false` alone is no longer partial (git fetches nothing
  there), a `partialclonefilter` is (git fetches there whatever
  `promisor` says), and `extensions.partialclone` names one.
- `check_shape_currency`: a `prepare` refusal of a read that failed
  (`update-upstream-unreadable`) gets a `why` of its own and no
  `fetch --all`, which would bring nothing to read it.
- `as_the_standard_wrote_it` takes that refusal as "not proved", as it
  took a None.

update-shape.py:
- `Upstream.blob` reads with `git cat-file blob`, and where that fails,
  `files_at` (one `ls-tree -r` per revision) says whether the revision
  lists a file at the path: None where it does not, a directory included,
  and `update-upstream-unreadable` where it does, so `check` and `apply`
  refuse rather than print `both`.

tests/test_doctor_partial_clone.py: new; nothing is fetched where
`rev-list --missing=print` lists the same objects before and after.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
One conflict, in shape-doctor.py's EXIT CODES list, exit 3: this branch
added a partial clone lacking what the comparison reads (#683), and main
added a validator that crashed or cannot be given what it needs (#278,
#410) and a naming policy that does not load (#678). All are kept, in
main's form.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… check` says REFUSAL AHEAD beside a merged-away row, so it no longer says that the `apply` `check` prints drops the row when that `apply` is refused, and the `shape currency` counts name every state `check` counts (#676)

The `pins` row: `with_merged_away_rows` named `check`, and said the
`apply` it prints drops each merged-away row, wherever the gate's only
findings were merged-away rows and neither the pin nor the manifest is a
link. A pinned COPY kept as a link is `copy-is-link` to `check`, which
says REFUSAL AHEAD over it, and its `apply` exits 2 writing nothing; so
does `both`, `upstream-removed`, `unmapped`, and a file `apply` would
rewrite that has a hard link. `refusals_ahead` now asks `check`'s own two
questions over the rows `prepare` classifies (`Row.is_conflict`, and
`hard_linked_rewrites`). Where either names a file, the row keeps the
gate as its NEXT, names what `check` reports, and leaves out the sentence
about `apply`. Where the doctor cannot ask -- the pin does not read, the
checkout is a partial clone short of what the comparison reads
(`partial_clone_row`, #683), or `prepare` refuses -- it keeps the gate
too, so it never fetches.

The `shape currency` row: the reason printed the five original states
and `merged-away` only, so a `copy-is-link`, `copy-missing`,
`upstream-removed`, `unmapped` or `already-at-target` row stood under no
count. Every non-zero state of `update-shape.py`'s `ORDER` now follows
the five, in that order, so a root with none of them reads as before.

Stacked on #742 (#683): `refusals_ahead` uses its `partial_clone_row`.

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 corrects merged-away pin guidance by modeling update-shape.py's refusal conditions, expands shape-currency counts to every nonzero check state, and adds defensive partial-clone handling so the doctor never fetches or reports false drift when comparison objects are unavailable.

Flow diagram for partial-clone safety checks

flowchart TD
    A[check_shape_currency] --> B[read_pin]
    B --> C{partial_clone}
    C -- no --> D[prepare]
    C -- yes --> E[objects_lacking]
    E --> F{objects missing}
    F -- yes --> G[partial_clone_row]
    G --> H[CANNOT ANSWER; person runs fetch --refetch --no-filter]
    F -- no --> D
    D --> I{upstream readable}
    I -- no --> J[update-upstream-unreadable]
    I -- yes --> K[classify rows and count states]
Loading

File-Level Changes

Change Details Files
Keep the pins finding gated when update-shape.py check reports REFUSAL AHEAD, instead of claiming the generated apply will drop the merged-away row.
  • Added a second preparation-based check for conflicting rows and hard-linked rewrites that cause apply refusals.
  • Preserved the gate and added structured refusal_ahead details when refusals are found or the doctor cannot safely ask.
  • Updated reason text and documentation to distinguish refused applies from successful re-pin drops.
shape-doctor.py
tests/test_update_shape_merged_away.py
Make shape currency report all nonzero states counted by update-shape.py, not only the original five states and merged-away.
  • Emit nonzero states from update-shape.py's ORDER after the always-present states.
  • Added coverage ensuring printed counts sum to all classified rows, including copy-is-link.
shape-doctor.py
tests/test_update_shape_merged_away.py
Prevent partial-clone reads from fetching implicitly or producing misleading drift classifications.
  • Detect promisor remotes and inspect pinned and standard commits with rev-list --missing=print before comparison.
  • Return a CANNOT ANSWER shape-currency row with a person-run --refetch --no-filter command when required objects are missing.
  • Distinguish missing files from unreadable upstream files using git cat-file and ls-tree, raising update-upstream-unreadable for failed reads.
  • Cover blob and tree filters, promisor configuration variants, no-fetch behavior, and recovery commands.
shape-doctor.py
update-shape.py
tests/test_doctor_partial_clone.py

Assessment against linked issues

Issue Objective Addressed Explanation
#676 Make the pins row’s explanation and NEXT command reflect update-shape.py check’s refusal state when merged-away rows coexist with other conflicts, rather than claiming a refused apply would drop pin rows. ✅
#676 Make the shape currency summary count every non-zero state in update-shape.py’s ORDER, while continuing to print the five established states even when their counts are zero. ✅

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

@sonarqubecloud

Copy link
Copy Markdown

@brettheap

Copy link
Copy Markdown
Contributor Author

(lane openRepoShape-3, 2026-10-16T23:33:01Z) Lane 3's independent review of #751 at b7312258, stacked on #742 (RV-f, ruling 8): READY. Two test gaps (surviving mutants) go to a v1.1.1 follow-up. It lands after #742, once its Linux tests job is green.

#676's claims hold on main and on #742's head:

  • check says REFUSAL AHEAD … AGENTS-shape.md, while the pins row says the apply it prints "drops its pin row".
  • The counts read 11 over 12 rows.

On the head, the row agrees with check in every state RV-f built. Each root has a merged-away shape/Makefile, and the last column is the printed apply, pasted:

root check says pins row counts the pasted apply
merged-away only nothing ahead NEXT check, unchanged / 1 merged-away exit 0, row dropped
a copy kept as a link AGENTS-shape.md gate kept, copy-is-link … / 1 copy-is-link exit 2 update-rewrite-symlink
the pin hard-linked the pin gate kept, hard-link / 1 merged-away exit 2 update-rewrite-hardlink
an unmapped row extra/notes.txt gate kept, unmapped … / 1 unmapped exit 2 update-conflict
both scripts/validate-pins.py gate kept, both 1 both exit 2 update-conflict
upstream-removed AGENTS-shape.md gate kept, upstream-removed … / 1 upstream-removed exit 2 update-conflict

Other checks

Lane: openreposhape-3 (openRepoShape-3)

@sourcery-ai

sourcery-ai Bot commented Oct 17, 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 1 day and 1 hour by commenting @sourcery-ai review. Upgrade to get a review now.

@brettheap
brettheap merged commit e313701 into main Oct 17, 2026
5 checks passed
@brettheap
brettheap deleted the l3-676-pins-merged-away branch October 17, 2026 00:41
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>
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