feat(ai): add Mention Note context picker for Vault Notes - #3290
feat(ai): add Mention Note context picker for Vault Notes#3290netcatty-bot wants to merge 33 commits into
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2b4632931
ℹ️ 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 text = decodeVaultNoteAttachment(attachment); | ||
| if (!text) return null; |
There was a problem hiding this comment.
Preserve empty Vault note mentions in the prompt
When an attached Vault note has an empty body, decodeVaultNoteAttachment validly returns "", but this falsy check drops the entire block, including its title and noteId. Consequently, a request such as “add deployment steps to this note” gives the agent no way to identify the selected empty note; if the composer is also empty, the UI permits sending but submits an empty model prompt. Distinguish decode failure (null) from valid empty content so the note header is retained.
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: a60f75731b
ℹ️ 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: b0c272a207
ℹ️ 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".
| ensureDraftForScope(scopeKey, fallbackAgentId); | ||
| updateDraftIfPresent(scopeKey, (draft) => ({ | ||
| ...draft, | ||
| attachments: [...draft.attachments, upload], |
There was a problem hiding this comment.
Cap the aggregate size of mentioned notes
The per-note cap added after the earlier size finding still leaves the aggregate payload unbounded: this appends every selection, including repeated selections of the same note. Eight 200k-character ASCII notes—or about four emoji-heavy notes—can place more than 2 MiB of base64 attachment data into one session; serializeSessionsForStorage cannot remove attachments from the sole newest session, so persistence retries fail and the chat remains memory-only and disappears after restart. Enforce a total attachment budget, deduplicate note mentions, or persist only note metadata after the initial turn.
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: 452c2df34c
ℹ️ 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: 3579a5c3d4
ℹ️ 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
Netcatty/application/state/aiStateSnapshots.ts
Lines 465 to 469 in fd1b433
When the newest session exceeds the storage budget only because of accumulated attachment bodies, protectNewestContinuation is false and this ciphertext-pruning loop includes that newest session before the attachment fallback runs. Since historical attachments are always replayed as metadata placeholders, the serializer can discard their bodies without affecting subsequent turns; instead it may preserve those unused bodies while removing reasoningEncryptedContent, which can make OpenAI Responses tool exchanges unreplayable after restart. Determine continuation protection from the attachment-stripped size or prune attachment bodies before touching the newest session's ciphertext.
ℹ️ 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 Please review current head 979dccc. Addressed the fd1b433 review-body finding about continuation preservation: newest-session protection is now calculated from its attachment-stripped size, retaining required reasoning continuation before removing replay-unused note payloads. The regression fails before and passes after; 50 focused tests pass. Two independent local reviewers returned CLEAN; one also verified 192 storage-size combinations. Please give the review result for this exact new head. |
|
Codex Review: Didn't find any major issues. What shall we delve into next? 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 reported no major issues. This PR is marked ready for human review/merge. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 979dccc0b3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| function vaultNoteAttachmentBudgetBytes(attachment: Pick<UploadedFile, "base64Data" | "vaultNoteId">): number { | ||
| const idBytes = typeof attachment.vaultNoteId === "string" | ||
| ? new TextEncoder().encode(attachment.vaultNoteId).length | ||
| : 0; | ||
| return base64DecodedByteLength(attachment.base64Data) + idBytes; | ||
| } |
There was a problem hiding this comment.
Count attachment metadata in the draft budget
After the base64 accounting fix, a note-bearing draft can still bypass the aggregate cap by bulk-selecting many empty or tiny files: this helper charges only decoded payload bytes and a Vault ID, while every accepted upload also persists its filename, path, media type, and object structure. Enough zero-byte files therefore have nearly zero budget cost but can push the newest session over the storage limit; attachment-body pruning cannot reduce that metadata-only payload, so persistence can still fail. Include the serialized metadata cost or impose an attachment-count limit.
Useful? React with 👍 / 👎.
|
@codex review |
|
@codex review Please review current head d8975c6. At the user's request this feature now stores only a note reference (exact ID and title) and directs the agent to the existing vault_notes_get tool for current contents. The prior PR-specific full-body attachment budgets, storage pruning, draft synchronization and external prompt truncation changes have all been removed. Please assess this reduced complete PR, not the superseded snapshot design. Full diff: 21 files, net +411 (production +302, tests +109), down from +1,630. Full tests: 11,450 passed, zero failures, 18 existing skips. 98 focused tests, lint and build pass; typecheck matches 732 baseline diagnostics with zero additions. Two independent local reviewers returned CLEAN. Full-sidebar browser checks verify exact same-title selection, refresh/deduplication, empty-note sends and stored ID with empty payload. Existing Vault tool checks verify latest content and missing-note failure. No live model response was tested. Please give the review result for this exact reduced head. Leave the PR unmerged. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8975c6fb2
ℹ️ 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 Please review current head d78738e. Fixed the reader-availability finding: only Catty or idle external MCP agents can mention notes. Unsupported modes have a disabled entry and localized explanation; send/steer reject existing references and preserve the draft. Running external turns cannot add note references, avoiding settings changes claiming tools unavailable to the active turn. Browser checks verified Skills entry disabled, existing-reference send blocked with visible feedback, and MCP entry enabled. Two independent local reviewers returned CLEAN; 98 focused tests pass. Keep this scoped to reference-only mentions. No storage/attachment budgets or new CLI surfaces are needed. Full PR is now net +432 lines, down from +1,630. Please report the result for this exact head and leave the PR unmerged. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d78738e96d
ℹ️ 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 Please review current head d7ec120. Fixed the long-message replay finding by placing compact JSON note identities ahead of prose and emitting the read instruction once. Actual external-history tests cover a 2,500-character user request and ten notes plus a do-not-edit constraint; both retain the IDs. The long-request test fails before the fix and passes after. Full reference-only PR: 21 files, +488/-28, net +460 (production +325, tests +135), down 72% from +1,630. No general storage, prompt-limit, draft-budget or CLI expansion. The prior availability gate remains: Catty or idle external MCP agents only; send/steer reject unsupported note references. 100 focused tests pass. Both independent local reviewers returned CLEAN on the complete latest diff. Fresh full-suite verification is running. Please return the result for this exact head. Leave this PR unmerged. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7ec12037e
ℹ️ 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 Please review current head 0e43e80. Fixed near-limit metadata overflow: the complete escaped note-reference block must fit 1,000 characters at selection and actual send/steer. This keeps all accepted IDs in existing 2,000-character raw replay with space for prose. Too-large reference groups get localized feedback without changing the draft. Ordinary attachments are not counted or restricted; generic history/storage limits remain unchanged. 101 focused tests pass, including long IDs/titles and ordinary 2MB attachments. Browser verification confirms rejecting a third long-metadata note with feedback and zero draft writes. Lint/build pass. Both independent local reviewers returned CLEAN; tests also covered escaped IDs through real history recovery. Complete PR: net +488 (production +344, tests +144), down 70% from +1,630. Please return the result for this exact head; leave the PR unmerged. |
|
Codex Review: Didn't find any major issues. Keep it up! 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 reported no major issues. This PR is marked ready for human review/merge. |
Summary
Adds Context → Mention Note to the AI composer. Users can search Vault notes and reference the exact note for summarizing or editing, including notes with identical titles. The conversation stores only the note ID and display title; the agent uses the existing
vault_notes_gettool to read current content. A missing note is reported instead of substituting a title match.Type of Change
Related Issue (optional)
Closes #3289
Changes Made
The full PR is 21 files, +516/-28 (net +488): production net +344, tests +144. This replaces the earlier net +1,630 design, a 70% reduction. References resolve current contents, not a frozen snapshot from selection time.
Screenshots / Demo
Verified in a local browser harness using the real AIStateProvider and complete AIChatPanelsHost: open Context → Mention Note; search
betaamong two Runbook notes; select with Enter; edit Beta while mounted; select it again and confirm one updated chip; send and inspect the resulting user message containing exact IDbetawith an empty payload. Empty-note selection and sending also pass. A direct existing Vault-tool check confirms latest-body reads and explicit missing-note failure after deletion.The harness has no AI provider configured: sending reaches the normal provider-configuration message. No paid model response or packaged Electron end-to-end run was tested.
Testing
npm run dev) — local Vite full-sidebar harness used.npm run lint)npm test) — 11,452 passed, 0 failed, 18 existing environment-gated skips; 11,470 total.npm run generate:capability-tools) — not applicable; reuses existing tools.101 focused tests pass on final head
0e43e805553b75fb96e0367b8fab76ac169930d6. The full-suite result above is from the preceding head; the final reference-size guard has focused tests and a browser check confirming a third long-metadata note is rejected with visible feedback and zero draft writes.npm run buildpasses.tsc --noEmitretains the same 732 pre-existing diagnostics as the merge base after normalizing paths, source locations and prop-count summaries; no new diagnostics. Both independent local reviewers returned CLEAN on the reduced full PR; one additionally verified serialization/restoration of note references alongside ordinary attachments.The long-request external-recovery regression fails before the fix and passes after. The ten-note recovery regression retains all IDs and a short do-not-edit instruction. Fresh remote review and CI for the final head are running.
Checklist
Keep this PR open for maintainer merge after review and checks complete.