fix(members): word the memory disclosure for the member's store - #7700
Conversation
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsNo findings. The sole candidate requires a member with [OPUS-REVIEWED] 66f9708 Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
First Principles Review (Fable 5) — ✅ PASSPremise-level review of All claims verified. The backend semantics match the copy ( First-Principles-Verdict: PASS A half-true disclosure becomes layer-accurate for the one config where it misled, and every item is the fix or its mandated i18n/test scaffolding. What this change shipsIntent: tell a user who gave a member its own memory store which half of memory actually moved — a FIX.
The fix sits at mechanism level for the drawer's wording; the deeper cause (no per-store separation of conversation memory and lessons) is a backend product gap genuinely out of scope, and the shipped copy states exactly that boundary ("not separated yet"). Backend claims verified: [FIRST-PRINCIPLES-REVIEWED] 66f9708 |
Design Review (Fable 5) — ✅ PASSDesign-level review of All backend claims in the description verify: Design-Verdict: PASS Accurate two-layer disclosure keyed on a config fact, verified against the backend split; right scope, alternatives weighed, wording pinned by tests. Suggestions
[DESIGN-REVIEWED] 66f9708 |
UX Review (Fable 5) — ✅ PASSUX-level review of The diff is small and focused: one new i18n string ( UX-Verdict: PASS The store-aware disclosure reads exactly as intended: a cold reader correctly understood both halves — own notes separate, conversation still shared — and called it important. [UX-REVIEWED] 66f9708 |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
|
@CrysisDeu — triage note on the current red board, so the next push can target the right thing. What I verified on The Design Review and First Principles lanes both BLOCK on the same premise, and the code agrees with them:
So Paths forward (author's call)
Mechanical reds, independent of the above (worth fixing in the same push whichever path is chosen):
Not rebasing or pushing on your branch until you pick a direction. |
The drawer's disclosure ('Members currently share one memory. What you
tell one member is known to all of them.') rendered the same sentence
for a member configured with its own memory_store, where it is only
half the story: the markdown layer (preferences, project notes) is read
from that store, while conversation memory and lessons stay in the one
global vector store every member reads (context.py get_memory_for hands
every named store the default's VectorMemoryStore).
An earlier revision hid the note on a dedicated store. Review lanes
rejected that: the sharing claim is still true at the vector layer, so
silence implied an isolation that does not exist. This revision keeps
the note on every member and words it for the store instead:
- default store: the existing sentence, unchanged.
- named store: 'Preferences and project notes come from the {store}
store rather than the shared default. Conversation memory and lessons
are not separated yet: what you tell this member is still known to
every member.'
The wording keys on the store itself (a config fact), not on a roster
heuristic of who else uses the store — whether a store is shared is a
backend property of which layers it scopes.
New key pages.membersPage.memory_dedicated_note in all twelve
hand-maintained catalogs plus regenerated en-XA. Capture: scout moves to
memory_store 'scout-own' and frame 07 asserts the store-aware wording
there; frame 02 (radar, default store) remains the counterpart.
b8db681 to
66f9708
Compare
bolichen97
left a comment
There was a problem hiding this comment.
Tech Lead review — approved.
Privacy-adjacent correctness (the thing that matters here): the copy is accurate. I verified the backend independently rather than trusting the description: context.py get_memory_for hands every named store the default store's VectorMemoryStore ("Share the global VectorMemoryStore so all agents get semantic/episodic reads"), and get_lessons_for keys on workspace, never on memory_store. So a named memory_store scopes only the markdown read side (preferences, project notes) while conversation memory and lessons remain global. The new memory_dedicated_note says exactly that, and — critically — it does not stop saying the shared part: "what you tell this member is still known to every member." No implied isolation guarantee is created, which is what the earlier hide-the-note revision got wrong and what the Design / First Principles lanes correctly blocked on.
Store identity is a config fact, not a roster inference. Deleting memoryShared is the right call: whether a store isolates a layer is a backend property, and the client had no business guessing it from who else is on the store. The second test pins that (two members on triage still get the store-scoped wording).
Edge case checked and dismissed: String(active.memory_store || 'default') === 'default' treats an explicitly-empty memory_store as default, which mirrors the backend's own memory_store or workspace fallback closely enough; the config loader and the create handler both default the field to "default", so an empty value is not a shipped state.
Scope and hygiene: 17 files but only 3 are substantive — MembersPage.tsx (+11/-2), its test (+32), and the capture script; the other 13 are the single new key across all twelve hand-maintained catalogs plus a regenerated en-XA, per the i18n rule. One commit, ## Pattern harvest present, all 57 checks green, all four AI lanes PASS on 66f9708, zero prior approvals. The 2026-09-04 triage note describes the superseded red board and its path 1 is what shipped here.
Approving as a kirocrew-ux-reviewers member for the website/src/** UX gate.
Problem / Motivation
The Members drawer's disclosure — "Members currently share one memory. What
you tell one member is known to all of them." — renders the same sentence for
a member configured with its own
memory_store. For that member it is onlyhalf the story: the markdown layer (preferences, project notes) is read from
the named store, while conversation memory and lessons stay in the one global
vector store every member reads. The user who took the trouble to point a
member at its own store cannot tell from the drawer which half moved.
An earlier revision of this PR hid the note on a dedicated store. The Design,
First Principles and Opus lanes rejected that, and they were right: the
sharing claim is still true at the vector layer (
context.pyget_memory_forhands every named store the default'sVectorMemoryStore;history_consolidation.pyroutes by workspace, never bymemory_store), sosilence implied an isolation that does not exist.
Change
Store-aware copy instead of hiding. The note renders on every member; its
wording follows the member's store:
pages.membersPage.memory_dedicated_note:"Preferences and project notes come from the
{{store}}store rather thanthe shared default. Conversation memory and lessons are not separated yet:
what you tell this member is still known to every member."
The wording keys on the store itself (a config fact), not on a roster
heuristic of who else uses the store — the previous
memorySharedcomputation is deleted. Whether a store is shared is a backend property of
which layers it scopes, not something the client should infer from
membership.
Lessons are named as shared, not separate:
context.pyreads them frommemory.vector_store(global) or the defaultLessonStore, and the creweditor's own preview banner already says "global lessons reach every agent".
i18n: the new key is added to all twelve hand-maintained catalogs
(bn/de/es/fr/hi/it/ja/ko/pt/ru/zh-CN/en) and
en-XAis regenerated viagen-pseudolocale.mjs.Screenshots
Dedicated-store member (scout on
scout-own) — store-aware wording belowConfiguration:
Default-store member (radar) — existing wording, unchanged (frame 02 on main)
Testing
store-aware sentence naming the store, "not separated yet", "still known to
every member", and NOT the default sentence; two members on the same named
store → store-aware sentence (pins that the roster heuristic is gone).
fixture member; the earlier "note absent" frame is removed. Frame 02 on main
remains the default-store counterpart; no other frame is re-captured.
tsc -bclean, eslint--max-warnings 0clean on touchedfiles,
npm run i18n:checkpasses (diff-scoped gates green). Test suitesare left to CI.
Pattern harvest
Rule candidate: review-prompt
Pattern: a UI disclosure gated on a client-side inference about backend
state ("is this member's memory shared?") — reviewers should ask whether the
condition is a config fact the client owns or a backend property it is
guessing at; when the answer is "guessing", prefer wording the disclosure for
the fact the client does know (the store name) over hiding it.
no linked issue: PM feedback from the Members-page review thread, no tracked issue exists.