Make --note repeatable for add and annotate commands#16
Open
DanielCardonaRojas wants to merge 1 commit intomainfrom
Open
Make --note repeatable for add and annotate commands#16DanielCardonaRojas wants to merge 1 commit intomainfrom
DanielCardonaRojas wants to merge 1 commit intomainfrom
Conversation
📝 WalkthroughWalkthroughThe changes introduce support for multiple annotations per CLI invocation by converting the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli/handlers.rs (1)
1213-1223:⚠️ Potential issue | 🟡 MinorShow all notes added by this annotate call.
With repeatable
--note, this success path only printsbm.annotations.last(). Forcodemark annotate ... --note first --note second --context ..., the user only sees the last note and the context disappears from the output because it was attached to the first inserted annotation. Render fromargs.note/args.context(or track the inserted annotations) so the CLI output matches what was written.Suggested fix
- // Show the newly added annotation - let latest_ann = bm.annotations.last(); - if let Some(ann) = latest_ann { - if let Some(ref note) = ann.notes { - println!(" Note: {}", note); - } - if let Some(ref ctx) = ann.context { - println!(" Context: {}", ctx); - } - println!(" Added by: {}", ann.added_by.as_deref().unwrap_or("unknown")); - } + // Show the annotations added by this invocation + for note in &args.note { + println!(" Note: {}", note); + } + if let Some(ref ctx) = args.context { + println!(" Context: {}", ctx); + } + if !args.note.is_empty() || args.context.is_some() { + println!(" Added by: {}", args.added_by); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/handlers.rs` around lines 1213 - 1223, The success path currently only prints bm.annotations.last(), which hides multiple --note entries and may miss context attached to earlier inserted annotations; update the annotate handler (the block using bm.annotations.last(), ann.notes, ann.context, and ann.added_by) to instead render all annotations created by this command by either iterating over args.note and args.context (printing each note and its corresponding context from args) or by tracking the inserted annotations when you perform the annotate insert (collecting those Annotation objects) and iterating over that collection to print each note, context, and added_by so CLI output matches what was written.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extras/skills/codemark/SKILL.md`:
- Around line 161-165: The docs currently state that --note is repeatable but
omit that --context applies only to the first annotation when multiple --note
flags are used; update the codemark annotate documentation to explicitly state
that behavior by adding a sentence near the examples for codemark annotate
explaining "When multiple --note flags are provided, each creates a separate
annotation and any --context supplied will be attached only to the first
annotation" and ensure the examples using --note and --context reflect this rule
(reference: codemark annotate, --note, --context).
In `@src/cli/handlers.rs`:
- Around line 436-465: Extract the duplicated annotation-fanout logic (the loop
that creates Annotation structs and calls db.insert_annotation) into a single
helper function (e.g., insert_annotations_for_bookmark(bookmark_id, args, db))
and replace the inline blocks in this handler plus handle_add_from_snippet,
handle_add_from_query, and handle_annotate to call that helper; within the
helper start a single DB transaction (using your DB's transaction API on the db
instance), perform all insert_annotation calls for args.note and args.context
inside the transaction, and commit only if all inserts succeed so the whole
multi-note write is atomic and shared across those four callers. Ensure the
helper still sets fields the same way (id via uuid::Uuid::new_v4, bookmark_id,
added_at via now_iso(), added_by from args.created_by, notes/context/source) and
returns/propagates errors from the transaction if any insert fails.
---
Outside diff comments:
In `@src/cli/handlers.rs`:
- Around line 1213-1223: The success path currently only prints
bm.annotations.last(), which hides multiple --note entries and may miss context
attached to earlier inserted annotations; update the annotate handler (the block
using bm.annotations.last(), ann.notes, ann.context, and ann.added_by) to
instead render all annotations created by this command by either iterating over
args.note and args.context (printing each note and its corresponding context
from args) or by tracking the inserted annotations when you perform the annotate
insert (collecting those Annotation objects) and iterating over that collection
to print each note, context, and added_by so CLI output matches what was
written.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 41d00202-9550-4d66-abdb-7c939af0e50c
📒 Files selected for processing (3)
extras/skills/codemark/SKILL.mdsrc/cli/handlers.rssrc/cli/mod.rs
Comment on lines
161
to
165
| # Notes are repeatable — each --note creates a separate annotation | ||
| codemark annotate <bookmark-id> \ | ||
| --note "Additional context discovered during implementation" \ | ||
| --note "See PR #123 for the fix details" | ||
| codemark annotate <bookmark-id> --context "Related to auth-refactor feature" |
There was a problem hiding this comment.
Document the --context rule for repeated notes.
This section explains that --note is repeatable, but it still omits the important caveat that --context is attached only to the first created annotation when multiple --note flags are present. Please add that here so the CLI docs match the implemented behavior.
Suggested doc update
-# Notes are repeatable — each --note creates a separate annotation
+# Notes are repeatable — each --note creates a separate annotation
+# If you also pass --context with multiple --note flags, the context is attached only to the first annotation
codemark annotate <bookmark-id> \
--note "Additional context discovered during implementation" \
--note "See PR `#123` for the fix details"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Notes are repeatable — each --note creates a separate annotation | |
| codemark annotate <bookmark-id> \ | |
| --note "Additional context discovered during implementation" \ | |
| --note "See PR #123 for the fix details" | |
| codemark annotate <bookmark-id> --context "Related to auth-refactor feature" | |
| # Notes are repeatable — each --note creates a separate annotation | |
| # If you also pass --context with multiple --note flags, the context is attached only to the first annotation | |
| codemark annotate <bookmark-id> \ | |
| --note "Additional context discovered during implementation" \ | |
| --note "See PR `#123` for the fix details" | |
| codemark annotate <bookmark-id> --context "Related to auth-refactor feature" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@extras/skills/codemark/SKILL.md` around lines 161 - 165, The docs currently
state that --note is repeatable but omit that --context applies only to the
first annotation when multiple --note flags are used; update the codemark
annotate documentation to explicitly state that behavior by adding a sentence
near the examples for codemark annotate explaining "When multiple --note flags
are provided, each creates a separate annotation and any --context supplied will
be attached only to the first annotation" and ensure the examples using --note
and --context reflect this rule (reference: codemark annotate, --note,
--context).
Comment on lines
+436
to
465
| // Insert annotations with notes and context if provided | ||
| if !args.note.is_empty() || args.context.is_some() { | ||
| // If context is provided, attach it to the first note | ||
| let first_note = args.note.first(); | ||
| if first_note.is_some() || args.context.is_some() { | ||
| let annotation = Annotation { | ||
| id: uuid::Uuid::new_v4().to_string(), | ||
| bookmark_id: actual_bookmark_id.clone(), | ||
| added_at: now_iso(), | ||
| added_by: Some(args.created_by.clone()), | ||
| notes: first_note.cloned(), | ||
| context: args.context.clone(), | ||
| source: Some("cli".to_string()), | ||
| }; | ||
| db.insert_annotation(&annotation)?; | ||
| } | ||
| // Additional notes without context | ||
| for note in args.note.iter().skip(1) { | ||
| let annotation = Annotation { | ||
| id: uuid::Uuid::new_v4().to_string(), | ||
| bookmark_id: actual_bookmark_id.clone(), | ||
| added_at: now_iso(), | ||
| added_by: Some(args.created_by.clone()), | ||
| notes: Some(note.clone()), | ||
| context: None, | ||
| source: Some("cli".to_string()), | ||
| }; | ||
| db.insert_annotation(&annotation)?; | ||
| } | ||
| } |
There was a problem hiding this comment.
Make repeatable-note writes atomic and shared.
This turns one codemark add invocation into several insert_annotation calls, but the loop is duplicated here and in handle_add_from_snippet, handle_add_from_query, and handle_annotate. If one insert fails midway, the command leaves a partially applied annotation set behind. Please move this fan-out into one helper and run it inside a single DB transaction so the operation is all-or-nothing across all four commands.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli/handlers.rs` around lines 436 - 465, Extract the duplicated
annotation-fanout logic (the loop that creates Annotation structs and calls
db.insert_annotation) into a single helper function (e.g.,
insert_annotations_for_bookmark(bookmark_id, args, db)) and replace the inline
blocks in this handler plus handle_add_from_snippet, handle_add_from_query, and
handle_annotate to call that helper; within the helper start a single DB
transaction (using your DB's transaction API on the db instance), perform all
insert_annotation calls for args.note and args.context inside the transaction,
and commit only if all inserts succeed so the whole multi-note write is atomic
and shared across those four callers. Ensure the helper still sets fields the
same way (id via uuid::Uuid::new_v4, bookmark_id, added_at via now_iso(),
added_by from args.created_by, notes/context/source) and returns/propagates
errors from the transaction if any insert fails.
DanielCardonaRojas
force-pushed
the
repeatable_notes
branch
from
c594988 to
60274fa
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli/handlers.rs (1)
1300-1309:⚠️ Potential issue | 🟡 MinorShow all newly added notes in
annotateoutput.
bm.annotations.last()only prints one annotation. For--note first --note second --context ctx, the output can show onlysecondand omit the context attached tofirst.🐛 Proposed fix
- // Show the newly added annotation - let latest_ann = bm.annotations.last(); - if let Some(ann) = latest_ann { - if let Some(ref note) = ann.notes { - println!(" Note: {}", note); - } - if let Some(ref ctx) = ann.context { - println!(" Context: {}", ctx); - } - println!(" Added by: {}", ann.added_by.as_deref().unwrap_or("unknown")); - } + // Show the newly added annotations + if !args.note.is_empty() || args.context.is_some() { + if args.note.is_empty() { + if let Some(ref ctx) = args.context { + println!(" Context: {}", ctx); + } + } else { + for (idx, note) in args.note.iter().enumerate() { + println!(" Note: {}", note); + if idx == 0 + && let Some(ref ctx) = args.context + { + println!(" Context: {}", ctx); + } + } + } + println!(" Added by: {}", args.added_by); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/handlers.rs` around lines 1300 - 1309, Replace the single-item display that uses bm.annotations.last() (the latest_ann variable) with a loop that iterates over the newly-added annotations and prints each one's notes, context, and added_by; specifically, change the block that references latest_ann and ann (and the ann.notes / ann.context / ann.added_by calls) to iterate over bm.annotations (or bm.annotations.iter().rev() if you want newest-first) and print each annotation's fields so multiple --note / --context entries are all shown.
♻️ Duplicate comments (1)
src/cli/handlers.rs (1)
523-552:⚠️ Potential issue | 🟠 MajorMake repeatable-note insertion shared and atomic.
These four fan-out blocks still perform multiple independent
insert_annotationcalls. A failure after the first insert leaves a partially applied--noteset, and the duplicated logic makes future behavior drift likely. Please centralize this in one helper and run the multi-annotation write in a single DB transaction when available.Also applies to: 704-733, 889-918, 1227-1255
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/handlers.rs` around lines 523 - 552, The code creates multiple Annotation objects and calls db.insert_annotation repeatedly (using Annotation, args.note, args.context and db.insert_annotation), which can leave partial state on failure; refactor by extracting a helper (e.g., build_annotations_from_args) that builds Vec<Annotation> from args.note and args.context (attaching context only to the first note), then perform a single atomic multi-insert inside a DB transaction (use your DB transaction API or add db.insert_annotations_transactional that inserts the Vec<Annotation> inside one transaction) and replace the repeated insert loops with a single call; apply the same refactor to the other similar blocks that create annotations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/cli/handlers.rs`:
- Around line 1300-1309: Replace the single-item display that uses
bm.annotations.last() (the latest_ann variable) with a loop that iterates over
the newly-added annotations and prints each one's notes, context, and added_by;
specifically, change the block that references latest_ann and ann (and the
ann.notes / ann.context / ann.added_by calls) to iterate over bm.annotations (or
bm.annotations.iter().rev() if you want newest-first) and print each
annotation's fields so multiple --note / --context entries are all shown.
---
Duplicate comments:
In `@src/cli/handlers.rs`:
- Around line 523-552: The code creates multiple Annotation objects and calls
db.insert_annotation repeatedly (using Annotation, args.note, args.context and
db.insert_annotation), which can leave partial state on failure; refactor by
extracting a helper (e.g., build_annotations_from_args) that builds
Vec<Annotation> from args.note and args.context (attaching context only to the
first note), then perform a single atomic multi-insert inside a DB transaction
(use your DB transaction API or add db.insert_annotations_transactional that
inserts the Vec<Annotation> inside one transaction) and replace the repeated
insert loops with a single call; apply the same refactor to the other similar
blocks that create annotations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 29d7e49f-7757-404a-a918-03aafc6873f8
📒 Files selected for processing (2)
src/cli/handlers.rssrc/cli/mod.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/cli/mod.rs
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
--notefromOption<String>toVec<String>inAddArgs,AddFromSnippetArgs,AddFromQueryArgs, andAnnotateArgs--noteusageDetails
Previously, users could only add a single note when creating or annotating bookmarks. To add multiple notes, they had to make separate
annotatecalls. Now--noteworks like--tag— repeatable for multiple values.Each note creates a separate annotation entry, allowing agents and users to document different aspects of the code independently. When
--contextis provided alongside multiple notes, it attaches to the first note only.Test plan
codemark add --note "first" --note "second"creates two annotationscodemark annotate <id> --note "first" --note "second"creates two annotations🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
--noteflag multiple times in a single command.Bug Fixes