fix(memory): degrade Memory page gracefully when proactive memory is disabled#3131
Merged
Conversation
…disabled Closes #3070. Server: `/api/memory` and `/api/memory/stats` previously returned HTTP 500 with "Proactive memory is not enabled" when `[memory.proactive] enabled = false`. That's a config state, not a server error — 500 triggers retries and error reporting that don't apply here. Both endpoints now return 200 with an explicit `proactive_enabled: bool` flag and empty/null payloads, so the dashboard can render an explanatory notice instead of an error banner. When proactive is enabled the same `proactive_enabled: true` flag is added alongside the existing fields, so old clients that only read `memories` / `stats.*` keep working unchanged. Dashboard: MemoryPage previously only queried the proactive endpoints and ignored `/api/memory/agents/{id}/kv` entirely — agents with real KV memories saw an empty page. Add a `useAgentKvMemory` hook routed through the standard query-key factory, plus a "Per-agent KV memory" section that always renders. When proactive is disabled the page hides the search/semantic/stats panels and shows just the KV section plus a notice; when enabled it shows both. Server-side tests cover both branches of both endpoints; dashboard behaviour is verified by the existing query hook tests plus a small factory-key test.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Self-review found the row-level `title={formatted}` would inline the
entire KV value (potentially multi-KB JSON blobs) into the DOM as a
hover preview. With 50 agents each carrying a few large KV entries,
that adds up to MBs of attribute strings the user never reads.
Cap the preview at 2000 chars (10x the visible truncation) — long
enough that hovering still reveals "more than fits", short enough that
a runaway value can't bloat the page. Full value still lives on the
server; if we ever need a true "view full" affordance it should be a
modal, not a tooltip.
Two CI fixes for #3131: - Quality: cargo fmt wanted the `loopback_get` request builder on one line. Self-fix. - OpenAPI Drift: the new `proactive_enabled` field on /api/memory and /api/memory/stats was added in source but the committed openapi.json was stale. `cargo xtask codegen --openapi` regenerates it; the four SDKs round-trip identically (serde_json::Value response body — no typed schema change), so only openapi.json moved.
github-actions
Bot
removed
the
ready-for-review
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3070 — Memory page was unusable when
[memory.proactive] enabled = false. Two underlying issues, both addressed in this PR.Server (
crates/librefang-api/src/routes/memory.rs)/api/memoryand/api/memory/statspreviously returned 500 with"Proactive memory is not enabled"when proactive was off. That's a config state, not a server error.GET /api/memory→{ memories: [], total: 0, proactive_enabled: false }GET /api/memory/stats→{ stats: null, proactive_enabled: false }proactive_enabled: trueflag alongside the existing fields — no breaking changes to the response shape.get_pm_store()helper still returns its 500 for endpoints that genuinely require proactive memory (search, ingest, etc.); only the read-list and stats endpoints degrade gracefully.Dashboard (
MemoryPage.tsx+ new query)useAgentKvMemory(agentId)hook hitting/api/memory/agents/{id}/kv. Query key frommemoryKeys.agentKv(agentId)factory entry — never inline arrays.proactive_enabled === false: hide proactive sections (search, semantic, stats), show a small notice card explaining the state, keep KV section visible.proactive_enabled === true: keep the proactive sections as before AND show the KV section underneath. Most users don't distinguish proactive vs KV — issue prefers the unified view.Test plan
Server-side tests already in
crates/librefang-api/tests/api_integration_test.rs(4 cases):test_memory_list_returns_200_when_proactive_disabledtest_memory_stats_returns_200_when_proactive_disabledtest_memory_list_includes_proactive_enabled_when_enabledtest_memory_stats_includes_proactive_enabled_when_enabledDashboard manual checks:
[memory.proactive] enabled = false→ Memory page renders KV memories without error[memory.proactive] enabled = true→ Memory page renders proactive sections + KV section underneathuseMemoryStatscallers still get the same fields (stats top-level just gainsproactive_enabled: true)Closes #3070.