Repository navigation
Trials P-33 and P-34: which issue a pull request closes, and when an issue's content last changed (#184) - #241
Conversation
…issue's content last changed (#184) Two fixtures from the canary, through the Maintainer's login: P-33 passes, with the merged case on main after the ruleset was disabled for one merge and restored, and a base that is not the default carrying no reference; P-34 passes, with the body's time in lastEditedAt and the title's in the timeline. Four rules of the issue world are grounded. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ppened, the conditions as conditions, and a query with a fragment read The review's fragment finding: the test that holds a cost measurement's repos read a GraphQL fragment's fields as root fields and failed; it now skips a fragment definition. P-33's restore and merged read are in time order; the decision of 2026-10-20 moved from the pass condition to the exchange's call; P-34's client log moved likewise; the grounding comment names the reverse read's disagreement on a closed pull request. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 48 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (2 skipped: 2… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly identifies trials P-33 and P-34 and summarizes their main findings about pull-request closing issues and issue content-change timing. Linked Issues check Issue #184coding requirements are met.trials/P-33.jsonrecords closing references and authors for default-branch draft, open, merged, and closed-unmerged pull requests. It records the no-reference…Out of Scope Changes check The changes stay within issue #184. The GraphQL root-field parser fix supports reading the P-33 cost-measurement query. The added test verifies the P-33 and P-34 grounding claims. The fixture files an…Full details: Linked Issues check
-
Explanation
Issue
#184coding requirements are met.trials/P-33.jsonrecords closing references and authors for default-branch draft, open, merged, and closed-unmerged pull requests. It records the no-reference result for non-default bases and draft-state actors and times. It records the §6 fallback and design issue#240.trials/P-34.jsonseparates body and title content times fromupdatedAtand records the reopening actor.sim/grounding.pylinks all four rules to the fixtures.tests/test_tools.pyverifies the fixture content, ordering, states, actors, and grounding. The reportedpython3 -m tools.check_trialsrun passes.
Full details: Out of Scope Changes check
-
Explanation
The changes stay within issue
#184. The GraphQL root-field parser fix supports reading the P-33 cost-measurement query. The added test verifies the P-33 and P-34 grounding claims. The fixture files and grounding declarations implement the linked trial requirements. No unrelated product behavior or unrelated files appear in the reviewed change summary.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (2 skipped: 2 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Comment @coderabbitai help to get the list of available commands.
…ontent, and the closing-reference rule qualified Muse on the branch: a fixture emptied or inverted left every check green with its rules still grounded. A test now holds the content that grounds each of the four rules, and the rule on the closing reference says it is read on a pull request to the default branch, and that one to another base carries none. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9089b02eab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex on the pull request: a fragment on Query spread at the root could carry a root field the reader of a cost measurement's repos would not see. Such a fragment now fails the reader, as a second operation does; a fragment on another type is still skipped. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Closes #184. By the working ruling on the issue (comment of 2026-10-19), through the trial skill, on the canary alone, as the Maintainer's own
ghlogin; no token minted, no App, no workflow.P-33,
trials/P-33.json: pass. One issue (#36 on the canary), and five pull requests whose descriptions each saidCloses #36: left a draft (#37); created a draft, marked ready, converted back, marked ready again (#38); merged to a base that is not the default (#39); closed unmerged (#40); open to a base that is not the default (#41). What GitHub answered, in the read the port will make:closingIssuesReferencesReadyForReviewEvent×2 andConvertToDraftEvent×1 on the timeline, each with its actor and timestate: CLOSEDCOMPLETEDwithcloserthe pull request on its timelineThe merged-on-main case: the canary's ruleset on
mainrequires an approving review the Maintainer cannot give their own pull request, so, on the Maintainer's decision of 2026-10-20 in this session, the ruleset's enforcement was set todisabledfor one merge and set back toactivefive seconds later; exchanges 3 and 4 record both, with the same conditions and rules before and after, and exchange 5 is the read that followed. The merge was a merge commit (gh pr merge --merge), which the active ruleset would have refused on three counts: it allows squash alone, needs an approving review, and needs the last push approved. One more observation the fixture holds: the reverse read from the issue (CrossReferencedEvent.willCloseTarget) saysfalsefor every pull request once the issue is closed, so the port reads the pull request's ownclosingIssuesReferences, never the issue's timeline, for the join.P-34,
trials/P-34.json: pass. One issue (#42), then a minute apart: the body edited, the title edited, a comment, a label, a close, a reopening.lastEditedAtanduserContentEditsmove for the body alone (21:16:22Z); the title's change is the timeline'sRenamedTitleEvent(21:17:28Z) with its actor;updatedAtis the reopening's (21:21:54Z). So the content's time is read apart fromupdatedAt, as the newer of two reads, which the port computes: here 21:17:28Z; andReopenedEventnames the Maintainer and when. The first body edit leaves twouserContentEditsnodes, the older carrying the original content's time. The outsider's reopening was not tried: no outside account exists (#163's ruling), and the event's shape is the same with another actor, whose access P-32 reads from the association.What the design learns is #240, "Design: what P-33 and P-34 showed of closing references, an issue's content time and a timeline's count": the non-default base (§6's fallback), the reverse read,
lastEditedAtfor the body alone,timelineItems.totalCountignoringitemTypes, and the close trailing the merge by a second. No design text changes here.Grounding. Four cells in
sim/grounding.py:a_closing_reference_is_read_from_any_pull_requestandwho_changed_the_draft_flag_is_readto P-33,an_issues_content_time_is_read_apartanda_reopening_names_its_actorto P-34, each with a comment on what the fixture also shows.python3 -m tools.check_trialspasses; no record changes.Removed. The
authorizationheaderghsent on each exchange, named inremoved; the response headers off the allowlist, taken out bytools/redact_trial.pybefore anything was read. Nothing else.Undone on the canary (step 8). The six branches
trial/p-33-*deleted; pull requests #37 and #41 closed (#39 and #38 merged, #40 closed by the trial); issues #36 (closed by the merge) and #42 closed. The ruleset isactivewith its rules as they were. Left in place: the merge commit be2218f of #38 on the canary'smain, with one filetrial/p-33/open.md, which the ruleset's non-fast-forward rule keeps and a revert would need the ruleset disabled again; and the closed issues and pull requests, which hold nothing unredacted.Reviews, one fix round on 7dbc787: fresh-context Claude review and security pass, AGY (gemini-3.8-flash) and Muse. Taken: P-33's query used a GraphQL fragment that
tests/test_tools.py's reader of a cost measurement's roots read as root fields, somake verifyfailed on the branch; the reader now skips a fragment definition (the smaller fix; re-running the read could not reproduce #38 as open). The exchanges are in the order they happened (the restore before the read). The decision of 2026-10-20 and P-34's client log moved out of thepassconditions into the exchanges'call. The grounding comment names the reverse read's disagreement on a closed pull request. Muse, read late: a fixture emptied or inverted left every check green with its rules still grounded, sotests/test_tools.py::test_the_issue_fixtures_hold_what_grounds_the_four_rulesnow holds the content that grounds each rule, as the P-32 fixture is held; and the closing-reference rule's text says it is read on a pull request to the default branch, and that one to another base carries none. Not taken: aPassed onnote in §6's rows, which no fixture row carries; recording the setup acts as exchanges, whichgh pr create,gh pr readyandgh issue editdo not capture, so the acts are narrated in the exchanges'calland what GitHub holds of them is in the reads.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests