Skip to content

docs(agents): record PR-supersession and constant-change review discipline - #1909

Merged
seonghobae merged 3 commits into
mainfrom
docs/agents-pr-supersession-and-constant-review
Sep 5, 2026
Merged

docs(agents): record PR-supersession and constant-change review discipline#1909
seonghobae merged 3 commits into
mainfrom
docs/agents-pr-supersession-and-constant-review

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

Summary

My lane in the per-session split agreed with the other four concurrent sessions (peer 3 took verification discipline in #1907; peer 2 has gate/merge mechanics; host 1 has close-time diff comparison and noema concurrency; host 2 has CI failure diagnosis). Both rules below come from mistakes this session actually made and corrected today, not from general advice.

Verification

Every factual claim was checked against the repository rather than against the summary that introduced it:

  • All nine PR numbers/states confirmed via gh pr view.
  • docs/product-goal-directive.md section 8 quoted verbatim after grepping the file.
  • Parent-count distribution counted directly: git log origin/main -200 --format=%p | awk '{print NF}' → 153 single-parent, 47 two-parent.
  • fail_under = 100 / fail-under = 100 confirmed in pyproject.toml.
  • Full suite on the final text: 2883 passed, 1 skipped.

An earlier draft cited a §8.5–8.6 that does not exist in that file and attributed a "timeout defaults to null" sentence to it that appears only in #1891's PR body. Both were wrong and were caught by grepping. That failure is recorded in the added text itself, since it is the same error class the section warns about.

Rebased onto #1907; the two AGENTS.md sections sit side by side with no overlap.

🤖 Generated with Claude Code

…pline

Two rules from mistakes this session actually made and corrected, per the
per-session lane split agreed with the other concurrent sessions (peer 3
took verification discipline in #1907; peer 2 has gate/merge mechanics;
host 1 has close-time diff comparison and noema concurrency; host 2 has
CI failure diagnosis).

- Narrowing a PR does not carry its delta. #1871 was closed in favor of
  #1877 plus #1879; both successors were green, but neither carried the
  coverage/docstring delta, leaving main's required 100% gate broken
  until #1883 recovered it. "Each piece works" and "the pieces together
  cover the original's scope" are different questions.
- Compare content, not ancestry. main mixes squash and merge commits
  (last 200: 153 single-parent, 47 two-parent, counted directly), so
  `git merge-base --is-ancestor` gives false negatives for squashed
  deltas and false positives for reverted merge-commit deltas.
- Never endorse a timeout or retry constant on a model-invocation path
  without reading docs/product-goal-directive.md section 8, which accepts
  more than two hours per model and states speed is not a core
  consideration. #1889/#1890/#1892 each capped a model step at 900s on
  real multi-hour-hang evidence and were all reverted (#1891, #1895).

Every PR number, the section-8 quotes, the parent-count distribution, and
the 100% gate values were verified against the repository directly. An
earlier draft of the timeout bullet cited a section number that does not
exist and attributed a sentence to that file which appears only in
#1891's PR body; both were caught by grepping rather than trusting the
summary that introduced them, and that failure is recorded in the text.

Full suite: 2883 passed, 1 skipped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 49 seconds.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 06777a87-5202-4ecd-85f1-8eb3137d8203

📥 Commits

Reviewing files that changed from the base of the PR and between c4a89b3 and 3657939.

📒 Files selected for processing (2)
  • AGENTS.md
  • CLAUDE.md

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

seonghobae and others added 2 commits September 5, 2026 17:44
#1906 landed "Verifying a 'superseded — closing' claim", which already
covers ancestry-vs-content in more depth than my draft's version did
(three-dot diff, `git grep -lF`, `git show origin/main:<path>`, and the
squash/merge-commit mix that makes `git merge-base --is-ancestor`
unreliable in both directions). Removed my duplicate bullet and pointed
at that section instead.

What remains here is the part a single-PR closure check does not cover:
a PR split across several successors fails differently, because no
individual successor looks wrong. Added the union question explicitly,
and kept the end-to-end check — git-level commands show whether text
moved, not whether behaviour is restored.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@seonghobae
seonghobae merged commit 8aea813 into main Sep 5, 2026
3 of 12 checks passed
@seonghobae
seonghobae deleted the docs/agents-pr-supersession-and-constant-review branch September 5, 2026 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant