Repository navigation
Check other people's knowledge PRs daily and tell them why (design v8) - #229
Conversation
A knowledge PR that someone else opened is never merged until it passes the gate. Its author is told why in ONE comment, and the PR is checked again once a day, not on every push. - poll_external checks each PR at most once per 24 hours. The per-PR time (external_checked:<PR>) is written as soon as that PR was checked, so a restart or a pass that fails half-way never checks a PR twice in a day; force=True ignores the limit. Each daily pass judges every open knowledge PR again unless its current head already passed and is in the merge flow. Heads that did not pass (failed, human, gate_failed, stale context, head changed, superseded, rebuild_failed) are judged again. A calibration_required head still waits for a calibrated judge (no paid calls). - After every judgement, including one sent to people and a local-gate refusal of an external PR, the service issues a post_findings item. The comment carries the result, the checked head, and every problem (L1 issues, L2 reasons, consistency conflicts, gate reasons). It is never issued in shadow mode or for a private upstream. A PR spanning several repositories is told through one of them that publishes and auto-merges, and not at all if any of its scopes has a private upstream. - publisher post_findings: keeps ONE comment per PR, the one of its own (checked by gh api user login) that carries the marker. That comment is edited in place; otherwise one is created. A marker comment someone else wrote is never edited. A failed post is retried every round, never acked as failed. Each findings item carries a revision (its issue time), and the publisher remembers the latest one it posted per PR, so an older item retried later never overwrites a newer comment. - Delivery is durable: the service keeps the latest findings per PR in the ledger (findings:<PR>, ONE global record per PR whatever repository scope it touches) until the publisher's ack confirms them. When an undelivered item expired it reissues the same text with its ORIGINAL revision, so a renewal can never overtake newer findings, whatever the PR's status. The publisher writes its revision record atomically and treats an unreadable one as absent. New: Ledger.cursors_with_prefix. - A failing external PR no longer goes to people's queue: its author fixes it. Setup refusals still go to people. - refine.reasons_from_decision is shared by refinement and findings. Tests: once a day, not on every push (cursor and next-day run); one findings comment says why; a local-gate refusal of an external PR tells the author instead of queueing people; the findings comment is created once, then edited in place, and a spoofed marker comment is never touched; a pass that fails half-way does not re-check finished PRs; a PR spanning repositories is told through a public one, and not at all with a private scope; a failed comment post is retried; an older findings item retried after a newer one never overwrites it; undelivered findings are issued again after they expire and stop once delivered; renewed findings keep their revision across a scope change; an unreadable revision record does not block findings. The external tests now run one pass per day, and failing heads are checked again the next day. No migration of per-repository findings records is needed: that layout existed only in an earlier, unmerged revision of this same change, and was never on main or deployed, so no such records exist anywhere. Co-Authored-By: Claude Opus 5.5 <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. |
![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: f6c5b1a5e3
ℹ️ 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".
| publisher confirms delivery. ``revision`` is when the findings were | ||
| written: the publisher drops an item older than what it already posted, | ||
| and a renewal keeps the original revision, so it can never overtake newer | ||
| findings.""" |
There was a problem hiding this comment.
Using the wall clock as the ordering token does not ensure that later findings have a larger revision. If the clock moves backward after a VM restore/manual adjustment, the publisher treats the new findings as older at _do_post_findings and leaves the previous comment in place; equal timestamps can likewise let processing order decide which text wins. This can leave a passing comment visible after a later gate failure until the clock surpasses the old value, so revisions should come from a durable per-PR monotonic sequence rather than rt.clock().
Useful? React with 👍 / 👎.
| if not _due(rt, number, force): | ||
| continue # checked today already: once a day, not on every push |
There was a problem hiding this comment.
The new daily guard is ineffective for a PR carrying kb:human-approved without a current maintainer approval: after passing this guard, _approvals may fetch up to 50 pages and then returns without calling _checked. Since the scheduler polls every intake interval, such a PR is checked again every 15 minutes rather than once per day, repeatedly consuming GitHub API quota until approval arrives. Record the check before that wait path as is done for the other completed outcomes.
Useful? React with 👍 / 👎.
PR 3 of the v8 knowledge-service change: a knowledge PR someone else opened is never merged until it passes the gate. Its author is told why in ONE comment, and the PR is checked again once a day, not on every push.
poll_externalchecks each PR at most once a day. The per-PR time is recorded in the ledger as soon as that PR is checked, so a restart or a half-failed pass never re-checks it. Heads that did not pass are judged again the next day; a calibration-blocked head waits.post_findings.Commit hook: two rounds of findings fixed. The last one, a migration of per-repository findings records, was declined: that layout only existed in an earlier, unmerged revision of this same change, so no such records exist. Full suite and SPEC freshness pass locally.
🤖 Generated with Claude Code