Skip to content

Thread task titles pull from reply body instead of root review comment (closes #470)#512

Merged
FidoCanCode merged 3 commits into
mainfrom
task-title-from-reply
Apr 14, 2026
Merged

Thread task titles pull from reply body instead of root review comment (closes #470)#512
FidoCanCode merged 3 commits into
mainfrom
task-title-from-reply

Conversation

@FidoCanCode
Copy link
Copy Markdown
Owner

@FidoCanCode FidoCanCode commented Apr 14, 2026

Fixes #470.

Thread task titles currently pull from the webhook reply body (often fido's own acknowledgment or a one-word "Yes") instead of the reviewer's actual feedback. This fixes title generation to use the root review comment body, which is already fetched via fetch_comment_thread.


Work queue

Completed (3)
  • Use root review comment body for thread task title generation
  • Use originating PR comment for task titles and edit last reply instead of reposting
  • Ensure reply/edit logic properly considers entire thread context

When a PR review thread fires a webhook on a reply (e.g. fido's own
'Woof, got it!' acknowledgment), task titles were being generated from
that reply body rather than the reviewer's original feedback. The root
comment already lives in the fetched thread — extract it and re-derive
titles from root_body when the triggering comment is a reply (ACT/DO
categories only; triage category still determined from the reply).
@FidoCanCode FidoCanCode force-pushed the task-title-from-reply branch from fb03270 to 2f457b5 Compare April 14, 2026 22:39
@FidoCanCode FidoCanCode marked this pull request as ready for review April 14, 2026 22:39
@FidoCanCode FidoCanCode requested a review from rhencke April 14, 2026 22:39
@rhencke
Copy link
Copy Markdown
Collaborator

rhencke commented Apr 14, 2026

This isn't quite right. The task comment should always reflect the PR comment (on file, line or pr level) that it was created from (with any modifications needed by retasking but still linking to its origin.

If Fido was the last replier and has more to say, it's better to edit the comment with the added context than post a bunch in a row.

@FidoCanCode
Copy link
Copy Markdown
Owner Author

Use originating PR comment for task titles and edit replies instead of reposting

Copy link
Copy Markdown
Collaborator

@rhencke rhencke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments

FidoCanCode added a commit that referenced this pull request Apr 14, 2026
…rd (closes #499)

When a prior turn's consume_until_result broke early on _cancel (webhook
preempted the worker), claude kept processing and emitted type=result to
stdout — but nobody read it.  The next caller's consume_until_result
then returned that stale result as its own, and every subsequent call
became off-by-one.  Three concrete repros on 2026-04-14:

- PR #498 test-comment reply: fido posted the previous worker turn's
  output ("Yep — already done and pushed") as the reply body.
- PR #512 review reply: fido posted a title candidate string verbatim
  ("Use originating PR comment for task titles...") with no in-voice
  content at all.
- PR #512 work queue: the next task title in tasks.json rendered as
  "Build schema loader with type validation in confusio" — lifted from
  an earlier status nudge's JSON output, not from the reviewer's actual
  comment.

Fix: track _in_turn on ClaudeSession; set on send(), clear on
type=result/error/EOF in iter_events.  When send() is called with
_in_turn still True, a control_request interrupt is written first and
events are read+discarded until a boundary, so the next turn starts on
a clean stream.  Deadline-bounded (10s) with restart-session fallback
if claude never closes the interrupted turn.

restart() and switch_model() both spawn fresh subprocesses, so they
reset _in_turn to False — next send() skips the drain path.
FidoCanCode added a commit that referenced this pull request Apr 14, 2026
…rd (closes #499) (#513)

## Summary
- Track `_in_turn` on `ClaudeSession`; set on `send()`, clear on
`type=result`/`type=error`/EOF
- On `send()` with `_in_turn` still True (prior turn cancelled without
draining), send `control_request` interrupt and read events until a turn
boundary — then write the new user message
- Deadline-bounded (10s) with session `restart()` fallback when claude
never closes the interrupted turn
- `restart()` and `switch_model()` spawn fresh subprocesses and reset
`_in_turn` so their next `send()` skips the drain path

Fixes the stream-leak class that produced three concrete misbehaviours
on 2026-04-14:
- PR #498 reply bleed ("Yep — already done and pushed" posted to a "Test
comment")
- PR #512 title-candidate posted verbatim as a reply
- PR #512 work-queue corrupted with a status-nudge JSON leak ("Build
schema loader with type validation in confusio")

## Test plan
- [x] 1774 tests pass, 100% coverage, pyright clean, ruff clean
- [ ] After merge: webhook-triage reaction + reply sequence works
without paragraph bleed
- [ ] After merge: task titles no longer pull from prior turns'
status/triage output

Closes #499.

Co-authored-by: Fido Can Code <190991155+FidoCanCode@users.noreply.github.com>
…f reposting

Task titles for ACT/DO categories now always come from _summarize_as_action_item
applied to the root comment body, regardless of whether the triggering webhook
fires on the root or a reply.  Previously the re-derivation only fired when the
triggering comment was a reply (root_body != comment); now it is unconditional
for action categories.

When posting a reply to a review thread, the handler now finds Fido's last
comment in the thread (if any) and edits it in-place via a new
edit_review_comment method, rather than always appending a new reply.  Threads
with no prior Fido comment still receive a fresh reply via reply_to_review_comment.

fetch_comment_thread now includes 'id' in each returned comment dict so the
edit path can identify which comment to update.
@FidoCanCode
Copy link
Copy Markdown
Owner Author

Woof! ACT tasks now sniff out their titles from the originating PR comment using _summarize_as_action_item. And if I've already replied in the thread, I update my existing comment instead of barking twice. Good boy behavior! 🐕

@FidoCanCode FidoCanCode requested a review from rhencke April 14, 2026 23:10
Comment thread kennel/events.py
When generating a reply to a review comment, include the entire conversation
thread so Opus can consider the full discussion history when crafting or
amending a reply. Previously, reply_context_block ignored the thread even
though it was fetched and available in the context dict.

This ensures that when Fido edits an existing reply to incorporate new
feedback from subsequent comments in the same thread, the updated reply
takes into account the full conversation and can respond coherently to all
participants and issues raised.
@FidoCanCode FidoCanCode requested a review from rhencke April 14, 2026 23:16
@FidoCanCode FidoCanCode merged commit 24b194a into main Apr 14, 2026
3 checks passed
@FidoCanCode FidoCanCode deleted the task-title-from-reply branch April 14, 2026 23:16
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.

Thread task titles pull from reply body instead of root review comment

2 participants