Skip to content

fix(cat): forget_memories actually deletes + skip guaranteed-413 Groq round-trips - #555

Closed
catomean wants to merge 2 commits into
mainfrom
fix/cat-forget-matching
Closed

fix(cat): forget_memories actually deletes + skip guaranteed-413 Groq round-trips#555
catomean wants to merge 2 commits into
mainfrom
fix/cat-forget-matching

Conversation

@catomean

@catomean catomean commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Incident (prod, 2026-08-02, user @mao)

User: "Photography, handmade ceramics, and French don't apply to me, and the weekends-only availability is wrong. Forget all of those."

The model emitted a well-formed forget_memories exec_action with exactly those phrases — twice — the action logged completed (economic-profile entries were removed), yet every targeted cat_memories row survived: "Is a professional photographer who speaks fluent French…", "Has a documentary photography background", "Selling a handmade ceramic coffee mug…", "Knows French", "Can only work on weekends."

Root causes (both verified against prod)

  1. Lexical matcher can't see inflections. Exact-substring token matching: photographyphotographer, ceramicsceramic, speakingspeaks — plus a hard ≥2-token-hit rule. All four phrases scored 0–1 hits → no match.
  2. Semantic floor was unreachable. FORGET_MATCH_SIMILARITY = 0.75, but embedding the actual incident phrases against the actual stored rows (text-embedding-3-small, prod embeddings) scores true targets 0.39–0.61 and unrelated rows ≤ 0.29. The fallback could never fire for real phrasings.

Fix

  • Light suffix-stripping stemmer (photography/photographerphotograph; equality-only stem compare, so constraint can never collide with construction) + majority-of-significant-stems rule (1 of 1, 1 of 2, 2 of 3…).
  • Semantic floor 0.75 → 0.45 (sits in the measured gap between targets and noise).
  • displayMessage now lists exactly what was removed, not just counts — an over-match is visible and re-addable at a glance.
  • Regression suite pins the incident corpus + the exact phrases the model passed, including the over-deletion guard: "weekend availability constraint" must NOT lexically nuke the "extra income on weekends" goal.

Second fix: guaranteed-failing Groq round-trip on every message

journalctl showed every Cat chat failing on primary Groq with HTTP 413 and falling back to OpenRouter. Measured against the platform key: Groq on-demand tier hard-rejects requests over 12,000 TPM — and a grown Cat prompt (22 memories + history + page excerpt) exceeds that deterministically. The chain now pre-flight-skips a platform Groq link that clearly can't fit, only when another link can serve; BYOK Groq is always attempted, and a skip never poisons the global link-health breaker.

Verification

  • npm run verify green: typecheck clean, lint 0 errors, 145 suites / 1467 tests pass (13 new).
  • Similarity numbers measured against the live prod DB + real OpenAI embeddings; TPM limit measured with a real 413 against the platform key.

🤖 Generated with Claude Code

catomean and others added 2 commits August 2, 2026 14:44
…rity scores

Prod incident 2026-08-02: the user asked Cat to forget photography /
handmade ceramics / French / weekends-only; the model passed exactly those
phrases, the action completed — and every targeted memory survived.

Two dead layers:
- Lexical: exact-substring token matching can't see inflections
  (photography≠photographer, ceramics≠ceramic, speaking≠speaks) and the
  hard >=2-hit rule killed the rest. Now: light suffix-stripping stemmer +
  majority-of-significant-stems rule, stems compared by equality only.
- Semantic: floor 0.75 sat above what true targets score with
  text-embedding-3-small (measured against prod: 0.39-0.61, noise <=0.29).
  Now 0.45.

Also list exactly WHAT was removed in the displayMessage (not just counts)
so an over-match is visible and re-addable at a glance.

Regression suite pins the incident corpus + phrases.

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

Every Cat message in prod was paying a guaranteed-failing Groq round-trip:
the assembled prompt (memories + history + page excerpt) exceeds Groq's
on-demand 12k TPM limit, so the primary 413'd on every single message and
the reply always came from the OpenRouter fallback anyway (confirmed via
journalctl + a measured 413 against the platform key).

The chain now skips a PLATFORM Groq link up front when the prompt clearly
won't fit — only when another link can serve, never for BYOK Groq keys
(possibly a higher tier), and a pre-flight skip never marks the link down
globally (other users' smaller prompts still fit it).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
catomean added a commit that referenced this pull request Aug 2, 2026
…th too (#562)

#545 registered forget_memories in both dispatch systems and promised it
"clears BOTH stores" — but the both-stores composition landed only in the
exec_action handler. The tool-call path (the default for tool-capable
providers) called forgetMemoriesMatching alone, so a "removed" skill kept
living in user_economic_profile and kept driving offers/interview prompts:
the exact half-forgotten failure the fix was written to end, surviving on
the other dispatch path.

The tool-call branch now runs forgetMemoriesMatching AND
removeFromEconomicProfile in parallel (mirroring handlers/context.ts),
reports both result sets in the tool content, only lists a fact as notFound
when BOTH stores missed it, and counts profile-only removals as success
instead of no_results.

Deliberately touches only tool-executor.ts + a new test — no overlap with
the open #555 matcher rework (memory.ts / context.ts), which carries the
shared-matcher refactor blockers separately.

New suite pins the both-stores contract on this path: union reporting,
profile-only success, and both-missed no_results. 428 cat-area tests green.

Co-authored-by: Georgy Butaev <41178744+g-but@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@catomean

catomean commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator Author

Closing as superseded — this work is already on main, in a more complete form.

Checked before closing, rather than assuming:

  • Groq 413 pre-flight — present in src/services/ai/groq.ts on main, and __tests__/unit/ai/groq-preflight.test.ts exists there too.
  • Forget matching (stemmer + lowered similarity floor) — present in src/services/cat/memory.ts on main, with more of it than this branch carries (20 matching references vs 10 here), via fix(cat): forgetting a two-word fact could delete an unrelated memory #579 and the surrounding fixes.
  • Every symbol this branch introducesstemWord, STEM_SUFFIXES, factStems, memoryStems, significantStems, FORGET_MATCH_SIMILARITY, GROQ_ON_DEMAND_TPM_LIMIT, promptFitsGroqOnDemand, overflowsPlatformGroq, MAX_LISTED — already resolves on main. Nothing here is unique.

It also conflicts in src/services/cat/memory.ts and __tests__/unit/cat/memory-forget.test.ts, and because main's version is the newer and larger one, resolving those conflicts would risk reverting improvements rather than adding any.

This is the parallel-session duplicate-work pattern: two sessions fixed the same bug on different branches, and the other one landed first. Closing so it stops blocking the auto-merge queue.

@catomean catomean closed this Aug 4, 2026
github-actions Bot pushed a commit that referenced this pull request Aug 29, 2026
…831)

The containment branch compared raw substrings both ways, and
MIN_FORGET_FRAGMENT_CHARS lets a four-character fact through. So "work" was
contained in "network", "framework", "coworking" and "homework": a user asking
Cat to forget "work" lost every memory that merely SPELLED it.

Reproduced before fixing — against a corpus of five memories, forgetting "work"
deleted three the user never mentioned. Cat then reported those three as
removed, accurately, which is exactly what makes this hard to notice: the
report is true, the deletion was not asked for.

This is the mirror of finding 8, shipped an hour ago, and the worse half. That
bug told someone a memory survived when it was gone; this one destroys memories
they never named, irreversibly.

Containment is now word-boundaried in both directions. Boundaries are checked
by CHARACTER CLASS, not a `\b` regex, because `\b` is ASCII-only in JavaScript:
it treats "café" as ending after "caf", so "café" would match inside
"cafétéria" while plain words behaved correctly. `\p{L}`/`\p{N}` cover the
accented alphabet the tokenizer in this file already speaks.

Deliberately NOT touched: the stemmer. "cafés" → "café", "weekends" →
"weekend", "photography" → "photographer" are the matches stemming exists for,
and a test now pins that they still fire — this narrows raw containment only.
One of my own test expectations was wrong on exactly that point and was
corrected rather than the code.

#563 finding 9. #555, which the finding said to coordinate
with, is closed.

73 Cat suites (1031 tests) and type-check green — the risk in narrowing a match
is under-matching, so the existing forget tests are the real check here.


Claude-Session: https://claude.ai/code/session_018waGt1ieA9TjpscqrbrnGb

Co-authored-by: Georgy Butaev <41178744+g-but@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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