Skip to content

fix(memoryManager): surface isArchived from getStates for tagged states#218

Merged
ProfSynapse merged 1 commit into
mainfrom
fix/state-list-event-refresh
May 26, 2026
Merged

fix(memoryManager): surface isArchived from getStates for tagged states#218
ProfSynapse merged 1 commit into
mainfrom
fix/state-list-event-refresh

Conversation

@ProfSynapse

Copy link
Copy Markdown
Owner

Summary

Bug found while manually testing PR #216 (issue #215): clicking the archive icon on a state in the workspace-settings UI did not remove it from the list, and the refresh button had no effect.

Root cause is a SQLite-metadata fast-path in MemoryService.getStates at line 555 — when stateMeta.tags was cached, the JSONL content fetch was skipped, and the skeleton return path never surfaced state.metadata.isArchived to consumers. This affected both the UI list filter and the AI-facing listStates tool filter at listStates.ts:106, partially breaking the AI-archive-only read contract from #215.

Fix is surgical (1 line): drop the stateMeta.tags ? null : shortcut so adapter.getState always runs. Line 556's tag-extraction OR-short-circuits when stateMeta.tags is set, so behavior on the untagged path is unchanged. Tool filter inherits the fix automatically (reads state.state?.metadata?.isArchived from the same items).

Manual-tested in Obsidian — archive icon now removes tagged states from the list immediately.

Test plan

  • New: MemoryServiceGetStates.test.ts — 3 regression tests (tagged-archived isArchived surfacing, tags preservation, no-shortcut assertion).
  • npm run build clean (lint + tsc + esbuild + connector regen).
  • npm test — 220/222 suites, 2792/2809 tests pass (2 pre-existing flakes unrelated).
  • Manual test in Obsidian: archive a tagged state → row disappears from list; refresh works; AI listStates respects archive on tagged states.

Follow-up tech debt (not blocking)

Proper long-term fix is denormalizing is_archived into SQLite states metadata so the fast-path can return a complete view (~80-120 LoC, v12→v13 migration). The shortcut existed for perf — denormalization restores it without correctness cost. Will file as a separate issue post-merge.

Follow-up to #215 manual-test finding (do not auto-close — #215 has further verification still pending after this lands).

🤖 Generated with Claude Code

The SQLite-metadata fast-path in MemoryService.getStates skipped the JSONL
content fetch when stateMeta.tags was cached. The resulting skeleton-branch
return path never surfaced state.metadata.isArchived to consumers, so:

- The states UI list (PR #216) showed archived tagged states as if active.
- The AI-facing listStates tool (PR #216) returned archived tagged states
  to the LLM as if active — partially breaking the AI-archive-only read
  contract from #215.

Dropping the shortcut so adapter.getState always runs. Line 556's tag
extraction OR-short-circuits when stateMeta.tags is set, so behavior on
the untagged path is unchanged. Tool filter at listStates.ts:106 inherits
the fix automatically (reads from the same getStates items).

Tech debt: proper long-term fix is denormalizing is_archived into SQLite
metadata so the fast-path can return a complete view (~80-120 LoC, v13
migration). Tracked as follow-up.

Tests:
- MemoryServiceGetStates.test.ts (NEW, +107): tagged-archived surfacing,
  tags preservation, explicit no-shortcut assertion.
- npm run build clean; 220/222 suites, 2792/2809 tests pass.

Follow-up to #216 manual-test finding (issue #215).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ProfSynapse ProfSynapse merged commit eae5f50 into main May 26, 2026
ProfSynapse added a commit that referenced this pull request May 26, 2026
Patch release covering #218 — surface isArchived from getStates for
tagged states (follow-up to #215 manual-test finding from PR #216).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ProfSynapse ProfSynapse deleted the fix/state-list-event-refresh branch May 26, 2026 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant