Skip to content

Fix blank memory titles in RATE_MEMORIES directive + terse format - #126

Merged
emp3thy merged 13 commits into
mainfrom
feature/rate-directive-display
Aug 7, 2026
Merged

Fix blank memory titles in RATE_MEMORIES directive + terse format#126
emp3thy merged 13 commits into
mainfrom
feature/rate-directive-display

Conversation

@emp3thy

@emp3thy emp3thy commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Summary

  • The Stop-hook RATE_MEMORIES directive rendered every pending memory as an opaque id with no title: agentcore-mode exposures are recorded under AWS-minted mem-<uuid> ids, which the local reflections.title / semantic_memories.content join can never resolve.
  • Fix: snapshot the memory's display text at exposure-record time into a new nullable session_memory_exposure.display column (migration 0017); every exposure writer now passes (kind, id, display) triples; the read path selects COALESCE(MAX(e.display), r.title, s.content) so pre-migration local rows still resolve live.
  • The directive itself is rewritten to a terse format (header + one evidence-first rule line + one titled line per memory, empty buckets omitted); all other rating rules live only in the rate-session-memories skill. The reason string, block-payload shape, and 8 KB cap are unchanged.
  • memory.list_session_exposures now returns real titles/content for agentcore-backed memories too.
  • Deliberately unchanged: the inline sqlite retrieve INSERTs in services/reflection.py / services/semantic.py (they only write local ids, which the join resolves live) and the Stop hook's standalone inline SQL copy (no service-layer import, per the sync-hook ADR).
  • Validated live mid-development: this session's own Stop hook fired with the terse directive and real snapshot titles.

Spec: docs/superpowers/specs/2026-08-06-rate-directive-display-design.md
Plan: docs/superpowers/plans/2026-08-06-rate-directive-display.md

Test plan

  • Full suite: 1800 passed, 22 skipped, 0 failed
  • Ruff: 0 new findings on branch-touched files
  • New coverage: migration column test, display persistence/truncation, COALESCE precedence (snapshot > live title > none), end-to-end bootstrap display wiring, foreign-id snapshot rendering through the hook subprocess, empty-bucket omission (both directions)
  • e2e sqlite journey (two-fire Stop sequence with real rating turn) green
  • Docs swept: website/architecture.md directive description updated; other hits verified accurate

🤖 Generated with Claude Code

emp3thy and others added 11 commits August 6, 2026 09:09
Root cause: agentcore exposures use AWS mem-<uuid> ids; local title join
misses, so the Stop-hook directive renders blank memory lines. Design:
snapshot display text at exposure-record time, COALESCE on read, and cut
the directive to a minimal format (rules stay in the skill).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Six tasks: display-column migration, exposure_log triples + COALESCE,
protocol/backend threading, terse directive, full-suite gate, docs sweep.
Spec amended: inline sqlite retrieve INSERTs stay join-resolved.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ot on read

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…sites

Task 3 of rate-directive-display: every StorageBackend.record_exposures
implementer and caller now passes (kind, id, display) triples end to
end, so agentcore-id exposures carry a title/content snapshot without
needing a local content row.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds test_bootstrap_exposure_rows_carry_display_snapshots, which seeds a
reflection with a known title and a semantic memory with known content,
runs a real SessionBootstrapService.bootstrap() call, and asserts the
resulting session_memory_exposure rows carry those exact display values.
Closes a coverage gap flagged in review: the existing display test built
a triple by hand and never exercised _record_exposure's
reflection_display/semantic_display dict-comprehension wiring
(r.get("title") / m.content), so a future edit pointing at the wrong
field or the wrong source list would have passed every existing test.
Verified by temporarily breaking the wiring locally and confirming this
test fails, then reverting.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eader

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tive test precision

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Claude BugBot Analysis

Reviewed the exposure-display-snapshot PR (migration, contextual_inject, session_close, exposure_log service, session_bootstrap, agentcore/protocol/sqlite storage backends, and associated tests). All tuple-shape changes (kind, id) -> (kind, id, display) were propagated consistently across every call site and signature; SQL parameter ordering in the INSERT/WHERE NOT EXISTS and the COALESCE(MAX(e.display), r.title, s.content) aggregation are correct; field accesses (m.text, r.get('title'), m.content) match their source types. No concrete defects found.

No bugs were detected in this PR.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Claude BugBot Analysis

Reviewed the display-snapshot exposure-ledger changes across hooks, services, and storage backends; all tuple arities, SQL column/placeholder counts, and data-type assumptions (dict vs sqlite3.Row, dataclass attributes) check out correctly against their call sites. No genuine defects found.

No bugs were detected in this PR.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Claude BugBot Analysis

Reviewed the display-snapshot exposure-ledger PR (migration, exposure_log.record/list_unrated, session_close directive, session_bootstrap/agentcore/contextual_inject wiring, and test updates); traced all changed call sites, dict/dataclass field usages, and SQL aggregate expressions back to their sources and found no genuine defects meeting the confidence bar.

No bugs were detected in this PR.

@emp3thy
emp3thy merged commit 0110c99 into main Aug 7, 2026
3 checks passed
@emp3thy
emp3thy deleted the feature/rate-directive-display branch August 7, 2026 00:45
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