Skip to content

Add answer-feedback workflow to screenote skill#4

Open
ivankuznetsov wants to merge 1 commit intomainfrom
answer-feedback
Open

Add answer-feedback workflow to screenote skill#4
ivankuznetsov wants to merge 1 commit intomainfrom
answer-feedback

Conversation

@ivankuznetsov
Copy link
Copy Markdown
Owner

Summary

  • Update SKILL.md Step 6 from "Offer Next Steps" to "Fix and Respond" — Claude now posts an explanatory reply comment (via add_annotation_comment) before resolving each annotation
  • Reply comments describe what was changed (file, line, summary) or explain why feedback was declined
  • Adds "Reply without fixing" option for won't-fix/by-design cases
  • If add_annotation_comment fails, Claude warns the developer and proceeds with resolution anyway
  • Companion PR for digest notifications: https://github.com/ivankuznetsov/screenote/pull/24

Test plan

  • Manual: run /screenote feedback and verify Claude posts reply comments before resolving
  • Existing MCP tools (add_annotation_comment, resolve_annotation) already tested in screenote repo

🤖 Generated with Claude Code

Update SKILL.md Step 6 to instruct Claude to post an explanatory reply
comment (via add_annotation_comment) before resolving each annotation,
so reviewers know what was fixed or why feedback was declined.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@cursor
Copy link
Copy Markdown

cursor bot commented Mar 10, 2026

PR Summary

Low Risk
Low risk: documentation-only changes that adjust the prescribed workflow, with no runtime/code-path modifications in this repo.

Overview
Claude’s Screenote feedback workflow is updated so addressing an annotation now follows fix → add_annotation_comment reply → resolve_annotation, including a reply-without-fixing option and guidance to proceed with resolution even if the reply comment fails.

Adds a new plans/answer-feedback-with-digest-notifications.md document describing a companion Screenote-app change for hourly resolution digest emails.

Written by Cursor Bugbot for commit c1e708d. This will update automatically on new commits. Configure here.

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.


The feature spans two repos:
- **claude-screenote** (this repo) — SKILL.md changes for comment-before-resolve behavior
- **screenote** (Rails app at `/home/asterio/Dev/screenote/`) — new column, mailer, and background job for digest notifications
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Local filesystem path committed in plan document

Low Severity

The plan doc includes a personal local filesystem path /home/asterio/Dev/screenote/ which exposes a developer's username and directory structure. This belongs in a local-only note rather than a committed file.

Fix in Cursor Fix in Web

rescue => e
Rails.logger.error("Digest notification failed for user #{recipient.id}: #{e.message}")
# Comments keep notified_at as NULL; next job run will retry
end
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rescue scope causes duplicate emails on update failure

Low Severity

In the proposed send_digest method, the rescue block covers both deliver_now and update_all. If the email is successfully delivered but the update_all call then raises, the rescue fires, notified_at stays NULL, and the next job run re-sends the same digest. The edge case table claims "unsent comments" are retried, but this scenario involves already-sent emails being duplicated.

Additional Locations (1)
Fix in Cursor Fix in Web

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.

1 participant