Repository navigation
fix(memory): tell a credit refusal and an outage apart from an empty memory (#6718) - #7013
Conversation
…memory (tinyhumansai#6718) Memory v2 folded the hosted engine's 402 and every transport fault into the generic ENGINE code, and a refused pre-turn recall only logged a warning, so a user out of credits got turns that read as "nothing is stored". Both were fixed for the old memory module in tinyhumansai#6841 and did not survive the rewrite. - MemoryError gains INSUFFICIENT_CREDITS (via tinymemory's is_insufficient_credits) and UNAVAILABLE, plus is_account_wide(). - pre_turn: when nothing was recalled because the engine refused the account, the turn gets a short notice saying memory is unavailable and why. The refusal is read from the hook's errors and from holistic recall's skipped sections, which is where a section's engine error ends up. - Source sync stops on any account-wide refusal instead of failing each item. - Background jobs: an account-wide refusal no longer uses up an attempt, so a belief build waits for credits instead of being dropped after five runs; the run that does drop a job says so. - UI: memoryErrorMessage(err, t) shows translated text for the two new codes in all 14 locales.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (2)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (40)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe change adds insufficient-credit and unavailable memory error codes. Recall hooks can create refusal notice packs for account-wide errors, and background jobs do not count those errors as retry attempts. The API and memory interface now support translated messages for both error types. ChangesMemory Refusal Handling
Priority: ⬇️ Low Estimated code review effort: 4 (Complex) | ~45 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant MemoryLifecycleHooks
participant MemoryEngine
participant MemoryError
participant TurnPack
MemoryLifecycleHooks->>MemoryEngine: recall
MemoryEngine-->>MemoryLifecycleHooks: recall result or error
MemoryLifecycleHooks->>MemoryError: classify refusal
MemoryLifecycleHooks->>TurnPack: build notice for account-wide refusal
TurnPack-->>MemoryLifecycleHooks: refusal notice pack
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The identified notice gap is not reachable in normal recall handling, and skipped refusal reasons retain their intended classification. The change is mergeable after normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The reviewed changes preserve existing access controls and use fixed refusal notices rather than forwarding raw errors to the model. However, a refusal partway through source sync can leave already-written documents without their follow-up processing. Retained concerns
Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❓ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 52.73% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 55 functions across 36 files. (4 skipped: 1 unsupported, 3 too large.)
A rabbit reads the memory code, Comment |
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 1 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Reviewing pending checks Review snapshot
Completeness: Complete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. Findings
Pending checks: Rust E2E (mock backend), Build Playwright E2E Artifact, E2E (Playwright / web lane), Desktop E2E (full suite, 3 OS) Before merge
How this fits togetherflowchart LR
n0["german"]:::impacted
n1["missingKeys"]:::impacted
n2["simplifiedChinese"]:::impacted
n1 -->|uses| n0
n1 -->|uses| n2
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0147 · 1,179,313 in / 45,870 out · 100,105 cached (8%) · gpt-5.6-luna, glm-5.3-flash, deepseek-v4.1-flash
critique: $0.0064 · 530,446 in / 21,713 out · 51,162 cached (10%) · gpt-5.6-luna, glm-5.3-flash
security: $0.0069 · 496,579 in / 18,378 out · 43,311 cached (9%) · gpt-5.6-luna
tests: $0.0006 · 48,706 in / 1,475 out · 2,752 cached (6%) · glm-5.3-flash
description: $0.0003 · 24,062 in / 293 out · 64 cached (0%) · glm-5.3-flash
e2e: $0.0004 · 55,320 in / 1,996 out · 2,816 cached (5%) · glm-5.3-flash, deepseek-v4.1-flash
| cancelled = true; | ||
| }; | ||
| }, [apply]); | ||
| }, [apply, t]); |
There was a problem hiding this comment.
Avoid refetching policy when translations change
When the active locale changes, t normally gets a new function identity, so this effect runs again and memoryPolicyGet() calls apply(). That replaces drafts with the stored policy, meaning a user who has edited a field but has not yet blurred or pressed Enter loses the edit merely by changing language. It can also overwrite a just-completed save if the refetch returns an older snapshot. Translate the error without making the policy-loading effect rerun on translation changes.
[RULE] unrelated-effect-dependency ·
Summary
INSUFFICIENT_CREDITS(the hosted engine's 402) orUNAVAILABLE(unreachable, timed out, overloaded) instead of the genericENGINE.Problem
openhuman#6841 fixed these on the old memory module. Memory v2 (#6949, #6993) replaced that module, and the fixes did not survive:
memory/error.rsfoldedConflict,UnavailableandEngineintoENGINE. The hosted 402 arrives asEnginewith a[USER_INSUFFICIENT_CREDITS]prefix. tinymemory exportsis_insufficient_creditsfor exactly this, but nothing in openhuman called it.lifecycle/hooks.rs::pre_turnlogged a refused recall atwarnand ran the turn with no pack, so a user out of credits got answers that read as "I don't know that".holistic_recalldoes not fail when a section's read fails. It skips the section, keeps the engine error's text as the skip reason, and returnsOkwith an empty pack.sources/sync.rsstopped only onUnauthorized/Off, so a 402 failed every item one by one.lifecycle/jobs.rsdropped a job after five failed runs, whatever the cause. About 25 minutes without credits left a permanent gap in the beliefs.Solution
MemoryError:InsufficientCreditsandUnavailablevariants, mapped inFrom<tinymemory_api::Error>.is_account_wide(): off, unauthorized, out of credits or unavailable.refusal_from_skip_reason(): reads the refusal back from a skip reason through thetinymemory_api::ErrorDisplaytags.pre_turn: the session-start and turn recalls now keep their errors. If no pack is produced,refusal_oftakes the first account-wide error, or the first refusing skipped section, andTurnPack::refusedrenders the notice. The notice setsTurnPack.refusalto the code and cites nothing.is_account_wide().memoryErrorMessage(err, t)maps the two new codes tomemory.error.*keys.UNAUTHORIZEDkeeps its raw message, which names the rejected key; the engine-save test depends on that.tis memoized on locale, so it was added to the hook dependency arrays.docs/specs/memory-v2.mdlists the codes, the refusal notice, and the job retry rule.memory/import.rsandMemoryImportBanner.tsxbelong to openhuman#7011, which works around the merged code by reading the engine error directly. Once this lands, fix(memory): stop a v1 import on credits or outage instead of skipping every item #7011 can switch to the new codes.Submission Checklist
## RelatedRefusingEnginefixtureImpact
openhuman.memory_*. Consumers that switch oncodeand treat unknown codes as generic failures are unaffected.jobs.jsonis unchanged; an existing job'sattemptscount is kept.Related
INSUFFICIENT_CREDITS/UNAVAILABLE; "layers still deriving" (the fourth failure mode in Stabilize the CortexDB-based memory engine in OpenHuman: validate hosted recall, cut over from tinycortex, then prove imports, context, efficiency and production readiness #6718) is not covered here.AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/6718-memory-stabiliseValidation Run
pnpm --filter openhuman-app format:check: prettier run on every changed filepnpm typecheck:tsc --noEmitcleanvitest run src/services/api/memoryApi.test.ts src/components/memory src/lib/i18n src/pages: 643 passed;cargo test -p openhuman --lib -- memory:: agent::tinyagents::middleware::memory_pack: 148 passedcargo clippy -p openhuman --lib --tests: no findings inmemory/Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
UNAUTHORIZEDtext is unchanged.an_engine_fault_that_is_not_account_wide_injects_nothing, existing hook and jobs tests.Duplicate / Superseded PR Handling
Summary by CodeRabbit