Repository navigation
🪴 fix: Surface Persistent Memory Before First Save - #16301
Conversation
|
Review head: |
|
@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: 3ff1d48a55
ℹ️ 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".
| } | ||
| return memory | ||
| ? `${memoryInstructions}\n\n# Existing memory about the user:\n${memory}` | ||
| : memoryInstructions; |
There was a problem hiding this comment.
When MongoDB memory loading fails, getFormattedMemories catches the exception and resolves with withKeys: '' and withoutKeys: '' (packages/data-schemas/src/methods/memory.ts:312-319), so callers never produce the undefined state this formatter reserves for unavailable memory. This empty-string branch therefore advertises persistent memory during read failures, while the added rejected-promise test exercises a behavior the real dependency does not expose. Preserve an explicit failure signal through the data-schemas method before treating '' as an eligible empty store.
AGENTS.md reference: AGENTS.md:L42-L44
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Resolved in f6ce2a2. The formatted result now marks read failures explicitly instead of returning an eligible empty string. Chat and inline contexts receive no memory text on failure; automatic extraction skips that turn and limited writes fail closed. The request-scoped snapshot is reused so the normal path adds no database read. Added storage and prompt/processor/tool regressions.
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. |
|
Review head: |
|
@codex review the latest head |
|
Codex Review: Didn't find any major issues. Breezy! 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". |
The e2e fake model reported input tokens for the chat messages alone, because the SDK hands a test override model the pruned messages without the systemRunnable pipe. Once #16301 gave every memory-enabled chat a system prompt, the calibrated context snapshot put used tokens below the instructions and the gauge dropped its Messages row. Count input over the complete prompt, as a real provider bills it.
…16339) * 🐢 fix: Back Off Waiting Completion Wake-ups and Deliver Them When Ready * fix: Signal every readiness path and make the completion wait cap configurable * fix: Mark held completion deliveries instead of pipeline expedite, and announce store-won approval expiry * fix: Scope settle expedites to the resumed conversation and close the remaining signal gaps * fix: Announce a won approval expiry once and release subagent wake-up registrations * 🧪 ci: Count System Instructions in Mock Model Usage (#16350) The e2e fake model reported input tokens for the chat messages alone, because the SDK hands a test override model the pruned messages without the systemRunnable pipe. Once #16301 gave every memory-enabled chat a system prompt, the calibrated context snapshot put used tokens below the instructions and the gauge dropped its Messages row. Count input over the complete prompt, as a real provider bills it. --------- Co-authored-by: Lia <lia@librechat.ai>
* fix: Surface Persistent Memory Before First Save * fix: Distinguish Unreadable Memory From Empty Memory --------- Co-authored-by: Lia <lia@librechat.ai>
The e2e fake model reported input tokens for the chat messages alone, because the SDK hands a test override model the pruned messages without the systemRunnable pipe. Once LibreChat-AI#16301 gave every memory-enabled chat a system prompt, the calibrated context snapshot put used tokens below the instructions and the gauge dropped its Messages row. Count input over the complete prompt, as a real provider bills it.
…ibreChat-AI#16339) * 🐢 fix: Back Off Waiting Completion Wake-ups and Deliver Them When Ready * fix: Signal every readiness path and make the completion wait cap configurable * fix: Mark held completion deliveries instead of pipeline expedite, and announce store-won approval expiry * fix: Scope settle expedites to the resumed conversation and close the remaining signal gaps * fix: Announce a won approval expiry once and release subagent wake-up registrations * 🧪 ci: Count System Instructions in Mock Model Usage (LibreChat-AI#16350) The e2e fake model reported input tokens for the chat messages alone, because the SDK hands a test override model the pruned messages without the systemRunnable pipe. Once LibreChat-AI#16301 gave every memory-enabled chat a system prompt, the calibrated context snapshot put used tokens below the instructions and the gauge dropped its Messages row. Count input over the complete prompt, as a real provider bills it. --------- Co-authored-by: Lia <lia@librechat.ai>
2 participants
Summary
Related to #16304 (backend prompt and tool-description localization follow-up).
When an eligible user has no saved memories, the chat prompt omits persistent-memory guidance even though a memory tool or separately enabled extractor can save the user's first memory. The assistant may then say it cannot remember information while the platform successfully records it for future conversations.
Always provide memory-capability guidance when the authorized memory result is empty, and include the existing-memory list only when it has entries. Keep the same opt-out, permission, per-agent tool registration, keyed/unkeyed partition, and request-cache rules. Use neutral wording that never promises a write or claims a memory action succeeded without confirmation. An unavailable or failed memory read still adds no context. Failed reads carry an explicit marker rather than being mistaken for an empty store; automatic extraction skips that turn and token-limited writes fail closed.
How it works
The shared formatter lives in
packages/api/src/agents/memory.ts; normal chat and the inline agents used by Chat Completions and Responses all call it. The data-schemas result distinguishes an empty partition from a failed read. Automatic extraction reuses the same request-scoped snapshot as chat, so normal runs add no database read. No database schema or configuration changes are required.Type of change
Testing
Tested environments/configuration: Node.js 24; focused coverage for first empty memory, read-only chat, automatic extraction, inline-tool registration, missing access, load failure and both API controllers. A provider-backed fresh-session smoke test has not been run.
Automated tests: Added regressions in
packages/data-schemas/src/methods/memory.spec.ts,packages/api/src/agents/memory.spec.ts, andapi/server/controllers/agents/{client.test.js,__tests__/openai.spec.js,__tests__/responses.unit.spec.js}for real failed reads versus empty stores, first-save guidance, automatic extraction, token-limited tools, and context handoff. CI atf6ce2a2f55b549400cfe65437b1c5fe1d5137822passed the package build, workspace TypeScript checks, backend unit-test shards, integration tests, static checks, Lighthouse, and API runtime smoke. Local JS syntax, touched-file Prettier, import sorting and whitespace checks passed. Local focused Jest could not start (@mongodb-js/saslprepis missing from the shared dependency installation); localnpx tsc --noEmitinpackages/apiandpackages/data-schemasstopped on missing installed@typesdependencies. Localnpm run lighthousestopped atbuild:data-providerbecause tsdown is absent there; the CI Lighthouse lane passed. The local staged-only static-check script found no staged files after commit; CI ran the actual static checks on the pushed diff.Screenshots / recordings
Not applicable: no client UI or styles changed; this changes model-bound instructions, so resulting provider prose is nondeterministic.