[brettheap]

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

Closes #752.

Lands after the v1.1.0 tag. (The issue's "Lands after: #736" is met: #736 landed as bf1f099b, this branch's base.)

What happened

A conflicted shape pin's NEXT took --ours wherever ours held a pin, whether it read or not, and --theirs only where ours held none. A side whose committed pin does not read, taken whole, left the pin as it was, so the doctor named the same checkout again on every run.

Reproduced on #736's head 3dfe2fde and on main bf1f099b's tree (f596d9cd, the predicted squash, equal to the landed one). In both_pure's merge:

• Stage 2 restaged as files: [ (RV-b's state): the NEXT was checkout --ours -- contracts/shape-pin.yaml. Run as printed, the doctor printed the same NEXT again.
• Both stages restaged that way: the same --ours NEXT, again after it ran.
• Ours removed and theirs not reading: the NEXT was checkout --theirs, a pin that does not read.

The issue's claim holds on current main.

What changed (shape-doctor.py)

• side_to_take (:4125-4174), called by pin_restore (:3957-3959), picks the side. It takes ours where ours' pin reads, else theirs where that reads.
  • Where both sides read, the why starts as before: "--theirs takes the other side. Take one side of the pin whole".
  • Where only one side holds a pin, it says, as before: "only that side of the merge holds a pin. Take it whole".
  • Where both sides hold one and only one reads, it says "only that side's pin reads as one, and the other side taken whole would leave a pin that does not read. Take it whole". It never offers the other side, which would offer the circle.
• No line where no side's pin reads. pin_restore returns ("unmerged", None, why). The why says that no side taken whole puts back a pin that reads, and that this doctor would name the same line again. It ends: "A pin is never mended by hand, so no line is named here: whether to finish the merge, or to abort it and first put back the pin a side broke, is a person's call". When only one side holds a pin, it says "the one side of it that holds a pin holds one that does not read".
• The callers keep that why and name no NEXT:
  • unreadable_pin_row (:4638) keeps its update-shape.py check line for the none restore alone.
  • unreadable_next (:4561-4590) keeps the why of a pin left stuck that way, instead of sending the reader to this doctor again with UNREADABLE_BY_HAND. That text would say no tool-written file has a pin that reads in its history. The manifest kinds row therefore prints what the shape currency row prints. Its return type now admits a None command.
• other_sides_copies (:4085-4121) loses shape-doctor.py: a conflicted shape pin's NEXT reads both sides' pins, so a copy conflicted beside it that only the side not taken has a files: row for is named in its why and left_for_a_person for a person to merge, and never taken (#687) #736's kept-pin-does-not-read wording. The pin taken here now always reads. Its words are otherwise unchanged; I checked the old kept_reads=True concatenation against the new one.
• Docstrings: pin_restore's unmerged entry (:3773-3782), EVERY PIN … IS READ FIRST (:3884-3886), and the two callers'.

Tests (tests/test_doctor_pin_restore_merges.py)

• test_the_side_taken_is_one_whose_pin_reads[ours-does-not-read|theirs-does-not-read] (:826):
  • the NEXT is checkout --theirs or --ours respectively, and nothing is taken (the proof reads both pins);
  • both rows give the Makefile, project.yaml and bootstrap.py to a person;
  • the why starts with the new note;
  • run as printed, the pin in the tree reads: the shape currency row carries no read error, and the manifest kinds row no longer lists the pin.
• test_no_line_is_named_where_no_sides_pin_reads[both|one] (:864): both rows have next: null and the same why, quoted whole, and both rows' restore is unmerged.
• shape-doctor.py: a conflicted shape pin's NEXT reads both sides' pins, so a copy conflicted beside it that only the side not taken has a files: row for is named in its why and left_for_a_person for a person to merge, and never taken (#687) #736's plural test no longer has an [unreadable] case. That state is shape-doctor.py pin_restore: where the side taken in a conflicted shape pin does not read as a pin, the NEXT still takes that side, and run as printed it leaves the doctor naming the same NEXT again #752's, and its expectations are the first test's now.
• Failed before the fix. All four new cases failed on main's doctor (stash of shape-doctor.py only): with --ours or --theirs NEXTs where none or the other was expected, and with "--theirs takes the other side".

Each run used a stub gh first on PATH that exits 97, GH_TOKEN=bogus and an empty GH_CONFIG_DIR, at the head:

/usr/bin/python3 -m pytest -q -p no:cacheprovider --basetemp=<scratch>/bt -o tmp_path_retention_policy=failed tests/test_doctor_pin_restore_merges.py tests/test_doctor_next_is_a_command.py tests/test_repo_hygiene.py tests/test_cli_reference.py
181 passed in 108.85s (0:01:48)

/usr/bin/python3 -m pytest -q -p no:cacheprovider --basetemp=<scratch>/bt-sd -o tmp_path_retention_policy=failed tests/test_shape_doctor.py
436 passed, 3 skipped, 1 xfailed in 346.17s (0:05:46)

Pinned shape copies

None. shape-doctor.py is in no copy list.

Docs

None changed. No doc describes the conflicted-pin restore. README.md, AGENTS.md and docs/cli.md are untouched, and so is docs/release-notes/.

For the reviewer

• The why points to two acts without naming them as lines. Where no side's pin reads, the way out is a person's: finish the merge, or abort it and put back the pin a side broke. On a branch, the pin a commit broke is the committed restore this doctor already names. The why mentions merge --abort and that restore only as acts, not as NEXT lines.
• shape-doctor.py pin_restore: a shape copy renamed on one side of a merge conflicts under its new name, which no pin's row lists, so the why never names it and only the merge row does #753 (a copy renamed on one side) stacks on this branch. Its PR changes the same lines of pin_restore.

🤖 Generated with Claude Code

Summary by Sourcery

Fix conflicted shape-pin remediation so generated NEXT commands leave a readable pin and unresolved cases are deferred to the user.

Bug Fixes:

• Ensure conflicted shape pins are restored from a side whose pin is readable, preventing the doctor from repeatedly recommending the same command.
• Avoid suggesting a remediation command when neither side of a conflicted pin contains a readable pin.

Enhancements:

• Propagate merge-resolution explanations consistently across shape-currency and manifest-kinds results.
• Simplify conflicted-copy messaging now that the selected pin is always readable.

Tests:

• Add coverage for selecting the readable side of a conflicted pin and for leaving the NEXT unset when no side is readable.

[sourcery-ai]

Reviewer's Guide

Updates conflicted shape-pin restoration so the doctor only checks out a side whose pin reads, and names no NEXT when neither side is readable; output propagation, copy messaging, documentation, and regression tests are adjusted accordingly.

Sequence diagram for readable conflicted-pin restoration

sequenceDiagram
    participant Doctor
    participant GitIndex
    participant PinReader
    Doctor->>GitIndex: ls-files -u
    GitIndex-->>Doctor: Pin stages
    Doctor->>PinReader: pin_reads_at(:2, pin)
    PinReader-->>Doctor: Readable or unreadable
    Doctor->>PinReader: pin_reads_at(:3, pin)
    PinReader-->>Doctor: Readable or unreadable
    alt Ours reads or both read
        Doctor-->>Doctor: Return --ours and stage 2
    else Only theirs reads
        Doctor-->>Doctor: Return --theirs and stage 3
    end
    Doctor-->>Doctor: Include explanatory why

Loading

Sequence diagram for unreadable conflicted-pin reporting

sequenceDiagram
    participant Doctor
    participant PinRestore
    participant ShapeCurrency
    participant ManifestKinds
    Doctor->>PinRestore: pin_restore(detail)
    PinRestore-->>Doctor: unmerged, no command, why
    Doctor->>ShapeCurrency: Keep no NEXT and explain person's call
    Doctor->>ManifestKinds: Propagate the same why
    ManifestKinds-->>Doctor: No fallback shape-doctor command

Loading

Flow diagram for conflicted shape-pin restoration

flowchart TD
    A[Conflicted shape pin] --> B{Which side's pin reads?}
    B -->|Ours reads| C[Choose --ours]
    B -->|Only theirs reads| D[Choose --theirs]
    B -->|Both read| E[Choose a side and explain alternative]
    B -->|Neither reads| F[No NEXT command]
    C --> G[Return unmerged restore with checkout command]
    D --> G
    E --> G
    F --> H[Return unmerged restore with person's-call explanation]

Loading

File-Level Changes

Change	Details	Files
Selects a merge side only when its conflicted pin is readable, and suppresses the NEXT command when neither side provides a readable pin.	Adds side selection based on pin_reads_at, preferring ours when readable and otherwise using readable theirs. Preserves the existing both-readable behavior and adds explanations for the single-readable case. Returns unmerged with a null command and a human-resolution explanation when no side's pin reads.	shape-doctor.py
Propagates no-command merge outcomes consistently through both doctor output rows.	Allows unreadable_next to return a nullable command and reuses the stuck pin's explanation instead of recursively directing users back to the doctor. Keeps the update-shape check command only for the separate none restore case. Updates docstrings and remediation text to document readable-pin requirements and human resolution.	shape-doctor.py
Simplifies conflicted-copy messaging now that any selected pin is guaranteed readable.	Removes the obsolete kept-pin readability parameter and unreadable-pin wording. Retains copy ownership and manual merge guidance for files conflicted beside the pin.	shape-doctor.py
Adds regression coverage for readable-side selection and unresolved unreadable-pin conflicts.	Verifies checkout chooses the opposite side when ours or theirs is unreadable and that the resulting pin reads after execution. Verifies both output rows use next: null, share the same explanation, and report unmerged when no side reads. Updates existing copy-conflict expectations to remove the obsolete unreadable case.	tests/test_doctor_pin_restore_merges.py

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#752	When a conflicted shape pin has at least one side whose pin reads, make the printed NEXT take a readable side rather than a side whose pin does not read.	✅
#752	When no side's conflicted pin reads, name no NEXT and explain in the why that neither side can restore a readable pin and the decision is for a person.	✅
#752	Add tests for both cases that run the printed NEXT as printed where applicable and verify the doctor's subsequent output does not repeat the same NEXT.	✅

Possibly linked issues

• shape-doctor.py pin_restore: where the side taken in a conflicted shape pin does not read as a pin, the NEXT still takes that side, and run as printed it leaves the doctor naming the same NEXT again #752: The PR fixes issue shape-doctor.py pin_restore: where the side taken in a conflicted shape pin does not read as a pin, the NEXT still takes that side, and run as printed it leaves the doctor naming the same NEXT again #752 by selecting a side whose pin reads, or naming no NEXT when neither side reads.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

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

[brettheap]

(lane openRepoShape-3, 2026-10-17T04:11:08Z) Independent review of #779 by RV-b (opus), on head 06eb23ef: READY. No MUST, SHOULD or NIT. It lands only after the v1.1.0 tag.

The issue's loop holds on main bf1f099b:

• with ours' pin as files: [, the NEXT is checkout --ours, and after it runs the doctor names the same NEXT again;
• likewise with both sides unreadable, with ours a YAML list, files: 5, or a mapping that is not a pin, and with ours deleted while theirs does not read.

On the head every state ends somewhere new. "After finish" means every conflicted path taken from the NEXT's side, then the merge committed.

state	NEXT	after NEXT	after finish
ours unparsable	checkout --theirs ("only that side's pin reads as one…")	the pin reads; NEXT update-shape.py check	COMPLIANT, SHAPE BEHIND
theirs unparsable	checkout --ours	the pin reads	COMPLIANT
ours a list / files: 5 / not a pin	--theirs	the pin reads	COMPLIANT, SHAPE BEHIND
both unparsable	none in either row, the same WHY
ours deleted, theirs reads	--theirs (unchanged from main)	no repeat	COMPLIANT, SHAPE BEHIND
ours deleted, theirs unparsable	none

What RV-b checked in the code:

• side_to_take (:4128, called at :3957) reads each stage through the same pin_at and check_pin as the rows.
• unreadable_next keeps the stuck WHY and names no command (:4584).
• unreadable_pin_row keeps update-shape.py check for the none restore only (:4638).
• Dropping shape-doctor.py: a conflicted shape pin's NEXT reads both sides' pins, so a copy conflicted beside it that only the side not taken has a files: row for is named in its why and left_for_a_person for a person to merge, and never taken (#687) #736's kept-pin-does-not-read wording is safe, because the side taken now always reads.

Mutants:

mutant	result
take a side whether it reads or not	4 failed
the manifest kinds row ignores the stuck WHY	2 failed
shape currency names check again	2 failed

Tests: the four targeted files gave 181 passed. The four new cases fail on main's doctor.

Merges and CI:

• clean with bf1f099b and with every open lane-3 v1.1.1 PR, pairwise;
• on all of them chained, nine doctor and pin suites gave 432 passed, and tests/test_shape_doctor.py gave 439 passed, 3 skipped;
• CI is green.

Lane: openreposhape-3 (openRepoShape-3)

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

[sourcery-ai]

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 hour and 39 minutes by commenting @sourcery-ai review. Upgrade to get a review now.