Skip to content

[replay 7239] fix(memory): characters joining a merged group chat no longer hide earlier memories - #453

Open
Promansis wants to merge 2 commits into
replay-base/7239-cleanfrom
replay/7239-gps
Open

Promansis wants to merge 2 commits into
replay-base/7239-cleanfrom
replay/7239-gps

Conversation

@Promansis

Copy link
Copy Markdown
Owner

Dottore replay shadow (fork only) of upstream PR 7239 at accf679, the first head CodeRabbit reviewed.

Original title: fix(memory): characters joining a merged group chat no longer hide earlier memories

SpicyMarinara and others added 2 commits October 7, 2026 12:54
…rlier memories

In merged Roleplay group chats the recall audience is every chat
character, and a scene was recalled only when all of them had been there.
After a character joined, no earlier scene qualified. Character-scoped
Chat Summaries were also rendered as whichever character's random id
sorted first, which could be the newcomer, so they rendered empty.

A merged reply can voice any character, so recall now keeps what any
present character remembers and marks what not everyone knows:
"Scene summary (known only to Alice):" for scenes and
"[Known only to Alice: ...]" for private summary sections. Memories no
present character has, including narrator-only scenes, stay out.
Individual group chats, single-reader recall, impersonation and narrator
rules are unchanged.

Closes Pasta-Devs#7237

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Keeps every CHANGELOG entry from both sides.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

🎭 Dottore Review — Concluded

Tip

Review posted. The specimen has left the observation table.

  • Commit: accf679 - Merge origin/staging into fix/merged-group-newcomer-recall

@github-actions

Copy link
Copy Markdown

🎭 Dottore Review

Dottore Verdict: Repair Required

Warning

REPAIR REQUIRED
Actionable findings or blocking proof gaps remain for this head.

Findings Nitpicks Controls Reviewed Head Mode
2 0 1 warn accf6790 full

Note

Mode: full
Head: accf679 - Merge origin/staging into fix/merged-group-newcomer-recall
Base: c2f214a - Merge pull request Pasta-Devs#7236 from Pasta-Devs/feat/advanced-memory-fix-all

🧭 Specimen Summary

  • The specimen replaces unanimous scene participation with any-reader recall for merged Roleplay chats, then labels recaps and character-scoped Chat Summaries with who remembers them. The shared macro helper gains a marking mode and preparation caches receive a revision bump; the interesting fractures appear where the broader audience meets first-reader macro evaluation and the old all-reader visibility filter.

🔎 Isolated Defects

Severity Location Finding
⚠️ MEDIUM packages/server/src/services/advanced-memory.ts:1689 (RIGHT) Resolve each reader's fragment before marking it
⚠️ MEDIUM packages/server/src/services/advanced-memory.ts:3382 (RIGHT) Align recap source eligibility with any-reader recall

🧹 Minor Imperfections

  • None recorded.
🤖 Copy prompt for isolated Dottore findings
Task: Fix `packages/server/src/services/advanced-memory.ts:1689` on the RIGHT side.
Finding: Resolve each reader's fragment before marking it
Severity: medium
Goal: A retained private fragment preserves its reader-specific macro meaning, and knowledge labels never keep otherwise empty content alive.
Cover: Chat Summaries and shared-scene recaps containing a private branch with {{char}}.; Private branches whose nested variable conditions resolve to empty text.
Avoid: Resolving the complete template as names[0] before scoping merely restores the original lost-branch defect.; Bob-only scene recaps already narrow the audience to Bob; shared recaps with private sections must work as well.
Acceptable fixes: Render fragments with the appropriate reader contexts before grouping equivalent nonempty results and adding labels.
Proof required: With Alice sorting first, a Bob-only branch containing {{char}} names Bob in both Chat Summaries and shared-scene recaps.; A private branch containing only a false variable condition produces neither a knowledge label nor a recalled empty scene.; Single-reader and individual-mode rendering remains unchanged.
Run the narrowest relevant check. If stale, leave code unchanged and record why.

Task: Fix `packages/server/src/services/advanced-memory.ts:3382` on the RIGHT side.
Finding: Align recap source eligibility with any-reader recall
Severity: medium
Goal: A merged audience can recall a correctly labelled scene when at least one remembering reader has safe source access, even if another present character cannot read any of its messages.
Cover: All scene messages are hidden from the newcomer while an existing participant has full access.; Partial remembering readers still require the existing visibility-snapshot safeguards.
Avoid: Changing allowed() globally to a union would also widen live-message and raw-excerpt access.; A current visibility snapshot cannot rescue the existing mandatory some(eligibleIds) check when the shared source intersection is empty.
Acceptable fixes: Derive recap eligibility from per-rememberer source views, retaining existing full-coverage or current-visibility checks and keeping raw-source eligibility separate.
Proof required: A closed Alice-only scene whose messages are all Bob-hidden remains recalled and labelled for Alice in a merged Alice/Bob prompt with sufficient budget.; Bob alone cannot recall that scene, and the merged prompt contains no Bob-hidden raw excerpts.; Stale partial-reader visibility and narrator-only scenes remain protected.
Run the narrowest relevant check. If stale, leave code unchanged and record why.

✅ Control Checks

Status Type Check Detail
⚠️ WARN Review Limitation Check Questions The asker failed on 1 packet(s), so those were reviewed from the finder's candidates alone.

🧪 Observations

  • Finder: Traced audience matching through archive validation, source selection, disabled-record filtering and recap admission; membership now unions readers while source eligibility still intersects them.
  • Finder: Followed marked summary branches into the first-name macro context and empty-recap filtering; unresolved bodies cross the new rendering boundary.
  • Finder: Checked empty, single and multiple audiences, narrator exclusion, missing-name defaults and individual mode; the original no-mark shared-helper path remains intact.

📝 Lab Notes

  • Dottore examined 2 candidate(s) against the code: 2 confirmed, 0 rejected, 0 unresolved.

@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.

Dottore Review — isolated defects from the specimen.

// evaluating those per reader is the upgrade path.
const readers = [...new Set(mergedReaders(ctx, audience).map((id) => ctx.names.get(id) ?? "Character"))];
if (readers.length > 1)
text = scopeCharacterSummary(text, readers, 0, (part, known) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ MEDIUM: Resolve each reader's fragment before marking it

Location: packages/server/src/services/advanced-memory.ts:1689 (RIGHT)

The union preserves Bob's branch, then rather elegantly assigns its actions to Alice. In a merged Alice/Bob Chat Summary, {{#if char == "Bob"}}{{char}} hid the key{{/if}} becomes [Known only to Bob: Alice hid the key], because scoping removes the guard before the single names[0] macro pass. Marking unresolved fragments also lets a false nested variable guard leave a nonempty [Known only to Bob: ], defeating the empty-recap filter.

Tip

Suggested fix: Resolve surviving fragments in their readers' macro contexts, discard empty results, and only then attach knowledge labels.

// A merged group recalls what any present character remembers, labelled with who does (#7237).
return ctx.individual
? characters.every((id) => record.audienceCharacterIds.includes(id))
: characters.some((id) => record.audienceCharacterIds.includes(id));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ MEDIUM: Align recap source eligibility with any-reader recall

Location: packages/server/src/services/advanced-memory.ts:3382 (RIGHT)

The new audience union admits an Alice-only scene, only for the source intersection to revoke admission. When every scene message is hidden from Bob but readable by Alice, allowed() removes them all for the merged audience, and the mandatory coverage check rejects the recap even with current visibility metadata. Adding Bob therefore still erases Alice's earlier recall in precisely the differing-source-view case the any-reader promise should cover.

Tip

Suggested fix: Check recap safety per remembering reader, while leaving live-message and raw-excerpt visibility on their stricter path.

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.

2 participants