feat: show a setup-required state when no grounded answer provider is configured#69
Conversation
Wires SelectedClaimsDraftBuilder into the claim review panel with a "Generate draft" button. Builds the markdown from only selectedSaveableItems (so unselected and alreadyKnown claims never make it in), shows a preview with source count, and falls back to a plain empty state when nothing saveable is selected. Nothing is persisted yet. Draft clears whenever a new review runs or a selection changes so the preview can't go stale.
alreadyKnown and uncertain claims are marked canBeSaved: false by the mapper, but generateDraft() silently drops them from selectedSaveableItems even if checked. The tile still let you check them, so the box could show "selected" for a claim that quietly never makes it into the draft. Disable the checkbox for those rows instead.
Wires up a "Save as new note" action on the claim draft preview, reusing the same note-insert path the older note-draft flow already uses. Only ClaimDraftResult.markdownContent gets written, never the raw grounded answer text. Tracks saving/saved/error states and refuses to fire a second insert for the exact same draft content once it's saved, so double-clicking doesn't create duplicate notes. Generating a new draft or changing the selection resets the save state since it's no longer the draft that got saved.
A save could be triggered twice before the first insertNote resolved, since only the disabled button protected against duplicates and the notifier itself didn't reject re-entry while a save was in flight.
If the user toggled a claim or regenerated the draft while a save was still in flight, the pending insertNote's completion would overwrite the current (already-different) state as saved, which is wrong.
Not part of the app, was committed by accident.
Tapping generate again with the same selection reset saveStatus to idle even though the markdown matched what was already saved, which re-enabled the save button and would have inserted a duplicate note.
Regenerating with the same selection while a save was still in flight created a new draft instance with identical markdown, so the identity check on completion skipped recording it as saved and left the door open for a duplicate insert.
The content actually inserted was only remembered when it still matched the current draft, so toggling away and back to the same selection lost track of it and let a duplicate get saved.
|
@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: a0c0d9c337
ℹ️ 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".
| key: const Key('claim-review-provider-not-configured-state'), | ||
| message: | ||
| 'Claim review needs a grounded answer provider set up first. ' | ||
| 'Configure one in Settings to see review results for your questions.', |
There was a problem hiding this comment.
In the default app, groundedAnswerProviderProvider is still hard-coded to NullGroundedAnswerProvider and no Settings control rewires it, so every production ask that reaches this state gives users an action that cannot actually enable claim review. Either make this message say grounded claim review is not available yet, or wire it to a real settings-backed provider; otherwise users will loop through Settings without a way to resolve the setup-required state.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2aeaf3d5b
ℹ️ 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".
| key: ValueKey<String>( | ||
| 'claim-draft-append-target-${selectedTargetNoteId ?? 'none'}', | ||
| ), |
There was a problem hiding this comment.
If the currently selected target note is removed while this panel stays mounted and availableNotes changes to a non-empty list that no longer contains that id, the FormField keeps its old internal value because this key only depends on selectedTargetNoteId. The initialValue fallback is then ignored on rebuild, so Flutter's dropdown still receives the stale value with no matching item and can assert/crash in the exact stale-note scenario the comment below is trying to handle. Include the validity of the selection (or otherwise clear the field state) in the key/value path when availableNotes changes.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2aeaf3d5b
ℹ️ 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".
| key: ValueKey<String>( | ||
| 'claim-draft-append-target-${selectedTargetNoteId ?? 'none'}', | ||
| ), |
There was a problem hiding this comment.
When the selected target note is removed from availableNotes while selectedTargetNoteId stays the same, this key does not change, so the existing DropdownButtonFormField keeps its old FormFieldState.value. initialValue only initializes/resets the field; it does not clear the current value on rebuild, so the dropdown can still build with the stale note id even though that id is no longer in items, hitting the same missing-item assertion this fallback is trying to avoid.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58b3ebde88
ℹ️ 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".
| const SizedBox(height: 12), | ||
| FilledButton( | ||
| key: const Key('append-claim-draft-button'), | ||
| onPressed: onAppendToExistingNote, |
There was a problem hiding this comment.
When selectedTargetNoteId drops out of availableNotes (the case handled above by rendering the dropdown with initialValue: null), this still leaves the Append button wired to the stale callback. During a notes refresh, allNotesProvider can be empty while the note still exists in the repository, so tapping here can append to the previously selected note even though the UI says no target is selected; gate onPressed on _targetNoteIsValid or clear the stale target before allowing the append.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5d223caea
ℹ️ 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".
| onAppendToExistingNote: claimReviewState.draft!.shouldSave && | ||
| !claimReviewState.isAppendInFlight && | ||
| !claimReviewState.isDraftAlreadyAppended |
There was a problem hiding this comment.
When the note-draft panel is already appending and the claim-draft panel is also visible, this enablement still leaves the claim append button active. Both append paths read getNoteById and later updateNote with a whole content string, so two quick appends to the same target can read the same original note and the later write drops the other appended block. Please gate this on the note-draft saving state too, and reciprocally block the note-draft append while a claim append is in flight, or serialize the note updates.
Useful? React with 👍 / 👎.
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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.
The append button and notifier had no guard once an append succeeded, so a repeat tap (or a direct notifier call) would append the same draft content again. Selecting a different target note still allows a fresh append, since that's not a duplicate.
selectTargetNote reset appendStatus unconditionally, so re-picking the same note from the dropdown (which fires onChanged even for an unchanged value) undid the already-appended guard and allowed a duplicate append.
generateDraft() reset appendStatus to idle unconditionally, so regenerating an unchanged draft after a successful append silently re-enabled the append button. Now appendedDraftContent/ appendedTargetNoteId are tracked together, mirroring how the save flow tracks savedDraftContent, so an already-appended draft stays recognized as such through regeneration and even switching targets away and back.
initialValue was passed through unchecked, so if the previously selected note disappeared from availableNotes (deleted, list refreshed) while still referenced as the target, DropdownButtonFormField threw on rebuild. Mirrors the same guard the older note-draft review panel already has: fall back to no selection, and key the dropdown by the selected value so it remounts cleanly.
appendedTargetNoteId only remembered the most recent target, so appending the same draft to A then B and reselecting A wrongly looked unappended. Now a set of appended target ids is tracked per draft content. Also locks the target dropdown and selectTargetNote itself while an append is in flight, since changing targets mid-append reset appendStatus and could let a second append start before the first one recorded its result.
A single appendedDraftContent string only remembered targets for the most recently appended content, so appending draft X to A, generating a different draft Y, then returning to X made X/A look unappended again. Now the append history is a map from content to its set of targets, so switching between distinct drafts no longer loses earlier history.
The claim review/draft UI had no way to abandon a session on its own - the existing Discard button only reset the older note-draft flow. Wires claimReviewProvider.reset() to a dedicated Discard button that shows whenever there are review results or a generated draft, clearing groups, selection, draft, and save/append status without touching any persisted notes.
codex flagged that discarding while insertNote/updateNote was still awaiting would show "Nothing was saved" while the write completed anyway. Disable the discard button whenever a save or append is in progress instead.
codex caught that savedDraftContent only remembered a single string, so saving draft A, saving a different draft B, then coming back to A's selection would forget A was already saved and let the save button re-insert a duplicate note. Tracks every content that's been saved instead, mirroring how the append path already handles this.
codex pointed out that regenerating the same selection mid-save reset saveStatus back to idle before the pending insertNote resolved, which re-enabled the Save button and let a second tap fire a duplicate insert. Same gap existed for append. Now regenerating identical content keeps the in-flight status until the write actually resolves.
…d status codex found that toggling a claim (or regenerating an unchanged draft) mid-save legitimately resets the displayed saveStatus/appendStatus back to idle while the old insertNote/updateNote call is still awaiting, and the save/append guards were checking that displayed status. A tap in that window slipped past the guard and started a second, overlapping write. Track the real in-flight state separately so a second tap is always a no-op until the first write actually resolves.
codex noticed the discard-disable check and the append target lock were both reading saveStatus/appendStatus, which a toggle mid-write resets to idle even though the real insertNote/updateNote is still running. Added isSaveInFlight/isAppendInFlight to session state, set only around the actual repository call, and pointed both guards at those instead.
The claim review panel used to go quiet whenever the answer, claims, or saves didn't turn out the way you'd expect - no spinner while it was working, and a blank screen if the provider came back with nothing, no claims, or everything already known. Also noticed save/append errors were rendering the raw exception straight into the UI, which is a bad look if that ever contains anything sensitive. Added a loading indicator, three empty states (no answer, no claims, all-known), a retry action for a failed review, and swapped every raw error.toString() for a plain message with a retry button.
…played status Toggling a claim mid-save or mid-append resets saveStatus/appendStatus to idle while the underlying write is still running, which re-enabled the Save/Append buttons for a write the notifier was still ignoring.
…n flight reset() replaces the whole session, including isSaveInFlight/isAppendInFlight, while the notifier's private write lock can still be true. That let a fresh draft show Save/Append as enabled for a tap the notifier was still ignoring, and let a stale write's completion record its content into the new session.
…s section The Sources section only resolved titles from the provider's top-level citations list, so a claim carrying its own citation title that wasn't echoed back there silently rendered as a bare URL in saved/appended notes.
1ea2915 to
39c2823
Compare
…ng-provider state
37e58c7 to
af20989
Compare
747b4d9 to
e248658
Compare
1 participant
Add a setup-required state and provider-name guardrails for the claim review flow
Right now the ask flow already checks
isProviderConfiguredbefore running the grounded-answer pipeline, but when it's not configured it just silently skips -- the user taps Ask and nothing happens, no explanation. Added aproviderNotConfiguredstatus to the claim review session so that case shows a plain "set one up in Settings" message instead of quietly doing nothing. Also added a small "Answered via " label to the claim review panel soproviderNameactually shows up somewhere as a safe, plain-text label (it was already flowing through the pipeline but never rendered). Went and double-checked the existing secret-safety guarantees while I was in there:SelectedClaimsDraftBuilderalready never touchesproviderNamewhen building markdown, and a throwing/misbehaving provider is already swallowed byGroundedAnswerIngestionService.ingest()so no raw exception text (or anything that looks like an API key) reaches the screen. Added tests for all of that instead of just trusting it. Went from 503 to 508 tests, all green,flutter analyzeclean. Didn't touch persistence, didn't add a real provider, didn't touch Brave.