Repository navigation
fix(review-items): Close only the Review items that a thread reply answers - #2525
Merged
Merged
Conversation
A typed reply in a thread closed every open action item that the person could answer in that Conversation, and saved the same text as the answer to each. A reply cannot show which question it answers, so close an item only when it is the only open one the person can answer there. Co-Authored-By: David Cramer <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Member
|
Really it should be closing the item thats related right? That could be a small classifier if it needed to be? e.g. our deciison api could run over each open item, and determine if the reply closes that? |
Replace the one-open-item rule with a decision model check. One decision call judges each open item that the replier can answer, so a thread reply closes only the items it answers and the others stay open. A failed decision keeps every item open; the agent still gets the reply. Co-Authored-By: David Cramer <[email protected]>
sentry-junior
Bot
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2dc8561. Configure here.
…lock The decision call ran while Slack ingress held the thread lock, so a slow call blocked later messages in the thread. Run the close after the lock is released, and give the decision call a 10 second timeout.
dcramer
reviewed
Nov 2, 2026
…fake Reply matching now calls completeDecision directly. The Slack and dashboard services no longer take a review-item decision service, and the matcher throws on failure so closeReviewItemsAnsweredByReply owns the fail-open. A router eval (review-item-replies.eval.ts) checks the exact items that a reply answers against the real decision model. The integration test drops its regex fake and covers only the rules that need no model choice: who may answer, and a retry of a reply that already closed an item. Co-Authored-By: David Cramer <[email protected]>
Contributor
|
1 screenshot change — 1 changed · 0 added · 0 removed Review screenshots in Frameshift
|
The reply match is a decision model classifier, not turn routing. Move its eval to evals/classifiers with its own config, global setup, and CI workflow, and restore the router suite to turn routing only. Co-Authored-By: David Cramer <[email protected]>
dcramer
approved these changes
Nov 2, 2026
This branch was successfully deployed
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
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.
A person answers one Review item in a thread, and every other open item for them in that thread closes too.
closeReviewItemsAnsweredByReplyclosed every open action item that the replier could answer in the Conversation, and saved the same reply text as the answer to each. Since #2506, a plain reply in a subscribed thread runs this path too. So one typed reply cleared all open questions with the wrong answers.Now the decision model checks the reply before anything closes (
review-items/reply-match.ts). OnecompleteDecisioncall (the same decision model as the passive reply router, promptjunior.review_item_reply_match) asks oneanswers/unrelatedquestion for each open item that the replier can answer. Only items withanswersat p >= 0.7 close. If the decision fails or takes more than 10 seconds, all items stay open and the agent still gets the reply. In Slack, the match runs after the ingress lock is released, so a slow decision does not block the thread. TheaddReviewItem/ update tool descriptions andreview-items/AGENTS.mddescribe this.The matcher calls
completeDecisiondirectly. No Slack or dashboard service takes a review-item decision function.Tests and evals. Which items a reply answers is a model choice, so it is tested in evals, not with a fake:
classifierseval suite (evals/classifiers/**, hard pass/fail, own config, global setup, andevals-classifiers.ymlworkflow).review-item-replies.eval.tschecks the exact set of items that a reply answers with the real decision model: picks an option, acknowledgement, follow-up question, one of two similar questions, both questions, a text answer, and an unrelated message. The workflow runs when the suite, the harness,reply-match.ts, orpi/client.tschanges.Card clicks and desk submits were not affected: they close only their own item and do not use the reply path.
Remaining risk: each thread reply makes one decision-model call while the replier has open items in that thread.
via @dcramer.
--
View Session [Sentry]