fix(ce-resolve-pr-feedback): stop dropping unresolved and actionable feedback#617
Merged
fix(ce-resolve-pr-feedback): stop dropping unresolved and actionable feedback#617
Conversation
`isOutdated` means the diff hunk shifted, not that the reviewer's concern was addressed. Filtering on it dropped unresolved threads whose surrounding lines happened to move, silently hiding valid feedback (e.g., PR #616 comment on src/targets/codex.ts:267). Filter on `isResolved` alone and surface `isOutdated` through dispatch so the resolver agent can handle line drift explicitly. The agent's rubric now bounds relocation to a single in-file anchor search: found -> re-evaluate there; not found with in-place code described -> `not-addressing`; not found with extraction suggested -> `needs-human`. No repo-wide grep, since the reviewer's surrounding context is gone and silently patching the wrong usage site is worse than asking.
tmchow
force-pushed
the
tmchow/resolve-pr-filter-bug
branch
from
6f2243c to
446086e
Compare
tmchow
changed the title
![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: 560922d4ba
ℹ️ 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".
The bot filter pre-dropped all top-level content from AI review bots (CodeRabbit, Codex, Gemini, Copilot) on the premise that these bots only post actionable feedback in inline review threads. That premise was wrong: Codex sometimes posts findings as a top-level PR comment with no inline thread counterpart. A source-level heuristic to separate wrapper from actionable (priority badges, blob permalinks) just shifts the brittle assumption one level down -- one bot format change away from silently dropping feedback again. Move the wrapper drop to SKILL.md step 2, which already has a content-aware actionability check with a Silent Drop rule (and already lists "Here are some automated review suggestions..." as an example to drop). The script now only pre-filters structurally-non-actionable CI/status bots like Codecov whose output shape is stable. Net: small, bounded token cost for reading bot wrappers in exchange for robustness to bot format changes. No silent data loss path.
tmchow
force-pushed
the
tmchow/resolve-pr-filter-bug
branch
from
560922d to
7513fd0
Compare
tmchow
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7513fd0fd3
ℹ️ 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".
…tdated flag Outdated and file-level review threads often have `line == null`, and targeted mode's thread lookup didn't return `isOutdated` at all. Both gaps defeat the relocation handling added earlier on this branch -- the resolver loses its anchor for null-line threads and doesn't know to run the same-file anchor search in targeted mode. Two changes: - `get-pr-comments` and `get-thread-for-comment` now return `originalLine`, `startLine`, `originalStartLine`, and (for the targeted script) `isOutdated`. Dispatch in both full and targeted modes passes these through to the resolver. - The resolver rubric tries location fields in order (`line` -> `startLine` -> `originalLine` -> `originalStartLine`) before falling back to the in-file anchor search, so outdated/file-level threads still have an anchor to start from.
This was referenced Apr 21, 2026
Merged
Closed
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two filter bugs in
ce-resolve-pr-feedbacksilently dropped review feedback from the skill's view. Both surfaced on the same PR (#616) — one inline review thread and one top-level Codex comment, neither of which the skill processed. The common failure mode: source-level filters encoded assumptions about what "must" be non-actionable, and the assumptions were wrong.Bug 1: Outdated-but-unresolved review threads were filtered out
The script required
isResolved == false AND isOutdated == false. GitHub marks a threadisOutdatedwhen the surrounding diff hunk shifts, not when the reviewer's concern is addressed, so a reviewer's still-live comment disappeared from the skill's view as soon as any nearby code was edited. Live example: the P1 comment on EveryInc/compound-engineering-plugin#616.Fix: filter on
isResolvedalone. TheisOutdatedflag flows through dispatch so the resolver agent knows to expect line drift. The agent rubric now bounds relocation to a single in-file anchor search — found → re-evaluate there; not found with in-place code described →not-addressingwith evidence; not found with extraction suggested →needs-human. No repo-wide grep, because when cross-file extraction has happened the reviewer's surrounding context is gone and silently patching the wrong usage site is worse than asking.Bug 2: AI review bots were pre-filtered on a bad premise
The script pre-dropped all top-level content from CodeRabbit, Codex, Gemini, and Copilot on the premise that these bots only post actionable feedback in inline review threads. Codex broke that: it sometimes posts findings directly as a top-level PR conversation comment with no inline thread counterpart. Live example: the P1/P2 Codex comment on EveryInc/compound-engineering-plugin#616, hitting
src/targets/codex.tsandsrc/commands/cleanup.ts.A content-signal heuristic (priority badges, blob permalinks) would fix the symptom but shift the brittle assumption one level down — one bot format change away from silently dropping feedback again.
Fix: move wrapper handling to the content-aware layer that's already there. SKILL.md step 2 has an Actionability check with a Silent Drop rule, and already lists
"Here are some automated review suggestions..."as an example to drop. The script now only pre-filters structurally-non-actionable CI/status bots (Codecov), whose output shape is stable. AI review bot content flows through; the agent recognizes wrappers by content and drops them silently.Trade-off: a small, bounded token cost for reading bot wrappers in exchange for robustness. No silent data loss path. If a bot's output format changes tomorrow, the content-aware layer adapts; a regex-based heuristic would not.
Filter shape, before and after
isOutdatedflag