Append generated claim draft to an existing note#64
Conversation
|
@codex review |
![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: d739ec74df
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2941853f5
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
c239cf0 to
567588f
Compare
4b0e0b9 to
fa44e9d
Compare
Adds the second persistence path alongside save-as-new-note: pick an existing note from a dropdown and append the draft markdown to it, separated from the original content by a --- line. Duplicate-append guard tracks every (draft-content, note-id) pair appended this session so: - re-tapping the same target does not duplicate - re-picking the same target after appending still reads as done - switching to note B then back to A correctly recognizes A as already appended even though B was the most recent target - appending a different draft Y, then returning to content X, still recognizes X's earlier targets as done (history keyed by content) Dropdown locks while an append is in flight to prevent races. Stale selected note (deleted from the list) falls back to no selection instead of crashing the DropdownButtonFormField. Error messages are user-friendly; raw exception text is never surfaced. Session-token (shared with saveAsNewNote) prevents cross-session state mutations after reset(). analyze: 0 issues tests: 479 passed Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
729ac4d to
faa0bd5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: faa0bd53b9
ℹ️ 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".
Two Codex P2 fixes: 1. DropdownButtonFormField remount on stale selection Key was based on selectedTargetNoteId alone, so when the selected note disappeared from availableNotes (deleted, list refreshed), the key was unchanged, the widget was not remounted, and FormFieldState kept the stale value — triggering Flutter's "value not in items" assertion. Added _effectiveTarget getter (null when the selected ID is absent from the list) and used it as both the key fragment and initialValue, so a stale selection triggers a remount and the dropdown resets to no selection. 2. toggle() preserving an in-flight append lock toggle() was resetting appendStatus → idle unconditionally, allowing the user to regenerate a draft and start a second concurrent append while the first updateNote write was still in flight. Both writes could then target the same note and race, duplicating or losing content. toggle() now skips the appendStatus reset when appendStatus == appending. The appendToExistingNote() completion paths release the lock to idle (not stuck at appending) when the draft changed mid-write, and the error path does the same silently instead of leaving the lock held. Regression test: "toggling a claim while an append is in flight does not reset appendStatus to idle". analyze: 0 issues tests: 480 passed Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 600fa5b9a8
ℹ️ 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".
generateDraft() was resetting appendStatus → idle unconditionally when the new draft's content had not been appended to the current target. If the user changed the selection and tapped Generate draft while an appendToExistingNote write was still in flight, this re-enabled the append button, allowing a second concurrent read/modify/write against the same note before the first write finished — risking duplicate or dropped content. generateDraft() now checks appendStatus == appending before deciding the new status: when an append is in flight, appendStatus is left unchanged (same guard as the toggle() fix in the prior commit). The completion path in appendToExistingNote() already releases the lock to idle when the draft changed mid-write, so no follow-on state is left stuck. Regression test: "generating a new draft while an append is in flight does not reset appendStatus to idle". analyze: 0 issues tests: 481 passed Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a06835a01
ℹ️ 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".
After saveAsNewNote() succeeds, isDraftAlreadySaved becomes true for the current content. The append button in search_screen.dart now checks this flag (matching the save-exclusivity design: save and append should not both apply to the same generated draft), and appendToExistingNote() in the provider adds the same guard as defense in depth. Regression test verifies the button is null and a direct notifier call does not call updateNote. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Fixed in b72a537. Added |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b72a537ee9
ℹ️ 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".
Two symmetric guards to make save-as-new-note and append-to-existing-note mutually exclusive for the same draft content: 1. Append blocked while save is in flight: appendToExistingNote() now checks pendingDraftContents and returns early; search_screen.dart adds saveStatus != saving to the append button condition. 2. Save blocked after any append: adds isDraftAlreadyAppendedAnywhere getter (true when appendedTargetsByContent[content].isNotEmpty) to ClaimReviewSessionState; saveAsNewNote() checks it before inserting; search_screen.dart adds the flag to the save button condition. Two regression tests verify both directions via UI button state and direct notifier calls. flutter analyze: 0 issues, flutter test: 484/484. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Fixed in f8359ce — two symmetric guards: Thread PRRT_kwDOSP1mZM6SLnkk (append while saving): Thread PRRT_kwDOSP1mZM6SLnkm (save after appending): Added Two regression tests cover both directions. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8359cea90
ℹ️ 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".
saveAsNewNote() now returns ignored when appendStatus == appending so a concurrent save cannot race with an in-flight updateNote and persist the same generated markdown both as a new note and into the target note. search_screen.dart adds appendStatus != appending to the save button condition. Also extends _SaveableAndAppendableRepository with updateGate so the new regression test can hold updateNote in flight and verify that a direct saveAsNewNote() call returns ignored without inserting. flutter analyze: 0 issues, flutter test: 485/485. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Fixed in 7beea8b. Added
Regression test holds updateNote in flight and verifies saveAsNewNote() returns ignored without calling insertNote. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7beea8bd12
ℹ️ 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".
_appendClaimDraftToExistingNote() called ref.read(claimReviewProvider) immediately after the first await with no mounted check. If the user navigated away while appendToExistingNote() was in flight, the widget was disposed and the ref.read threw a framework exception instead of silently skipping the snackbar. Added mounted check matching the pattern already used in _saveClaimDraftAsNewNote and _appendToExistingNote. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Fixed in 5361cac. Added |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5361cac109
ℹ️ 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".
When both panels target the same note concurrently, each reads the original content and writes a full replacement — whichever finishes second silently drops the other's appended markdown. Fix: check noteDraftReviewProvider.status == saving in both the UI guard (onAppendToExistingNote) and the provider (appendToExistingNote()), using _ref.read(noteDraftReviewProvider). Added import for note_draft_review_state.dart in claim_review_provider.dart. Regression test starts a gated note-draft append, then verifies that a direct claim-draft appendToExistingNote() call returns early with 0 updatedNotes while the first append completes normally. flutter analyze: 0 issues, flutter test: 486/486. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Fixed in 4f6efd6. Added |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f6efd6065
ℹ️ 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".
Reverse direction of the cross-panel race: adding a guard only for the
note-draft→claim-draft direction left SearchScreen enabling the
note-draft panel's Append button and _appendToExistingNote() callable
while claimReviewState.appendStatus == appending.
Two guards added in search_screen.dart (avoids circular import that
would arise from adding the check in NoteDraftReviewNotifier):
1. onAppendToExistingNote callback for NoteDraftReviewPanel is null
when claimReviewState.appendStatus == appending.
2. _appendToExistingNote() returns early at the top for the same
condition, blocking direct method-call paths.
Regression test starts a gated claim-draft append, then verifies that
the note-draft panel's OutlinedButton("Append to existing note") has
onPressed == null while the claim-draft append is in flight.
flutter analyze: 0 issues, flutter test: 487/487.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Fixed in 9b1c7b8. Two UI-level guards in
Regression test holds claim-draft append at a gate, then verifies the note-draft panel's |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b1c7b8cd7
ℹ️ 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".
Thread PRRT_kwDOSP1mZM6STrkS — discard while appending: _discardDraft() calls claimReviewProvider.notifier.reset() which increments _requestSequence. The session check suppresses state updates but updateNote is already in flight and completes, modifying the DB after the user chose to discard. Fix: onDiscard for NoteDraftReviewPanel is now null when claimReviewState.appendStatus == appending. Thread PRRT_kwDOSP1mZM6STrkX — silent background append failure: When updateNote throws and the draft changed mid-append, the error was swallowed (appendStatus reset to idle, nothing shown). Added backgroundAppendError field to ClaimReviewSessionState (matches the existing backgroundSaveError pattern). Provider sets it in the background-failure catch branch; cleared at the start of each new append and on runReview. search_screen.dart renders it under a 'claim-draft-background-append-error' key. Two regression tests: discard button null while appending; background append error visible after gate-held failure with mid-append toggle. flutter analyze: 0 issues, flutter test: 489/489. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Fixed in fb696af. PRRT_kwDOSP1mZM6STrkS (discard while appending): PRRT_kwDOSP1mZM6STrkX (silent background append failure): Added flutter analyze: 0 issues. flutter test: 489/489. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Append generated claim draft to an existing note
Save-as-new-note was the only way out of the claim draft panel, so this adds the other half: pick an existing note from a dropdown (same pattern the older note-draft flow already uses) and append the draft markdown to it instead. Reused
NoteDraftReviewRepository.getNoteById/updateNote, added a---separator so the appended content is clearly set off from whatever was already in the note, and left the original content untouched above that line.This one took a lot of rounds to get solid, more than I expected going in. The duplicate-append guard needed several passes: no guard at all, then one that broke on re-picking the same note, then one that broke on regenerating an identical draft, then one that only remembered the most recent appended target (append to A, then B, then pick A again looked unappended), then finally realized a single remembered draft also wasn't enough — append draft X to A, generate a different draft Y, come back to X, and X/A looked unappended too, since there was only room for one content's history at a time. Ended up tracking append history as a map from draft content to the set of notes it's been appended to, which actually handles all of the above at once. Also closed a race where changing the target dropdown mid-append reset the guard before the first append finished; the dropdown now locks while appending, same as the button. Separately, the target dropdown had the same stale-selection crash the older note-draft panel already guards against (deleting the selected note out from under it), fixed the same way that panel does. Test file
search_screen_claim_draft_append_test.dartcovers all of the above plus the basics (preserving old content, the separator, a no-save draft being a no-op, a failed append not getting marked as done). Went from 455 passing to 469,flutter analyzeis clean throughout.