fix: improve user-card lookup integration - #363
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughUpdates tenant metadata handling, prompt guidance, identity search ranking, and user-card lookup/listing so visible aliases and projected IDs resolve consistently. ChangesIdentity Lookup, Search Re-ranking, and Tenant Header
Sequence Diagram(s)sequenceDiagram
participant Model
participant get_user_card
participant LibravDBClient
participant findUserCardByAlias
Model->>get_user_card: user_id = "ExampleUser-1001"
get_user_card->>LibravDBClient: getUserCard("ExampleUser-1001")
LibravDBClient-->>get_user_card: { cardJson: null }
get_user_card->>findUserCardByAlias: listByMeta(type=user_card)
findUserCardByAlias-->>get_user_card: alias candidate with stable key
get_user_card->>LibravDBClient: getUserCard(canonicalId)
LibravDBClient-->>get_user_card: projected card
get_user_card-->>Model: { card: { ... }, updated_at, version }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/memory-tools.ts`:
- Around line 215-219: The identity search path in memory-tools.ts is too
narrow: overfetching only happens when shouldOverfetchForIdentityQuery(query)
matches the raw string, and userCardMatch is requiring all query tokens instead
of just the entity tokens. Update the search flow around
resolveResultLimit/searchMaxResults to also trigger overfetch when the query
explicitly indicates identity intent via kind or signals, and revise
userCardMatch to compare against the extracted entity tokens rather than the
full query token set so identity-card rows can be surfaced and promoted
correctly.
In `@src/tools/memory-recall.ts`:
- Around line 714-720: Restrict alias fallback in userCardAliasMatchesLookup so
it only considers visible alias/name/speaker/user-id fields and does not
tokenize provider, account type, or channel metadata. Update the lookup path in
memory-recall.ts by introducing or using a dedicated extractor like
extractUserCardAliasLookupText for the card text before calling identityTokens,
and keep the matching logic in userCardAliasMatchesLookup limited to those
alias-bearing labels.
- Around line 696-699: Tighten the user-card source check in the metadata
parsing path so only OpenClaw-origin records are accepted. In the logic around
the card JSON parsing and the alias/listing flow (the code that currently
inspects card.source and the list_user_cards fallback), reject records unless
source is exactly the OpenClaw marker string, rather than allowing missing or
non-string source values through. Update both the metadata acceptance branch and
the list_user_cards filtering path to use the same strict source validation so
foreign type=user_card entries never reach alias fallback.
In `@test/unit/memory-recall.test.ts`:
- Around line 66-74: The shared fixture helper userCardResult currently omits
source by default, which makes the positive memory-recall tests use source-less
cards unintentionally. Update userCardResult to default source to
"openclaw-user-cards" and keep the card_json construction using that source
unless a test explicitly passes a sentinel for missing metadata. Then adjust the
positive cases in memory-recall.test.ts to rely on the default source-bearing
fixture and only use the explicit missing-source value where that behavior is
being tested.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9bfc9755-1393-4ff8-bfb3-f2d0972ba3eb
📒 Files selected for processing (8)
src/libravdb-client.tssrc/memory-provider.tssrc/memory-tools.tssrc/tools/memory-recall.tstest/unit/libravdb-client.test.tstest/unit/memory-provider.test.tstest/unit/memory-recall.test.tstest/unit/memory-tools.test.ts
compoodment
left a comment
compoodment
left a comment
There was a problem hiding this comment.
Vale Review — PR #363
Quality: Q4/5 — sharp
Head: 4720e3c
Findings:
- src/memory-tools.ts:413 —
identityEntityTokensstill keeps common prompt-shape words likekind, anduserCardMatchthen requires every remaining query token to appear in the card identity text at lines 421-422. A documented identity question likewhat kind of <alias>will not promote the user-card hit, so prompt echoes can still outrank the actual card for one of the advertised lookup forms. major
Proof gaps: I did not rerun the full suite or a live daemon probe for this head; this is current-head source review against the reranking logic and added tests.
Verdict: request-changes — identity reranking is still too brittle for natural identity questions in the tool guidance.
– Vale
|
Thanks Vale, confirmed. The advertised Fixed in 7e86c5e by excluding Validation:
|
compoodment
left a comment
compoodment
left a comment
There was a problem hiding this comment.
Vale Review — PR #363
Quality: Q4/5 — sharp
Head: 7e86c5e
Findings:
- None
Proof gaps: Full suite and live daemon probe were not rerun here; review covered current metadata, diff, comments/reviews, CodeRabbit check, and current-head source around identity ranking and user-card alias fallback.
Verdict: approve — the new head fixes Vale's prior what kind of <alias> reranking failure and adds a focused regression for that prompt shape.
– Vale
|
Thanks for the PR Juan. The alias resolution idea — making 1. Tenant header rename is incorrect. The header was changed from 2. Unbounded scan on every card miss. 3. Source filtering silently drops cards. 4. Overfetch regex is too broad. 5. Prompt guidance became subjective. Changing from "Only use memory_search if the card is empty or missing" to "call memory_search when the card lacks enough profile notes" makes model behavior non-deterministic — different models will judge "enough" differently. 6. Too many unrelated changes in one PR. Five distinct behavioral changes (header rename, alias scanning, identity reranking, list dedup, prompt wording) should be separate PRs so each can be reviewed and tested independently. The alias resolution concept is worth revisiting as a focused PR — just the |
3 participants
Summary
list_user_cardsand filters foreign card sources.memory_searchby overfetching/reranking user-card and speaker hits above prompt echoes/tool artifacts.x-libravdb-tenant-key.Linked context
Closes #362
Real behavior proof
get_user_cardreturnednullfor a visible user-card alias even though the card existed under its stable projected OpenClaw speaker key; identitymemory_searchcould prefer prompt echoes/tool artifacts over the user-card hit.libravdb-memoryloaded from the packaged plugin artifact and a LibraVDB daemon over a Unix socket.get_user_card({ user_id: "ExampleUser-1001" }) -> { "card": null }, followed by an assistant response claiming the user card did not exist.Tests and validation
Regression coverage added/updated:
x-libravdb-tenant-keyget_user_cardresolves raw sender IDs to scoped projected user cardsget_user_cardresolves bullet-prefixed visible aliases from identity fieldslist_user_cardsdeduplicates:64d/:256dprojection variantsmemory_searchprefers user-card/speaker hits and excludes historical tool artifacts from the top result windowRisk checklist
listByMeta(type=user_card)when exactgetUserCardmisses.openclaw-user-cards, matches identity/profile fields only, and keeps note-only mentions from resolving aliases.Current review state
Ready for review. Please focus on whether the alias fallback and identity reranking are the right plugin-side invariant, and whether the tenant header spelling matches the daemon contract.
Summary
get_user_cardcan resolve direct IDs, raw sender IDs, and visible aliases from projected identity fields, while ignoring aliases found only in notes.list_user_cardsprojection variants like:64d/:256d, filtered to the canonicalopenclaw-user-cardssource, and canonicalized selection via per-card replacement rules.memory_searchby detecting identity-intent queries, conditionally overfetching, then reranking/slicing results so canonical user-card and speaker hits are promoted above prompt-echoes and historical tool artifacts.memorytool guidance so sparse user cards do not block follow-up recall when history/details/preferences or explicit memory requests are made.x-libravdb-tenant-keyvia a sharedTENANT_KEY_HEADER+createTenantInterceptor.Validation
list_user_cardsdeduplication across projection variants,memory_searchreranking/overfetch behavior (including preference over prompt echoes/tool artifacts).Complexity (Big O + “cyclomatic-ish” growth)
memory_search(identity-intent path): worst-case work shifts from mostly linear filtering to overfetch + rerank. WithRraw hits returned frommanager.search, the reranking step that sorts ranked candidates is O(R log R) (plus tokenization/feature extraction over snippets). Non-identity queries stay closer to O(R) for filtering/slicing.list_user_cards: deduplication uses map-based canonicalization; processing is O(N) over the returned daemon entries, with extra constant-factor parsing/selection per entry.get_user_card: when the direct card lookup is missing, the fallback scans projected stored entries fromlistByMeta(O(N)) and selects the best match via identity-token comparisons (additional work proportional to token/text length per candidate).src/memory-tools.ts: 84 (if=26,&&=7,||=17,ternary=22,catch=3)src/tools/memory-recall.ts: 168 (if=61,&&=13,||=19,ternary=44,catch=18)src/libravdb-client.ts: 70src/memory-provider.ts: 22