diff --git a/CHANGELOG.md b/CHANGELOG.md index 723b0d2..2a87690 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,29 @@ # Changelog +### Batch: entity-resolver-relationship-stats-543 (Issue #543) + +#### Added +- **Pronouns + relationship stats in turn-context entity injection** (nova-mind#543) โ€” the turn-context entity injection (`memory/plugins/turn-context/src/entity-resolver.ts`'s `formatEntityContext()`) previously showed only a 7-key allowlist of facts. It now also renders: + - `entities.pronouns` in the `๐Ÿ‘ค **Talking with:**` header, e.g. `๐Ÿ‘ค **Talking with:** Tabatha Janell Wilson (she/her)`. + - An optional trust suffix from `entities.trust_level` (free-text, e.g. `โ€” trust: friend`) โ€” suppressed when the value is `'unknown'` (the column default) or NULL, since that conveys no information. + - A new `๐Ÿ“Š Known contact:` stats line with unfiltered fact count, first-seen date (`entities.created_at`), and last message timestamp + `provider:external_chat_id` ref (from `channel_transcripts`/`channel_sessions`) โ€” falling back to `entities.last_seen` when no transcript row exists. + - `relationships/lib/entity-resolver/resolver.ts`'s `getEntityProfile()` return type changed from a bare `EntityFacts` map to `EntityProfile` (`{ facts, stats }`) โ€” a breaking signature change for any direct consumer. `resolver.ts` also gains a shared `mapDbEntity()` helper so `pronouns`/`trust_level`/`last_seen`/`created_at` ride on the existing cheap identifier-resolution query (`resolveEntity()`/`resolveEntityByIdentifiers()`) rather than a second round trip โ€” this means pronouns/trust/last-seen survive a `getEntityProfile()` stats-query timeout, since they're already on the resolved `Entity` before the stats race even begins. + - The stats query itself (unfiltered fact count + most recent transcript, via a single aggregate query with two `LEFT JOIN LATERAL`s) stays inside the existing 1s `Promise.race` timeout in `resolveEntityContext()`; on timeout or error it degrades to `{ facts: {}, stats: { factCount: 0, lastMessage: null } }`, matching the library's existing fail-closed contract. + - Honorific guard path (`resolveEntityForGuard()`) is unchanged โ€” it never triggers the stats query. + - `database/schema.sql`'s `entities.trust_level` column comment updated from an enum-style description to reflect that it is free-text, not enforced by a CHECK constraint. + +#### Fixed +- **`entities.last_seen` timezone-shift bug** (nova-mind#543) โ€” `resolveEntity()`/`resolveEntityByIdentifiers()` originally rendered the naive (no-tz) `entities.last_seen` timestamp with `AT TIME ZONE 'UTC'`, which Postgres interprets as "convert from the session's timezone to UTC" โ€” silently shifting the displayed value under any non-UTC session (e.g. `America/Chicago`). Fixed to render directly via `to_char(e.last_seen, 'YYYY-MM-DD HH24:MI "UTC"')` with no timezone conversion, matching how `created_at` is already rendered. See the RS-062 regression test in `relationships/lib/entity-resolver/test.ts` (run under `SET timezone='America/Chicago'`). + +#### Tests +- `memory/plugins/turn-context/src/entity-resolver.test.ts` (nova-mind#543) โ€” RS-001 through RS-062: full formatting matrix for `formatEntityContext()` (pronouns, trust suffix incl. `'unknown'`/NULL suppression, stats-line permutations, zero-fact/new-contact rendering, group-channel cache non-contamination), plus stats-query timeout/error-degradation behavior. +- `relationships/lib/entity-resolver/test.ts` (nova-mind#543) โ€” integration scaffold plus RS-062 (timezone regression under `SET timezone='America/Chicago'`). + +#### Issues Closed +- #543 โ€” Pronouns + relationship stats in turn-context entity injection + +See `relationships/CHANGELOG.md` for the entity-resolver library's own changelog entry. + ### Batch: pg-notify-alert-sender-508 (Issue #508) #### Fixed diff --git a/TEST-DESIGN-543-entity-resolver-relationship-stats.md b/TEST-DESIGN-543-entity-resolver-relationship-stats.md new file mode 100644 index 0000000..782ad09 --- /dev/null +++ b/TEST-DESIGN-543-entity-resolver-relationship-stats.md @@ -0,0 +1,278 @@ +# Test Design: Entity Resolver Pronoun Injection + Relationship/Stats Summary + +**Feature:** `getEntityProfile()` (or new sibling function) in `relationships/lib/entity-resolver/resolver.ts` (installed copy: `~/.openclaw/lib/entity-resolver/resolver.ts`), consumed by `resolveEntityContext()`/`formatEntityContext()` in `memory/plugins/turn-context/src/entity-resolver.ts`. +**Issue:** NOVA-Openclaw/nova-mind#543 (SE Workflow run #513, Step 3 โ€” QA Test Design) +**Author:** Gem, QA Lead +**Status:** Design only โ€” no implementation. Coder implements against this document. + +--- + +## 1. Scope & Approach + +### 1.1 Feature Under Test (from issue body + 2026-07-28 I)ruid directive comment) + +1. Inject `entities.pronouns` into the ๐Ÿ‘ค **Talking with:** block. +2. Add a relationship/stats summary line pulled cheaply at resolution time: + - `entity_facts` count for the entity + - Last message timestamp + reference pointer (most recent `channel_transcripts` row for `sender_entity_id` โ†’ `timestamp` + `provider:external_chat_id` from the joined `channel_sessions` row) + - `entities.trust_level` + - `entities.last_seen` + - `entities.created_at` โ†’ relationship age / "first seen" +3. Target format (from issue): + ``` + ๐Ÿ‘ค Talking with: Tabatha Janell Wilson (she/her) โ€” trust: friend + ๐Ÿ“Š Known contact: 48 facts ยท first seen 2026-01-30 ยท last message 2026-07-27 02:11 UTC (discord:1513392492651872306) + ``` +4. Single aggregate query preferred over N lookups; must fit inside the **existing 1s `Promise.race` timeout budget** in `resolveEntityContext()`. + +### 1.2 Where the Fix Lands + +- **Source of truth:** `/home/nova/nova-mind/relationships/lib/entity-resolver/resolver.ts` (+ `types.ts`, `index.ts` for exports). This is deployed/installed to `~/.openclaw/lib/entity-resolver/`. Both copies are currently byte-identical (`diff` confirmed) โ€” the deploy step is out of scope for this test design but **must be verified as a manual/CI step** so the installed copy doesn't drift from the fix (this drift already caused issues in this codebase's history; see `types.ts`/`resolver.ts` parity check in Definition of Done). +- **Consumer:** `/home/nova/nova-mind/memory/plugins/turn-context/src/entity-resolver.ts` โ€” `formatEntityContext()` (pure formatting) and `resolveEntityContext()` (orchestration + timeout race). +- **New test file (unit tier):** `memory/plugins/turn-context/src/entity-resolver.test.ts` (does not exist yet โ€” new file, following the `node:test` + `node:assert/strict` convention already used in `index.test.ts` and `honorific-guard.test.ts`). +- **New/extended test file (integration tier, live-DB):** `relationships/lib/entity-resolver/test.ts` (existing file, extend with a new `runRelationshipStatsTests()` block following the existing seed/cleanup/`check()` pattern already used for `--irc-tests`). + +### 1.3 In Scope +- `formatEntityContext()` output shape/text for all pronoun and stats-line permutations (unit, pure function โ€” no DB needed for most cases). +- The new aggregate query (whatever it's named, e.g. `getEntityRelationshipStats()` or folded into `getEntityProfile()`) โ€” correctness, single-round-trip design, index usage, timeout behavior. +- Interaction with the existing 1s `Promise.race` in `resolveEntityContext()` โ€” does the new query run inside the same race, a separate race, or sequentially after the existing facts race (affects total worst-case latency โ€” flagged as an open question below). +- `resolveEntityForGuard()` non-regression (#421) โ€” the lightweight guard path must NOT pick up the new query. +- Cache key behavior (#150) โ€” cached `Entity` object shape changes (now carries pronouns/trust_level/last_seen/created_at); must not leak across senders in group channels. +- Privacy/visibility decision for the fact count (decided below, ยง5). + +### 1.4 Out of Scope +- `resolveEntity()` / `resolveEntityByIdentifiers()` identifier-matching logic itself โ€” unchanged, already covered by `--irc-tests` and other existing suites. +- Extending the profile-facts allowlist (nova-mind#543's own "Proposal option 1" โ€” widening the 7-key allowlist) โ€” that is a **separate, not-mutually-exclusive** proposal per the issue and is NOT part of this spec expansion. Do not conflate; flag if implementation scope-creeps into it. +- IRC/Discord/Slack channel-ref formatting nuances beyond `provider:external_chat_id` (see open question on thread refs). +- Daily-report consumption of the new fields (downstream feature, separate issue potential). + +### 1.5 Test Levels + +1. **Unit tests** (`entity-resolver.test.ts`, `node:test` + `node:assert/strict`, matching `honorific-guard.test.ts` convention): + - `formatEntityContext()` is a pure function โ€” test with hand-built `Entity`/`EntityFacts`-plus-stats objects, no DB, no mocking needed. This is the primary vehicle for the formatting matrix (ยง4.1โ€“4.3). + - `resolveEntityContext()` / `resolveEntityForGuard()` orchestration tests require a seam to inject/mock the pg client or the new stats-fetch function, since `getDbPool()` is a module-level singleton with no DI hook today (same limitation flagged in the prior `agent_domains.keywords` test design for `loadDomains()`). **Design note for Coder:** either (a) export the new stats-fetch function separately so the turn-context test file can mock it at the module-import boundary (`node:test`'s `mock.module()`, Node 22+, confirmed compatible โ€” matches the recommendation already given for `domain-identifier.test.ts`), or (b) accept an injectable query-fn parameter. Prefer (a) โ€” keeps `resolver.ts`'s public API surface stable. +2. **Integration tests** against **nova-staging** (SSH `nova-staging@localhost` โ€” never production; see standing instruction) โ€” validates the real aggregate query (JOIN correctness, `LATERAL` join behavior if used, index usage via `EXPLAIN`, actual PG error semantics for timeout/connection failure). Extend `relationships/lib/entity-resolver/test.ts` with a new test block following the existing `testEntityIds` seed/cleanup pattern (use a fresh non-overlapping ID range, e.g. `99101`โ€“`99110`, to avoid collision with the existing `--irc-tests` range `99001`โ€“`99007`). + +### 1.6 Entry Criteria +- Source change implemented per requirements in ยง1.1 +- `npm run typecheck` clean in both `relationships/lib/entity-resolver/` and `memory/plugins/turn-context/` +- Staging DB reachable with ability to seed/clean disposable test entities, facts, channel_sessions, and channel_transcripts rows + +### 1.7 Exit Criteria +- All test cases below pass +- `npm run build` clean in both packages; `dist/` updated for `turn-context` (per repo convention โ€” `dist/entity-resolver.js` must reflect source changes, confirmed via the existing `dist/` mirror already present) +- Installed copy at `~/.openclaw/lib/entity-resolver/resolver.ts` matches the `nova-mind` repo source (parity check โ€” no drift) +- No new warnings/errors in plugin logs when run against healthy `nova_memory` schema (manual smoke check) +- Total P99 latency contribution of the new query stays within the existing 1s budget under realistic staging load (see ยง4.6 performance cases) + +--- + +## 2. Preconditions Common to All Cases + +- `Entity` type (in `types.ts`) will need to widen to carry `pronouns`, `trustLevel`, `lastSeen`, `createdAt` (naming TBD by Coder โ€” flagged as open question). Tests must be written against whatever field names Coder settles on; this document uses `pronouns`, `trustLevel`, `lastSeen`, `createdAt` as placeholders. +- The turn-context `entity-resolver.ts` dynamically imports the resolver lib at runtime (`ensureEntityResolver()`); unit tests for `formatEntityContext()` do NOT need this import path since the function is pure and can be tested directly against manually constructed inputs โ€” no live import/mocking required for the formatting matrix. +- Integration-tier tests must seed disposable `channel_sessions` + `channel_transcripts` rows in addition to `entities`/`entity_facts`, and clean up all four tables in `finally`/`cleanup()` (extending the existing pattern, which currently only touches `entities`/`entity_facts`). +- Cache tests (ยง4.5) must call `clearCache()` before each test to avoid cross-test contamination (same requirement noted implicitly by the existing cache test in `test.ts`). + +--- + +## 3. Data Model Reference (from schema, for test-input construction) + +- `entities.pronouns` โ€” `varchar(50)`, nullable, no default. +- `entities.trust_level` โ€” `varchar(20)`, **default `'unknown'`** (never NULL for entities created after this default existed; may be NULL for pre-existing rows if explicitly set NULL โ€” treat both as a possible edge case). +- `entities.last_seen` โ€” `timestamp`, nullable, no default (NULL until first observed interaction is recorded by whatever process maintains it โ€” brand-new entities will have NULL here even if `created_at` is populated). +- `entities.created_at` โ€” `timestamp DEFAULT CURRENT_TIMESTAMP` โ€” effectively never NULL. +- `entity_facts` โ€” one row per fact; `entity_id` FK; count via `COUNT(*) WHERE entity_id = $1` uses `idx_entity_facts_entity`. +- `channel_transcripts.sender_entity_id` โ€” nullable `bigint` FK to `entities.id`; `(sender_entity_id, timestamp)` composite index exists (`idx_channel_transcripts_sender_entity`) โ€” ideal for `ORDER BY timestamp DESC LIMIT 1` per entity. +- `channel_sessions` โ€” `provider`, `external_chat_id`, `external_thread_id` (nullable). The example ref `discord:1513392492651872306` looks like `provider:external_chat_id`. **Open question:** does `external_thread_id` need to be included when present (e.g. a Discord thread or Slack thread reply)? See ยง6. + +--- + +## 4. Test Cases + +Numbering: `RS-0xx` (Relationship Stats). + +### 4.1 Happy Path โ€” Pronoun Injection + +**RS-001: Pronouns present โ†’ rendered in parenthetical after display name** +- Input: `entity = { name: "Tabatha Janell Wilson", fullName: "Tabatha Janell Wilson", pronouns: "she/her", ... }`. +- Expected: `formatEntityContext()` output line 1 is `๐Ÿ‘ค **Talking with:** Tabatha Janell Wilson (she/her)` (exact spacing/parenthetical placement per the issue's example โ€” note the example shows plain-text `๐Ÿ‘ค Talking with:` without markdown bold, but the *existing* code uses `**Talking with:**`; Coder must preserve existing markdown bold convention unless explicitly told to drop it โ€” flag as open question, see ยง6). +- Pass/fail: exact string match on the header line. + +**RS-002: Pronouns present + trust_level present โ†’ trailing " โ€” trust: X" suffix** +- Input: `pronouns: "she/her"`, `trustLevel: "friend"`. +- Expected: header line ends with ` โ€” trust: friend` per the issue's example format. +- Pass/fail: exact string match. + +**RS-003: Full stats line renders all four data points in the documented order** +- Input: `factCount: 48`, `createdAt: 2026-01-30T...`, `lastMessage: { timestamp: 2026-07-27T02:11:00Z, ref: "discord:1513392492651872306" }`. +- Expected: second line reads `๐Ÿ“Š Known contact: 48 facts ยท first seen 2026-01-30 ยท last message 2026-07-27 02:11 UTC (discord:1513392492651872306)` โ€” verify separator (`ยท`), date formats (`YYYY-MM-DD` for first-seen, `YYYY-MM-DD HH:MM UTC` for last-message), and parenthetical ref. +- Pass/fail: exact string match (format-sensitive โ€” this is a golden-output test). + +**RS-004: Existing fact-key bullet list (from `getEntityProfile()`'s existing 7-key allowlist) still renders below the new stats line, unchanged** +- Input: entity with pronouns + stats + at least one existing profile fact (e.g. `timezone: "America/Chicago"`). +- Expected: `โ€ข **Timezone:** America/Chicago` bullet still appears, in its existing position/format, after the new stats line. This is the regression guard for the untouched allowlist-fact rendering path. +- Pass/fail: bullet list content and position unchanged from pre-fix behavior. + +### 4.2 Edge Cases โ€” Missing/Null Fields + +**RS-010: NULL pronouns โ†’ no parenthetical, header line degrades gracefully to name only** +- Input: `pronouns: null` (or field absent). +- Expected: `๐Ÿ‘ค **Talking with:** Tabatha Janell Wilson` โ€” no trailing `()`, no dangling space, no `undefined`/`null` string literal leaking into output. +- Pass/fail: exact string match; explicit assertion that output does NOT contain the substrings `"null"`, `"undefined"`, or `"()"`. + +**RS-011: NULL trust_level โ†’ header line has no " โ€” trust: ..." suffix (vs. rendering "trust: unknown")** +- **Decision required from Coder, flagged in ยง6.** This document recommends: only render the trust suffix when `trust_level` is a *meaningful* value (i.e., NOT `'unknown'` and NOT NULL), since `'unknown'` is the column default and conveys no information โ€” showing "trust: unknown" for every never-assessed entity is noise, not signal. Test asserts the recommended behavior; if Coder chooses differently, this test must be updated to match and the decision documented in code comments. +- Input: `trustLevel: "unknown"` (the DB default) and separately `trustLevel: null`. +- Expected (recommended): header line omits the trust suffix entirely in both cases. +- Pass/fail: header line has no `" โ€” trust:"` substring for either input. + +**RS-012: Zero entity_facts, but transcripts + entity metadata present โ†’ stats line renders "0 facts", not omitted** +- Input: `factCount: 0`, other stats populated. +- Expected: `๐Ÿ“Š Known contact: 0 facts ยท first seen ... ยท last message ...` โ€” the stats line still renders (it's not solely about the fact count; last-seen/trust/age are independently useful) unless Coder decides zero-facts-and-zero-transcripts should suppress the whole line (see RS-014). +- Pass/fail: stats line present, `"0 facts"` literal substring, no crash. + +**RS-013: No channel_transcripts rows for entity (never messaged, or pre-dates transcript logging) โ†’ "last message" segment omitted or shows a defined fallback** +- Input: `lastMessage: null` (aggregate query's LEFT JOIN found no row). +- Expected: stats line omits the `ยท last message ... (...)` segment cleanly (no trailing `ยท` with nothing after it, no `null`/`undefined` leaking). Recommended rendering: `๐Ÿ“Š Known contact: 48 facts ยท first seen 2026-01-30` (stats line ends after first-seen). +- Pass/fail: no dangling separator, no `null`/`undefined` substrings, first-seen segment still present if `createdAt` is available. + +**RS-014: Brand-new entity โ€” zero facts, NULL last_seen, no transcripts, created_at = now (degenerate case, the actual "new Discord contact" scenario from the issue's root-cause narrative)** +- Input: `factCount: 0`, `lastSeen: null`, `lastMessage: null`, `createdAt: `, `trustLevel: "unknown"`, `pronouns: null`. +- Expected: This is the exact inverse of the bug this feature fixes โ€” a genuinely new contact SHOULD render as sparse/new-looking (e.g. `๐Ÿ“Š Known contact: 0 facts ยท first seen 2026-07-28`), distinguishable from Tabatha's rich 48-fact example. No crash, no leaked nulls, output remains well-formed. This test is the acceptance-criteria anchor: it proves the fix doesn't accidentally make every entity look identical regardless of relationship depth โ€” the whole point of the feature is differentiating "known contact" from "brand new." +- Pass/fail: well-formed degenerate output; visually/structurally distinguishable from RS-003's rich example (fewer clauses in the stats line). + +**RS-015: Entity resolved but has `last_seen` populated, yet zero transcript rows exist (data inconsistency: last_seen updated by a path other than transcript logging, e.g. manual entity edit or a different presence-tracking mechanism)** +- Input: `lastSeen: `, `lastMessage: null`. +- Expected: Clarify whether "last message" and "last_seen" are the same concept or two independent signals. Per the issue, `last_seen` is its own bullet-worthy fact type separate from the transcript-derived last-message pointer โ€” but the issue's example format only shows "last message ... (ref)", not a separate `last_seen` mention. **Open question flagged in ยง6:** does `entities.last_seen` get its own clause, or is it superseded/replaced by the transcript-derived last-message timestamp for display purposes (keeping `last_seen` as a raw DB field used only if transcripts are unavailable)? This test should assert whichever resolution is chosen, and must not silently drop `last_seen` data if the design intends it to be surfaced independently. +- Pass/fail: matches documented design decision; no silent data loss if `last_seen` is meant to always surface. + +### 4.3 Edge Case โ€” Large Fact Volume + +**RS-020: Entity with thousands of facts (fact count = 4127, arbitrary large N) โ†’ count renders correctly, no truncation, no performance cliff** +- Input (integration tier, staging): seed 4000+ `entity_facts` rows for one disposable test entity. +- Expected: `COUNT(*)` aggregate returns the exact integer (not capped, not approximated); stats line renders `"4127 facts"` verbatim; query completes well within budget (see ยง4.6 for the explicit timing assertion). +- Pass/fail: exact count; query uses `idx_entity_facts_entity` (verify via `EXPLAIN` in the integration test โ€” assert `Index Scan` or `Index Only Scan`, not `Seq Scan`, on `entity_facts`). + +### 4.4 Error Conditions + +**RS-030: Aggregate/stats query times out โ†’ graceful degradation to current (pre-fix) behavior** +- Precondition (unit tier, mocked): stats-fetch function/query hangs past the 1s budget. +- Input: call `resolveEntityContext()`. +- Expected: `Promise.race` resolves with the timeout fallback (empty stats, matching the existing `getEntityProfile()` timeout pattern which resolves to `{}` on timeout โ€” not a rejection). The ๐Ÿ‘ค block still renders with at least the display name; pronouns should ALSO gracefully degrade to absent if pronouns are fetched via the same query. **Design-critical open question (ยง6):** if pronouns are fetched via the SAME query as the stats (single aggregate query, per the issue's stated preference), then a stats-query timeout also loses pronouns โ€” meaning pronoun display becomes dependent on the same 1s race as the (heavier) stats query, rather than being cheap/always-available. Verify Coder's actual design and adjust this test's expected pronoun behavior accordingly. If pronouns are instead fetched as part of the cheaper, already-existing entity-identification query (in `resolveEntityByIdentifiers`/`resolveEntityOnly`), pronouns should survive a stats-query timeout independently โ€” this is the recommended design (see ยง6) and this test should assert pronouns ARE still present even when the stats portion times out, IF that's the chosen architecture. +- Pass/fail: no exception propagates; entity name always renders; behavior for pronouns-under-timeout matches whichever architecture is chosen (documented, not left ambiguous). + +**RS-031: Pool/connection error on the stats query (e.g., pool exhausted, ECONNREFUSED) โ†’ does not crash `resolveEntityContext()`, matches existing `getEntityProfile()` error-catch pattern** +- Precondition (unit tier, mocked): stats-fetch throws a generic connection error (not a timeout, a real rejection). +- Input: call `resolveEntityContext()`. +- Expected: error is caught (matching the existing `try/catch` around `getEntityProfile()` in `resolveEntityContext()`), logged via `console.error`, and the function returns with base entity info only (name, possibly pronouns depending on architecture) โ€” never an unhandled rejection, never a thrown error escaping to the caller. +- Pass/fail: no unhandled rejection; `console.error` called with an identifiable message; text output well-formed with base info only. + +**RS-032: Aggregate query itself throws a genuine SQL error (e.g., a bad JOIN post-migration-drift) โ€” must not be silently swallowed into a false "no stats" without any log trace** +- Precondition: mock stats-fetch to throw a `42703 undefined_column`-style error (simulating schema drift, distinct from RS-030/031's transient conditions). +- Input: call `resolveEntityContext()`. +- Expected: same graceful-degradation behavior as RS-031 (no crash), BUT the error must be logged distinctly enough to be diagnosable (not conflated with "entity has no stats yet" โ€” a silent empty stats line for a genuine schema-drift bug would be a regression of the exact class of bug that motivated the original `agent_domains.keywords` fix). Recommend: log the raw error message via `console.error`, same pattern as existing `getEntityProfile()` catch block. +- Pass/fail: error logged with actionable detail; behavior does not regress into permanently-silent failure. + +### 4.5 Domain-Specific Scenarios + +**RS-040: Group-channel cache key behavior (#150 non-regression) โ€” cached `Entity` object now carries pronouns/trust/last_seen/created_at; must not cross-contaminate senders sharing a `sessionKey`** +- Precondition: `clearCache()`. Two distinct entities (A: `pronouns: "she/her"`, `trustLevel: "friend"`; B: `pronouns: "he/him"`, `trustLevel: "unknown"`) both resolving under the same `sessionKey` (simulating a group channel) but different `senderId`. +- Input: `resolveEntityContext(sessionKey, { senderId: A_id, ... })` then `resolveEntityContext(sessionKey, { senderId: B_id, ... })`. +- Expected: cache keys are `sessionKey:A_id` and `sessionKey:B_id` (existing `#150` fix) โ€” entity A's cached object (now including her pronouns/trust/stats) must never be returned for sender B's lookup, and vice versa. This is a direct extension of the existing #150 regression test but now covers the WIDER cached object (more fields = more surface area for a stale-cache leak to be embarrassing, e.g. showing sender B "trust: friend" and sender A's pronouns). +- Pass/fail: B's resolved text/pronouns/trust never equal A's; both correct after both calls. + +**RS-041: Cache TTL โ€” stats/pronoun data does not silently go stale beyond the existing 30-minute cache TTL in an unexpected way** +- Precondition: entity cached with `trustLevel: "unknown"`; DB `trust_level` updated post-cache to `"friend"` (simulating an in-session trust upgrade). +- Input: `resolveEntityContext()` called again within the 30-min TTL. +- Expected: returns the STALE cached `trustLevel: "unknown"` (matches existing cache semantics โ€” this is not a new bug, just confirming the wider cached object doesn't change TTL behavior). Document this as expected/known staleness, not a defect โ€” flagged so nobody "fixes" it as an unrelated surprise later. +- Pass/fail: stale value returned as expected (negative-space regression test โ€” asserts the OLD behavior is preserved, not violated by the new fields). + +**RS-042: Honorific-guard path (#421) non-regression โ€” `resolveEntityForGuard()` must remain lightweight and NOT trigger the new stats query** +- Precondition: mock/spy on the stats-fetch function. +- Input: call `resolveEntityForGuard(sessionKey, info)` directly (the `helperConfig.entity_resolver === false` path in `index.ts` that bypasses the full `resolveEntityContext()`). +- Expected: stats-fetch function is NEVER called; `resolveEntityForGuard()` returns only `{ entityId, displayName }` as before โ€” zero added latency, zero added DB round-trips. This is the most important non-regression case: the guard exists specifically to be cheap and always-on (per its docstring), and accidentally wiring the new stats query into the shared `resolveEntityOnly()` helper (which BOTH `resolveEntityContext` and `resolveEntityForGuard` call) would silently regress guard latency on every single turn, not just when `entity_resolver` is enabled. +- Pass/fail: stats-fetch call count === 0 after calling `resolveEntityForGuard()`; guard output shape unchanged (still just entityId/displayName, no pronouns/trust bleeding into the guard's return type). + +**RS-043: Honorific-guard text itself is unaffected by pronoun/trust data โ€” `buildHonorificGuard()` still only consumes `entityId`/`agentId`/`displayName`** +- Input: any entity with rich stats data, run through the full `index.ts` pipeline. +- Expected: `buildHonorificGuard()`'s output string is byte-identical to pre-fix behavior for the same `entityId`/`agentId`/`displayName` inputs โ€” it must not start referencing pronouns or trust level (out of scope for that subsystem; confirms no accidental scope creep across the two features that happen to share the same underlying entity resolution). +- Pass/fail: exact string match against pre-fix golden output for the same 3 inputs (reuse existing `honorific-guard.test.ts` fixtures). + +**RS-044: Privacy/visibility โ€” decision and test for whether `entity_facts` count respects `visibility`/`privacy_scope`** +- **Decision (documented here per the task's explicit ask):** The relationship-stats fact count should be **unfiltered by `visibility`/`privacy_scope`** โ€” i.e., `COUNT(*) FROM entity_facts WHERE entity_id = $1` with no visibility predicate. + - **Rationale:** `entity_facts.visibility` and `privacy_scope` govern who ELSE (which other entities/agents) may see a fact ABOUT this entity โ€” they are an exposure control for third parties, not a control on whether the agent talking directly to entity X may know "how much do I know about the person I'm currently talking to." The injected ๐Ÿ‘ค block is agent-internal context, never echoed back to the user or to any other entity in the conversation. Filtering the count by visibility would produce a systematically LOWER number than reality whenever an entity has any `private`/`trusted`-scoped facts (which are exactly the highest-trust, most relationship-relevant facts โ€” often the ones that should count MOST toward "how well do we know them"). Filtering here would actively undermine the feature's stated goal ("indicator as to how well you know them"). + - **Caveat / residual risk to flag to Coder and I)ruid:** this reasoning holds for the 1:1 case. In a **group channel**, if the injected context for sender A were ever accidentally surfaced to sender B (the exact class of bug #150 was created to prevent), an unfiltered count could indirectly signal "there are N private facts about this person" to the wrong audience โ€” though it leaks only a COUNT, not fact content, so the blast radius is low. Given the existing #150 cache-key fix already prevents cross-sender context leakage at the injection layer, this residual risk is judged acceptable, but it should be explicitly acknowledged rather than assumed away. +- Test: seed one entity with a mix of `visibility = 'public'`, `'trusted'`, and `'private'` facts (e.g., 20 public + 15 trusted + 13 private = 48, matching the issue's example number for a satisfying callback). Assert the rendered fact count is **48** (all facts, unfiltered), not a lower filtered number. +- Pass/fail: count reflects all rows regardless of `visibility`/`privacy_scope`, matching the documented decision. If Coder implements filtering instead (disagreeing with this recommendation), this test must be explicitly updated and the rationale for the reversal documented in the PR โ€” silent divergence from this decision is not acceptable per the task's "decide and document" requirement. + +### 4.6 Performance / Timeout Budget + +**RS-050: Aggregate query completes well within the 1s `Promise.race` budget under normal conditions (staging, warm connection pool)** +- Precondition (integration tier): seeded entity with realistic data volume (dozens of facts, hundreds of transcript rows). +- Input: time the aggregate query directly (not just the race-wrapped call) via `console.time`/`process.hrtime` in the integration test. +- Expected: query completes in low tens of milliseconds (well under the 1000ms budget โ€” recommend asserting `< 200ms` as a generous regression-catching ceiling, not a tight SLA) given the covering indexes identified in ยง3. +- Pass/fail: measured duration `< 200ms` on staging under normal load; flag as a soft/informational assertion if staging load is variable, but must be re-run and confirmed manually before sign-off if it fails. + +**RS-051: `EXPLAIN` confirms index usage for both the fact-count subquery and the last-transcript lookup โ€” no sequential scans on `entity_facts` or `channel_transcripts`** +- Input: `EXPLAIN (FORMAT JSON) ` against staging with realistic row counts (seed enough rows, e.g. 5,000+ unrelated `entity_facts` rows across other entities, to make a seq-scan-vs-index-scan difference actually observable). +- Expected: `Index Scan`/`Index Only Scan`/`Bitmap Index Scan` on `idx_entity_facts_entity` for the count; `Index Scan` on `idx_channel_transcripts_sender_entity` for the last-message lookup (LIMIT 1 with ORDER BY DESC should use the index's sort order directly, avoiding a full sort). +- Pass/fail: no `Seq Scan` node present for either table in the plan. + +**RS-052: Combined worst-case latency โ€” stats query + existing facts query (if run sequentially rather than merged/parallel) does not exceed the 1s budget for THIS resolution, and does not silently double the effective timeout ceiling to 2s** +- **Design-critical open question (ยง6):** does the new stats query run (a) merged into a single query alongside the existing `getEntityProfile()` allowlist-facts query, (b) as a second query racing its OWN independent 1s timeout (making the effective worst case sequentially up to 2s if `resolveEntityContext()` awaits both races in sequence rather than `Promise.all`-ing them), or (c) replacing the existing `getEntityProfile()` call entirely? +- Input/Expected: whichever architecture is chosen, this test asserts the TOTAL added latency contribution from entity-resolution stays bounded โ€” recommend Coder merge into a single query per the issue's explicit stated preference ("single aggregate query preferred"), and recommend wrapping that ONE query in the existing `Promise.race(..., 1000ms)` pattern (not a second independent race), so the worst-case ceiling for entity resolution as a whole does NOT silently grow from ~1s to ~2s. +- Pass/fail: total entity-resolution wall-clock time (both the identifier-resolution query AND whichever facts/stats query(ies) run) stays within a single 1s degradation ceiling, not stacked ceilings. This should be asserted with a slow-query mock that would time out if two full 1s races were stacked sequentially, but resolves in time if merged/parallelized correctly. + +### 4.7 Formatting / Boundary Values + +**RS-060: Fact count boundary โ€” exactly 1 fact ("1 fact" singular vs "1 facts" โ€” grammar check)** +- Input: `factCount: 1`. +- Expected: **Open question for Coder** โ€” does the implementation pluralize correctly ("1 fact") or keep it simple ("1 facts", matching the issue's own example which never demonstrates the singular case)? Recommend correct singular/plural handling for polish, but this is non-blocking; document whichever choice is made and lock the test to it. +- Pass/fail: matches documented choice โ€” either is acceptable as long as it's consistent and intentional, not accidental. + +**RS-061: `trust_level` enum boundary values โ€” all documented values render without error** +- Input: iterate `trustLevel` over `'owner'`, `'admin'`, `'user'`, `'unknown'`, `'untrusted'` (per the column comment's documented value set โ€” note this is NOT a DB CHECK constraint, just a documented convention, so also test an arbitrary/unexpected string value like `'friend'` since the issue's own example uses `"friend"`, which is NOT in that documented list). +- Expected: all render as `โ€” trust: X` (except `'unknown'`/NULL, per RS-011) with no crash, no special-casing that breaks on the undocumented-but-used `"friend"` value. This surfaces a real inconsistency worth flagging: the column comment's documented value set (`owner, admin, user, unknown, untrusted`) does NOT include `"friend"`, which is exactly what the issue's own target-format example uses. **Flag to I)ruid/Coder:** either the column comment is stale/incomplete, or `trust_level` is freely-editable text without an enforced value set and the example is using a value nobody previously documented. The implementation must not assume a closed enum when rendering โ€” treat `trust_level` as an arbitrary non-empty string. +- Pass/fail: no crash/no special-casing failure for `"friend"` or any other non-listed value; confirms implementation doesn't hardcode a value-checking switch statement that would silently drop unrecognized trust levels. + +**RS-062: Timestamp formatting โ€” `first seen` uses date-only (`YYYY-MM-DD`), `last message` uses date+time+UTC marker โ€” verify timezone handling is explicit and not silently server-local** +- Input: `createdAt` and `lastMessage.timestamp` as UTC `timestamptz` values from Postgres (both `entities.created_at` and `channel_transcripts.timestamp` are stored as timestamp/timestamptz โ€” verify exact column types don't produce a timezone-conversion bug when rendered, since `entities.created_at` is plain `timestamp` (no tz) while `channel_transcripts.timestamp` is `timestamptz`). +- Expected: output explicitly labeled `UTC` for the last-message clause (per the issue's example) and consistent, unambiguous date rendering for first-seen โ€” no accidental server-local-timezone conversion silently applied to the `timestamp` (non-tz) `created_at` column, which could misrepresent the "first seen" date depending on server TZ configuration vs. the DB session TZ. +- Pass/fail: explicit UTC labeling present for last-message; first-seen date is stable/correct regardless of process `TZ` env var (test by running under `TZ=America/Chicago` vs `TZ=UTC` and asserting identical output โ€” this is a real, plausible bug source given the column-type mismatch). + +--- + +## 5. Privacy/Visibility Decision Summary (restated for visibility) + +**Decision:** Fact count is unfiltered by `visibility`/`privacy_scope` โ€” see full rationale in RS-044. This section exists so the decision isn't buried only in a single test case. + +--- + +## 6. Open Questions (per instructions โ€” documented rather than blocking) + +1. **Markdown bold in the ๐Ÿ‘ค header line:** the issue's example shows plain `๐Ÿ‘ค Talking with:` (no bold), but the existing code renders `๐Ÿ‘ค **Talking with:**`. Does the fix intentionally drop the bold markdown, or is the issue's example just informal shorthand? Recommend preserving existing bold convention unless explicitly told otherwise โ€” flagged in RS-001. +2. **Pronoun fetch path vs. stats-query timeout coupling:** should pronouns be fetched as part of the SAME aggregate/stats query (simplest, per the issue's "single aggregate query" preference, but couples pronoun display to the heavier query's timeout fate), or as part of the cheaper, already-existing identifier-resolution query in `resolveEntityByIdentifiers`/`resolveEntityOnly` (pronouns survive a stats-query timeout independently)? Recommend the latter for resilience โ€” pronouns are a near-zero-cost addition to a query that already runs and already succeeds before the stats race even begins. Flagged in RS-030. +3. **`entities.last_seen` vs. transcript-derived last-message timestamp:** are these the same displayed value, or two independent data points? The issue lists both `last_seen` and "last message timestamp" as separate bullet items in the requirements list, but the example format only shows one timestamp clause. Does `last_seen` need its OWN clause in the rendered output, or is it functionally superseded by the transcript-derived value for display (while still being fetched/available for other consumers)? Flagged in RS-015. +4. **Channel/session ref granularity:** should the parenthetical ref include `external_thread_id` when present (e.g., `discord:1513392492651872306:987654321` for a thread), or always just `provider:external_chat_id`? The issue's example only shows the two-part form. Flagged in ยง3. +5. **Query architecture (merged vs. dual-race):** does the new stats data get merged into ONE query with the existing `getEntityProfile()` allowlist-facts query (replacing/extending that function), or does it run as an ADDITIONAL, separate query? If separate, is it wrapped in its own independent `Promise.race(1000ms)`, risking an effective 2s worst-case ceiling for entity resolution as a whole? Recommend a single merged query wrapped in the EXISTING race. Flagged in RS-052 โ€” this is the highest-risk architectural decision from a performance-budget-compliance standpoint and should be resolved explicitly, not left to fall out of implementation convenience. +6. **Singular/plural fact-count grammar** ("1 fact" vs "1 facts") โ€” cosmetic, non-blocking, flagged in RS-060. +7. **`trust_level` value-set inconsistency** โ€” the column comment documents `owner, admin, user, unknown, untrusted` but the issue's own example uses `"friend"`, which isn't in that list. Is `trust_level` actually free text in practice, or does the documented comment need updating, or does `"friend"` in the issue's example reflect an as-yet-unshipped separate change to the value set? Flagged in RS-061 โ€” implementation must not assume a closed set either way. +8. **Deploy/parity step:** how does a fix to `relationships/lib/entity-resolver/resolver.ts` get synced to the installed `~/.openclaw/lib/entity-resolver/resolver.ts` copy consumed at runtime? Not previously documented in the issue or in either file's README โ€” this should be confirmed as part of the PR (a build/copy step, a symlink, or manual sync) so the fix doesn't silently fail to take effect at runtime despite passing all repo-local tests. This is a deployment-hygiene gap, not a QA scope item per se, but sign-off should not proceed without this being confirmed. + +--- + +## 7. Definition of Done + +All of the following must be true before this fix is considered QA-approved and ready for sign-off: + +1. **All test cases RS-001 through RS-062 pass** โ€” unit tier via `npm test` (`tsx --test src/**/*.test.ts`) in `memory/plugins/turn-context/`, plus the RS-020/RS-050/RS-051 integration cases executed against nova-staging (extending `relationships/lib/entity-resolver/test.ts`). +2. **Zero regressions:** existing `honorific-guard.test.ts` (RS-042, RS-043) and `index.test.ts` (`buildPromptResult` placement tests) continue to pass unmodified; existing `--irc-tests` block in `relationships/lib/entity-resolver/test.ts` continues to pass unmodified. +3. **Performance budget compliance confirmed** (RS-050, RS-051, RS-052) โ€” the single highest-risk requirement from the issue ("must fit existing 1s timeout budget"); must not be waved through on "looks fine in dev," needs actual staging measurement. +4. **Privacy/visibility decision (ยง5) is implemented as documented, or explicitly reversed with documented rationale** (RS-044) โ€” silent divergence is not acceptable. +5. **All open questions in ยง6 are explicitly resolved by Coder** (documented in code comments or PR description), and any test case whose expected behavior depended on an open question is updated to match the chosen resolution before merge. +6. **`npm run typecheck` and `npm run build` clean** in both `relationships/lib/entity-resolver/` and `memory/plugins/turn-context/`; `dist/` reflects the change. +7. **Installed-copy parity confirmed** (ยง6, item 8) โ€” `~/.openclaw/lib/entity-resolver/resolver.ts` matches the repo source post-fix, verified via `diff`. +8. **Manual smoke test** against real `nova_memory` (staging) confirms: (a) a real known entity (e.g., a seeded test analog of "Tabatha") renders pronouns + stats correctly, (b) a brand-new entity renders the sparse/degenerate form (RS-014) without error, (c) no new warnings/errors appear in plugin logs during normal multi-turn operation. + +Any failing test blocks PR sign-off. Any test that cannot be executed due to missing staging access must be explicitly waived by I)ruid with the gap documented, not silently skipped. diff --git a/database/schema.sql b/database/schema.sql index a618359..ce8137b 100644 --- a/database/schema.sql +++ b/database/schema.sql @@ -1032,7 +1032,7 @@ COMMENT ON COLUMN entities.collaborate IS 'If true, collaborate with this entity COMMENT ON COLUMN entities.collaboration_scope IS 'full | domain-specific | supervised - determines collaboration breadth'; -COMMENT ON COLUMN entities.trust_level IS 'Trust level for confidence scoring: owner, admin, user, unknown, untrusted'; +COMMENT ON COLUMN entities.trust_level IS 'Free-text trust/relationship descriptor for the entity (e.g. owner, admin, friend). Values are not enforced by a CHECK constraint; consumers should treat unexpected values as valid arbitrary strings.'; COMMENT ON COLUMN entities.introduction_context IS 'How/why we connected with this entity, relationship context'; diff --git a/memory/docs/semantic-recall.md b/memory/docs/semantic-recall.md index 8fffa77..aa9cf0c 100644 --- a/memory/docs/semantic-recall.md +++ b/memory/docs/semantic-recall.md @@ -101,8 +101,14 @@ Integrates with the entity-resolver library for **channel-aware** sender identif Uses `resolveEntityByIdentifiers()` with **conflict detection** โ€” if identifiers match different entities, entity injection is skipped to avoid incorrect context. +As of nova-mind#543, the injected block also renders pronouns, an optional trust suffix, and a +relationship-stats line when available โ€” see `memory/plugins/turn-context/README.md`'s +[Entity Context Formatting](../plugins/turn-context/README.md#entity-context-formatting) section +for the full rendering rules: + ``` -๐Ÿ‘ค **Talking with:** I)ruid +๐Ÿ‘ค **Talking with:** I)ruid (he/him) +๐Ÿ“Š Known contact: 214 facts ยท first seen 2025-02-08 ยท last message 2026-07-28 03:10 UTC (webchat:session-abc123) โ€ข **Timezone:** America/Chicago โ€ข **Communication Style:** Direct, technical ``` diff --git a/memory/plugins/turn-context/README.md b/memory/plugins/turn-context/README.md index 248ac52..f3c9795 100644 --- a/memory/plugins/turn-context/README.md +++ b/memory/plugins/turn-context/README.md @@ -24,7 +24,7 @@ before_prompt_build: |-----------|------|-------------| | **Classifier** | `classifier.ts` | Classifies messages into `info_request`, `action`, `conversation`, `continuation`, `command`. Rule-based first pass (~60-70%), Ollama LLM fallback for ambiguous cases. | | **Domain Identifier** | `domain-identifier.ts` | Matches messages to subject-matter domains via keyword matching + embedding similarity against `agent_domains` table. Returns top 1-3 domains with assigned agents (via JOIN, not hardcoded). Tolerates a missing `agent_domains.keywords` column on drifted schemas โ€” see [Schema-Drift Tolerance](#schema-drift-tolerance-domain-identifier) below. | -| **Entity Resolver** | `entity-resolver.ts` | Resolves sender identity via the entity-resolver library. Cache keyed by `sessionKey:senderId` (not just sessionKey) to support group channels. Returns both formatted text and numeric `entityId`. Also exports `resolveEntityForGuard()`, a lightweight resolution (id + display name only, no facts lookup) used by the honorific guard when `entity_resolver` is gated off. | +| **Entity Resolver** | `entity-resolver.ts` | Resolves sender identity via the entity-resolver library. Cache keyed by `sessionKey:senderId` (not just sessionKey) to support group channels. Returns both formatted text and numeric `entityId`. Formatted text (`formatEntityContext()`) includes pronouns and an optional trust suffix in the `๐Ÿ‘ค **Talking with:**` header, plus an optional `๐Ÿ“Š Known contact:` relationship-stats line โ€” see [Entity Context Formatting](#entity-context-formatting) below (#543). Also exports `resolveEntityForGuard()`, a lightweight resolution (id + display name only, no facts lookup) used by the honorific guard when `entity_resolver` is gated off. | | **Semantic Recall** | `semantic-recall.ts` | Spawns `proactive-recall.py` for memory retrieval. Supports tiered recall (domain-scoped first, full fallback) and visibility filtering (group channels โ†’ public facts only). | | **Turn Reminders** | `turn-reminders.ts` | Queries `agent_turn_context` table for per-turn reminder text. **Always fires regardless of message type** โ€” not gated by prompt_helper_config. | | **Honorific Guard** | `honorific-guard.ts` | Deterministic (non-LLM) instruction appended after the base system prompt, enforcing the "Sir" honorific policy based on the resolved sender entity and the responding agent. **Always fires regardless of message type or `entity_resolver` gating** โ€” see [Honorific Guard](#honorific-guard) below. | @@ -45,6 +45,48 @@ Per-agent overrides: insert rows with `agent_name` set to override defaults for On classifier failure, defaults to `info_request` (full pipeline) โ€” safe fallback. +## Entity Context Formatting + +`formatEntityContext()` (in `entity-resolver.ts`) is the pure function that renders the resolved +entity and its `getEntityProfile()` result into the injected `๐Ÿ‘ค **Talking with:**` block. As of +nova-mind#543 it renders up to three lines: + +1. **Header (always present):** `๐Ÿ‘ค **Talking with:** {displayName}`, where `displayName` is + `entity.fullName || entity.name`. + - If `entity.pronouns` is set, it's appended in parentheses: `(she/her)`. + - If `entity.trustLevel` is set **and is not `'unknown'`**, a trust suffix is appended: + ` โ€” trust: {trustLevel}`. `'unknown'` is the `entities.trust_level` column default and is + suppressed as noise โ€” every never-assessed entity would otherwise show `trust: unknown`. + `trustLevel` is free-text (see `database/schema.sql`'s column comment) โ€” any non-empty, + non-`'unknown'` string renders as-is; there is no enforced value set. +2. **Relationship stats line (optional):** `๐Ÿ“Š Known contact: {factCount} fact(s) ยท first seen + {createdAt} ยท last message {timestamp} ({ref})`. Rendered only when there is something + meaningful to show โ€” i.e. `factCount > 0`, or `entity.createdAt`, or a resolved last message, + or `entity.lastSeen` is present. A lone `factCount: 0` with no other metadata (a genuinely new, + never-before-seen contact) suppresses the whole line rather than showing `0 facts` on its own. + - The `last message` segment comes from `profile.stats.lastMessage` (most recent + `channel_transcripts` row, via `getEntityProfile()`). If absent, falls back to + `entity.lastSeen` labeled `last seen {lastSeen}` instead โ€” never both. + - Segments are joined with ` ยท ` and any missing segment (no last message/last seen, no + createdAt) is cleanly omitted rather than leaving a stray separator. +3. **Fact bullet list (unchanged from pre-#543 behavior):** one `โ€ข **Label:** value` line per + entry in `profile.facts` (the existing 7-key allowlist), rendered below the stats line. + +### Data source and timeout interaction (#543) + +`pronouns`, `trustLevel`, `lastSeen`, and `createdAt` are fetched as part of the **same, +already-cheap identifier-resolution query** used to find the entity in the first place +(`resolveEntityByIdentifiers()` in the entity-resolver library) โ€” not as part of the heavier +`getEntityProfile()` stats/facts query. This means they are already on the `Entity` object before +`resolveEntityContext()` even starts the `getEntityProfile()` 1s `Promise.race`, so **pronouns and +trust survive a stats-query timeout**: on timeout, `profile` degrades to +`{ facts: {}, stats: { factCount: 0, lastMessage: null } }`, but the header line above it still +renders normally with pronouns/trust/lastSeen intact. Only the `๐Ÿ“Š Known contact:` stats line and +the fact bullet list are affected by a stats-query timeout. + +The honorific guard (`resolveEntityForGuard()`) never triggers the `getEntityProfile()` stats +query at all โ€” it only needs entity id + display name. + ## Honorific Guard Deterministic, non-LLM guard that appends a short instruction to `appendSystemContext` (Step 2.6 diff --git a/memory/plugins/turn-context/src/entity-resolver.test.ts b/memory/plugins/turn-context/src/entity-resolver.test.ts new file mode 100644 index 0000000..b7b1220 --- /dev/null +++ b/memory/plugins/turn-context/src/entity-resolver.test.ts @@ -0,0 +1,353 @@ +import { describe, it, beforeEach, afterEach } from "node:test"; +import assert from "node:assert/strict"; +import * as fs from "fs"; +import * as os from "os"; +import { join } from "path"; +import { + formatEntityContext, + resolveEntityContext, + resolveEntityForGuard, + setEntityResolverPathForTest, + type SenderInfo, +} from "./entity-resolver.ts"; + +/** + * Unit tests for the entity-resolver subsystem in turn-context. + * + * Covers Gem's RS-001 through RS-062 design cases for nova-mind #543. + */ + +// โ”€โ”€ Fixtures โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + +const baseEntity = { + id: 1, + name: "Tabatha Janell Wilson", + fullName: "Tabatha Janell Wilson", + type: "person", +}; + +const richProfile = { + facts: { timezone: "America/Chicago" }, + stats: { + factCount: 48, + lastMessage: { + timestamp: "2026-07-27 02:11 UTC", + ref: "discord:1513392492651872306", + }, + }, +}; + +// โ”€โ”€ Formatting matrix (pure function, no DB) โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + +describe("formatEntityContext", () => { + it("RS-001: pronouns present โ†’ rendered in parenthetical after display name", () => { + const result = formatEntityContext( + { ...baseEntity, pronouns: "she/her" }, + { facts: {}, stats: { factCount: 0, lastMessage: null } } + ); + const lines = result.split("\n"); + assert.equal(lines[0], "๐Ÿ‘ค **Talking with:** Tabatha Janell Wilson (she/her)"); + }); + + it("RS-002: pronouns present + trust_level present โ†’ trailing trust suffix", () => { + const result = formatEntityContext( + { ...baseEntity, pronouns: "she/her", trustLevel: "friend" }, + { facts: {}, stats: { factCount: 0, lastMessage: null } } + ); + const lines = result.split("\n"); + assert.equal( + lines[0], + "๐Ÿ‘ค **Talking with:** Tabatha Janell Wilson (she/her) โ€” trust: friend" + ); + }); + + it("RS-003: full stats line renders all data points in documented order", () => { + const result = formatEntityContext( + { ...baseEntity, pronouns: "she/her", trustLevel: "friend", createdAt: "2026-01-30" }, + richProfile + ); + const lines = result.split("\n"); + assert.equal(lines[0], "๐Ÿ‘ค **Talking with:** Tabatha Janell Wilson (she/her) โ€” trust: friend"); + assert.equal( + lines[1], + "๐Ÿ“Š Known contact: 48 facts ยท first seen 2026-01-30 ยท last message 2026-07-27 02:11 UTC (discord:1513392492651872306)" + ); + }); + + it("RS-004: existing fact-key bullet list still renders below the stats line", () => { + const result = formatEntityContext( + { ...baseEntity, createdAt: "2026-01-30" }, + richProfile + ); + assert.ok(result.includes("๐Ÿ“Š Known contact:")); + assert.ok(result.includes("โ€ข **Timezone:** America/Chicago")); + const statsIdx = result.indexOf("๐Ÿ“Š Known contact:"); + const bulletIdx = result.indexOf("โ€ข **Timezone:**"); + assert.ok(statsIdx < bulletIdx, "stats line must appear before fact bullets"); + }); + + it("RS-010: NULL pronouns โ†’ no parenthetical, graceful degradation", () => { + const result = formatEntityContext( + baseEntity, + { facts: {}, stats: { factCount: 0, lastMessage: null } } + ); + assert.equal(result, "๐Ÿ‘ค **Talking with:** Tabatha Janell Wilson"); + assert.ok(!result.includes("null")); + assert.ok(!result.includes("undefined")); + assert.ok(!result.includes("()")); + }); + + it("RS-011: NULL or 'unknown' trust_level โ†’ no trust suffix", () => { + for (const trustLevel of [null, undefined, "unknown"]) { + const result = formatEntityContext( + { ...baseEntity, trustLevel }, + { facts: {}, stats: { factCount: 0, lastMessage: null } } + ); + assert.ok(!result.includes(" โ€” trust:"), `failed for trustLevel=${String(trustLevel)}`); + assert.ok(!result.includes("trust: unknown"), `failed for trustLevel=${String(trustLevel)}`); + } + }); + + it("RS-012: zero entity_facts still renders stats line", () => { + const result = formatEntityContext( + { ...baseEntity, createdAt: "2026-01-30" }, + { facts: {}, stats: { factCount: 0, lastMessage: null } } + ); + assert.ok(result.includes("๐Ÿ“Š Known contact:")); + assert.ok(result.includes("0 facts")); + }); + + it("RS-013: no channel_transcripts rows โ†’ last message segment omitted cleanly", () => { + const result = formatEntityContext( + { ...baseEntity, createdAt: "2026-01-30" }, + { facts: {}, stats: { factCount: 48, lastMessage: null } } + ); + assert.ok(result.includes("48 facts")); + assert.ok(result.includes("first seen 2026-01-30")); + assert.ok(!result.includes("last message")); + assert.ok(!result.includes("null")); + assert.ok(!result.includes("undefined")); + // No dangling separator after first-seen + assert.ok(!result.includes("first seen 2026-01-30 ยท")); + }); + + it("RS-014: brand-new entity renders sparse/degenerate form without leaked nulls", () => { + const result = formatEntityContext( + { id: 2, name: "New User", type: "person", createdAt: "2026-07-28" }, + { facts: {}, stats: { factCount: 0, lastMessage: null } } + ); + assert.equal(result, "๐Ÿ‘ค **Talking with:** New User\n๐Ÿ“Š Known contact: 0 facts ยท first seen 2026-07-28"); + assert.ok(!result.includes("null")); + assert.ok(!result.includes("undefined")); + }); + + it("RS-015: last_seen fallback renders only when no transcript row exists", () => { + const withLastMessage = formatEntityContext( + { ...baseEntity, lastSeen: "2026-07-26 08:00 UTC", createdAt: "2026-01-30" }, + { facts: {}, stats: { factCount: 48, lastMessage: richProfile.stats.lastMessage } } + ); + assert.ok(withLastMessage.includes("last message")); + assert.ok(!withLastMessage.includes("last seen")); + + const withoutLastMessage = formatEntityContext( + { ...baseEntity, lastSeen: "2026-07-26 08:00 UTC", createdAt: "2026-01-30" }, + { facts: {}, stats: { factCount: 48, lastMessage: null } } + ); + assert.ok(!withoutLastMessage.includes("last message")); + assert.ok(withoutLastMessage.includes("last seen 2026-07-26 08:00 UTC")); + }); + + it("RS-060: exactly 1 fact uses correct singular form", () => { + const result = formatEntityContext( + { ...baseEntity, createdAt: "2026-01-30" }, + { facts: {}, stats: { factCount: 1, lastMessage: null } } + ); + assert.ok(result.includes("1 fact")); + assert.ok(!result.includes("1 facts")); + }); + + it("RS-061: trust_level renders arbitrary non-empty string; 'unknown' suppressed", () => { + const values: Array<[string, boolean]> = [ + ["owner", true], + ["admin", true], + ["user", true], + ["unknown", false], + ["untrusted", true], + ["friend", true], + ]; + for (const [trustLevel, shouldRender] of values) { + const result = formatEntityContext( + { ...baseEntity, trustLevel }, + { facts: {}, stats: { factCount: 0, lastMessage: null } } + ); + const rendered = result.includes(` โ€” trust: ${trustLevel}`); + assert.equal(rendered, shouldRender, `trustLevel=${trustLevel}`); + } + }); + + it("RS-062: timestamp formatting is stable and explicit (string inputs)", () => { + const result = formatEntityContext( + { ...baseEntity, createdAt: "2026-01-30", lastSeen: "2026-07-27 02:11 UTC" }, + { + facts: {}, + stats: { + factCount: 1, + lastMessage: { timestamp: "2026-07-27 02:11 UTC", ref: "discord:1" }, + }, + } + ); + assert.ok(result.includes("first seen 2026-01-30")); + assert.ok(result.includes("last message 2026-07-27 02:11 UTC")); + }); +}); + +// โ”€โ”€ Orchestration tests with mocked entity-resolver module โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ + +function writeFixture(source: string): { path: string; restore: () => void } { + const tmpDir = fs.mkdtempSync(join(os.tmpdir(), "turn-context-ertest-")); + const fixturePath = join(tmpDir, "mock-entity-resolver.ts"); + fs.writeFileSync(fixturePath, source); + const restorePath = setEntityResolverPathForTest(fixturePath); + return { + path: fixturePath, + restore: () => { + restorePath(); + try { + fs.unlinkSync(fixturePath); + fs.rmdirSync(tmpDir); + } catch {} + }, + }; +} + +const resolvedTabathaFixture = ` +export const resolveEntityByIdentifiers = async () => ({ + ok: true, + entity: { + id: 42, + name: "Tabatha Janell Wilson", + fullName: "Tabatha Janell Wilson", + type: "person", + pronouns: "she/her", + trustLevel: "friend", + createdAt: "2026-01-30", + }, + facts: [], +}); +export const getCachedEntity = () => null; +export const setCachedEntity = () => {}; +`; + +const resolvedNewUserFixture = ` +export const resolveEntityByIdentifiers = async () => ({ + ok: true, + entity: { + id: 99, + name: "New User", + type: "person", + createdAt: "2026-07-28", + }, + facts: [], +}); +export const getCachedEntity = () => null; +export const setCachedEntity = () => {}; +`; + +describe("resolveEntityContext orchestration", () => { + it("RS-030: stats query timeout โ†’ graceful degradation, pronouns survive", async () => { + const { restore } = writeFixture( + resolvedTabathaFixture + + `export const getEntityProfile = async () => new Promise(() => {});` + ); + + try { + const result = await resolveEntityContext("session-1", { + senderId: "123", + provider: "discord", + }); + assert.ok(result.text); + assert.ok(result.text.includes("Tabatha Janell Wilson (she/her)")); + // Stats degraded to entity-derived first-seen only; no last-message clause + assert.ok(result.text.includes("๐Ÿ“Š Known contact:")); + assert.ok(result.text.includes("first seen 2026-01-30")); + assert.ok(!result.text.includes("last message")); + assert.equal(result.entityId, 42); + } finally { + restore(); + } + }); + + it("RS-031: stats query connection error โ†’ caught, base info returned", async () => { + const errors: string[] = []; + const originalError = console.error; + console.error = (...args: unknown[]) => { + errors.push(args.map(String).join(" ")); + }; + + const { restore } = writeFixture( + resolvedTabathaFixture + + `export const getEntityProfile = async () => { throw new Error("ECONNREFUSED"); };` + ); + + try { + const result = await resolveEntityContext("session-2", { + senderId: "123", + provider: "discord", + }); + assert.ok(result.text); + assert.ok(result.text.includes("Tabatha Janell Wilson")); + assert.ok(errors.some((e) => e.includes("ECONNREFUSED"))); + } finally { + restore(); + console.error = originalError; + } + }); + + it("RS-032: stats query SQL error โ†’ logged distinctly, graceful degradation", async () => { + const errors: string[] = []; + const originalError = console.error; + console.error = (...args: unknown[]) => { + errors.push(args.map(String).join(" ")); + }; + + const { restore } = writeFixture( + resolvedTabathaFixture + + `export const getEntityProfile = async () => { const e = new Error("column lm.timestamp does not exist"); e.code = "42703"; throw e; };` + ); + + try { + const result = await resolveEntityContext("session-3", { + senderId: "123", + provider: "discord", + }); + assert.ok(result.text); + assert.ok(result.text.includes("Tabatha Janell Wilson")); + assert.ok(errors.some((e) => e.includes("42703") || e.includes("column lm.timestamp"))); + } finally { + restore(); + console.error = originalError; + } + }); +}); + +describe("resolveEntityForGuard non-regression", () => { + it("RS-042: guard path does NOT trigger stats query", async () => { + let profileCalls = 0; + const { restore } = writeFixture( + resolvedNewUserFixture + + `export const getEntityProfile = async () => { globalThis.__profileCalls = (globalThis.__profileCalls || 0) + 1; return { facts: {}, stats: { factCount: 0, lastMessage: null } }; };` + ); + + try { + const result = await resolveEntityForGuard("guard-session", { + senderId: "456", + provider: "discord", + }); + assert.equal(result.entityId, 99); + assert.equal(result.displayName, "New User"); + assert.equal(profileCalls, 0, "getEntityProfile must not be called for guard path"); + } finally { + restore(); + } + }); +}); diff --git a/memory/plugins/turn-context/src/entity-resolver.ts b/memory/plugins/turn-context/src/entity-resolver.ts index 2505b1c..a549e59 100644 --- a/memory/plugins/turn-context/src/entity-resolver.ts +++ b/memory/plugins/turn-context/src/entity-resolver.ts @@ -16,19 +16,46 @@ import { join } from "path"; // Entity type from dynamic import โ€” use any with runtime property checks type Entity = any; -type EntityFacts = Record; +type EntityProfile = { facts: Record; stats: EntityRelationshipStats }; +type EntityRelationshipStats = { factCount: number; lastMessage: { timestamp: string; ref: string } | null }; let resolveEntityByIdentifiers: any; let getEntityProfile: any; let getCachedEntity: any; let setCachedEntity: any; let entityResolverLoaded = false; +let entityResolverPath = join(os.homedir(), ".openclaw", "lib", "entity-resolver", "index.ts"); + +/** + * Test seam: override the path used to dynamically import the entity-resolver + * library. Returns a restore function. + * + * This is the ONLY supported way for unit tests to inject a mock resolver; it + * resets the internal loaded state so the next call re-imports from the new + * path. See entity-resolver.test.ts. + */ +export function setEntityResolverPathForTest(path: string): () => void { + const previousPath = entityResolverPath; + entityResolverPath = path; + entityResolverLoaded = false; + resolveEntityByIdentifiers = undefined; + getEntityProfile = undefined; + getCachedEntity = undefined; + setCachedEntity = undefined; + return () => { + entityResolverPath = previousPath; + entityResolverLoaded = false; + resolveEntityByIdentifiers = undefined; + getEntityProfile = undefined; + getCachedEntity = undefined; + setCachedEntity = undefined; + }; +} async function ensureEntityResolver(): Promise { if (entityResolverLoaded) return !!resolveEntityByIdentifiers; entityResolverLoaded = true; try { - const entityResolverPath = join(os.homedir(), ".openclaw", "lib", "entity-resolver", "index.ts"); const mod = await import(entityResolverPath); resolveEntityByIdentifiers = mod.resolveEntityByIdentifiers; getEntityProfile = mod.getEntityProfile; @@ -222,11 +249,53 @@ export function extractIdentifiers( // โ”€โ”€ Format helpers โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€ -function formatEntityContext(entity: Entity, facts: EntityFacts): string { +/** + * Format an entity and its profile into the turn-context injection block. + * + * Pure function โ€” safe to unit test with hand-built inputs. + */ +export function formatEntityContext(entity: Entity, profile: EntityProfile): string { const displayName = entity.fullName || entity.name; let context = `๐Ÿ‘ค **Talking with:** ${displayName}`; - const factEntries = Object.entries(facts); + if (entity.pronouns) { + context += ` (${entity.pronouns})`; + } + + // Only render trust when it carries real information; 'unknown' is the + // column default and is noise for display. See nova-mind#543. + if (entity.trustLevel && entity.trustLevel !== "unknown") { + context += ` โ€” trust: ${entity.trustLevel}`; + } + + // Relationship/stats summary line (nova-mind#543). + // Render only when there is something meaningful to show beyond a lone + // "0 facts" count (which is expected for brand-new contacts that have no + // other metadata yet). + const factCount = typeof profile?.stats?.factCount === "number" ? profile.stats.factCount : 0; + const lastMessage = profile?.stats?.lastMessage; + const hasStatsMetadata = + factCount > 0 || entity.createdAt || lastMessage || entity.lastSeen; + + if (hasStatsMetadata) { + const statsParts: string[] = []; + statsParts.push(`${factCount} fact${factCount === 1 ? "" : "s"}`); + + if (entity.createdAt) { + statsParts.push(`first seen ${entity.createdAt}`); + } + + if (lastMessage) { + statsParts.push(`last message ${lastMessage.timestamp} (${lastMessage.ref})`); + } else if (entity.lastSeen) { + // Fallback only when no transcript-derived last message is available + statsParts.push(`last seen ${entity.lastSeen}`); + } + + context += `\n๐Ÿ“Š Known contact: ${statsParts.join(" ยท ")}`; + } + + const factEntries = Object.entries(profile?.facts || {}); if (factEntries.length > 0) { context += "\n"; for (const [key, value] of factEntries) { @@ -272,13 +341,17 @@ export async function resolveEntityContext( return { text: null, entityId: null, displayName: null }; } - // Load entity facts with a 1s timeout - let facts: EntityFacts = {}; + // Load entity profile (allowlist facts + relationship stats) with a 1s timeout. + // Pronouns/trust/last_seen/created_at are already on the entity from the + // identifier-resolution query, so they survive a stats-query timeout. + let profile: EntityProfile = { facts: {}, stats: { factCount: 0, lastMessage: null } }; if (getEntityProfile) { try { - facts = await Promise.race([ - getEntityProfile(entity.id), - new Promise((resolve) => setTimeout(() => resolve({}), 1000)), + profile = await Promise.race([ + getEntityProfile(entity.id) as Promise, + new Promise((resolve) => + setTimeout(() => resolve({ facts: {}, stats: { factCount: 0, lastMessage: null } }), 1000) + ), ]); } catch (err) { console.error( @@ -289,7 +362,7 @@ export async function resolveEntityContext( } const displayName = entity.fullName || entity.name; - const result = formatEntityContext(entity, facts); + const result = formatEntityContext(entity, profile); console.log( `[turn-context] Loaded entity context for: ${entity.name} (entityId=${entity.id}) (${info.senderId})` ); diff --git a/psyche/ARCHITECTURE-entities-users.md b/psyche/ARCHITECTURE-entities-users.md index 0729fe2..51ad6a7 100644 --- a/psyche/ARCHITECTURE-entities-users.md +++ b/psyche/ARCHITECTURE-entities-users.md @@ -40,7 +40,7 @@ The core table for all tracked entities. | `photo` | bytea | Avatar/photo (binary) | | `user_id` | varchar(255) | External user identifier | | `auth_token` | varchar(255) | Authentication token | -| `trust_level` | varchar(20) | Trust level string, default `'unknown'` (values: `owner`, `admin`, `user`, `unknown`, `untrusted`) | +| `trust_level` | varchar(20) | Free-text trust/relationship descriptor, default `'unknown'`. Not enforced by a CHECK constraint โ€” `owner`/`admin`/`user`/`unknown`/`untrusted` are the values `confidence_helper.py`'s `get_initial_confidence()` recognizes for confidence scoring (others fall back to its default weight), but arbitrary values (e.g. `friend`) are valid and rendered as-is by the `turn-context` plugin's entity-injection trust suffix (nova-mind#543). See `database/schema.sql`'s column comment. | | `collaborate` | boolean | Can this entity collaborate? | | `collaboration_scope` | text | 'full', 'domain-specific', or 'supervised' | | `introduction_context` | text | How was this entity introduced? | diff --git a/relationships/ARCHITECTURE-entity-resolver.md b/relationships/ARCHITECTURE-entity-resolver.md index ff288fe..4c0b7ed 100644 --- a/relationships/ARCHITECTURE-entity-resolver.md +++ b/relationships/ARCHITECTURE-entity-resolver.md @@ -52,9 +52,22 @@ interface Entity { name: string; // Display name fullName?: string; // Full legal name (optional) type: string; // Entity type (person, organization, etc.) + pronouns?: string; // entities.pronouns, omitted when NULL (#543) + trustLevel?: string; // entities.trust_level, free-text, omitted when NULL (#543) + lastSeen?: string; // pre-rendered 'YYYY-MM-DD HH24:MI UTC', omitted when NULL (#543) + createdAt?: string; // pre-rendered 'YYYY-MM-DD', omitted when NULL (#543) } ``` +**Note (#543):** `pronouns`/`trustLevel`/`lastSeen`/`createdAt` are populated by the shared +`mapDbEntity()` helper in `resolver.ts` from columns fetched in the SAME identifier-resolution +query as `id`/`name`/`full_name`/`type` โ€” there is no separate round trip for them, and they are +omitted from the object entirely (not set to `undefined` explicitly, just absent) when the +underlying column is NULL. Because they ride on the cheap identifier query rather than the +heavier stats query in `getEntityProfile()`, they survive a `getEntityProfile()` timeout in the +`turn-context` consumer (see `resolveEntityContext()` in +`memory/plugins/turn-context/src/entity-resolver.ts`). + **Example:** ```typescript // Resolve by phone number @@ -78,15 +91,19 @@ if (entity) { } ``` -#### `getEntityProfile(entityId: number, factKeys?: string[]): Promise` +#### `getEntityProfile(entityId: number, factKeys?: string[]): Promise` -Loads entity profile facts for personalization. +Loads entity profile facts for personalization, **plus cheap relationship stats** (nova-mind#543). +As of #543 the return type changed from a bare `EntityFacts` map to `{ facts, stats }` โ€” this is a +breaking signature change for any direct consumer that previously read fields off the returned +object as if it were the facts map itself (e.g. old code reading `profile.timezone` must be +updated to `profile.facts.timezone`). **Parameters:** - `entityId`: Database entity ID from resolveEntity() -- `factKeys`: Optional array of specific fact keys to load +- `factKeys`: Optional array of specific fact keys to load (affects `facts` only, not `stats`) -**Default fact keys loaded:** +**Default fact keys loaded (unchanged, still capped at 20 rows):** - `timezone`, `current_timezone` - User's timezone information - `communication_style` - Preferred communication approach - `expertise` - Areas of knowledge/skill @@ -96,22 +113,43 @@ Loads entity profile facts for personalization. **Returns:** ```typescript -interface EntityFacts { - [key: string]: string; // Fact key-value pairs +interface EntityRelationshipStats { + factCount: number; // UNFILTERED entity_facts row count for this entity (not just the allowlist) + lastMessage: { + timestamp: string; // 'YYYY-MM-DD HH24:MI UTC', rendered server-side + ref: string; // `${provider}:${external_chat_id}`, e.g. 'discord:1513392492651872306' + } | null; // null when the entity has no channel_transcripts rows +} + +interface EntityProfile { + facts: EntityFacts; // the existing allowlist facts (unchanged shape) + stats: EntityRelationshipStats; // new in #543 } ``` **Example:** ```typescript -// Load default profile facts +// Load default profile facts + stats const profile = await getEntityProfile(entityId); -console.log(`Timezone: ${profile.timezone}`); -console.log(`Style: ${profile.communication_style}`); +console.log(`Timezone: ${profile.facts.timezone}`); +console.log(`Style: ${profile.facts.communication_style}`); +console.log(`Known for ${profile.stats.factCount} facts`); +if (profile.stats.lastMessage) { + console.log(`Last heard from them: ${profile.stats.lastMessage.timestamp} (${profile.stats.lastMessage.ref})`); +} -// Load specific facts only -const timezoneFacts = await getEntityProfile(entityId, ['timezone', 'current_timezone']); +// Load specific facts only (stats are always computed regardless of factKeys) +const { facts } = await getEntityProfile(entityId, ['timezone', 'current_timezone']); ``` +**Implementation (#543):** a single aggregate SQL query (`fc` subquery for the unfiltered count, +a `LEFT JOIN LATERAL` against `channel_transcripts`/`channel_sessions` ordered by +`timestamp DESC LIMIT 1` for the last message, and a `LEFT JOIN LATERAL` for the allowlist facts, +capped at 20 rows) โ€” designed as one round trip so the whole call stays inside the existing 1s +`Promise.race` timeout budget consumed by the `turn-context` plugin's `resolveEntityContext()`. +On any query error or timeout, resolves to `{ facts: {}, stats: { factCount: 0, lastMessage: null } }` +rather than throwing, matching the library's existing fail-closed error contract. + #### `resolveEntityByIdentifiers(identifiers: EntityIdentifiers): Promise` Resolves an entity by identifiers **with conflict detection**. Unlike `resolveEntity()`, this function fetches ALL matching entities and checks whether they agree. Returns `null` if no entity matched. @@ -263,7 +301,14 @@ nova-mind#522. **Query Strategy:** ```sql -- resolveEntity() query (simplified) โ€” returns first match -SELECT DISTINCT e.id, e.name, e.full_name, e.type +-- As of #543, also selects pronouns/trust_level/last_seen/created_at so the returned +-- Entity carries relationship metadata without a second round trip (see mapDbEntity()). +-- last_seen and created_at are rendered server-side with to_char() rather than left as +-- raw timestamp values โ€” see the timezone note below. +SELECT DISTINCT e.id, e.name, e.full_name, e.type, + e.pronouns, e.trust_level, + to_char(e.last_seen, 'YYYY-MM-DD HH24:MI "UTC"') AS last_seen, + to_char(e.created_at, 'YYYY-MM-DD') AS created_at FROM entities e JOIN entity_facts ef ON e.id = ef.entity_id WHERE (ef.key = 'phone' AND ef.value = ?) @@ -273,9 +318,19 @@ WHERE (ef.key = 'phone' AND ef.value = ?) LIMIT 1 -- resolveEntityByIdentifiers() query โ€” returns ALL matches for conflict detection --- Same WHERE clause but NO LIMIT, groups results by entity, and checks for conflicts +-- Same WHERE clause and same #543 pronouns/trust_level/last_seen/created_at columns, +-- but NO LIMIT, groups results by entity, and checks for conflicts ``` +**Timezone note on `last_seen` (#543, fixed post-merge):** `entities.last_seen` is a plain +`timestamp` column with no timezone attached. An earlier version of this query applied +`AT TIME ZONE 'UTC'` to it, which Postgres interprets as "this naive value is actually in the +*session's* timezone โ€” convert it to UTC," silently shifting the displayed time under any +non-UTC session (e.g. `America/Chicago`). The fix renders the naive value directly via +`to_char(e.last_seen, 'YYYY-MM-DD HH24:MI "UTC"')` with no `AT TIME ZONE` conversion, matching +how `created_at` (also a naive `timestamp`) is already rendered. See the RS-062 regression test +in `test.ts` (run under `SET timezone='America/Chicago'`). + ### Caching Layer **In-Memory Cache:** @@ -349,12 +404,12 @@ export async function handleSignalMessage(message: SignalMessage, context: HookC const profile = await getEntityProfile(entity.id); // Use timezone for time-aware responses - if (profile.timezone) { - context.userTimezone = profile.timezone; + if (profile.facts.timezone) { + context.userTimezone = profile.facts.timezone; } // Adapt communication style - if (profile.communication_style === 'direct') { + if (profile.facts.communication_style === 'direct') { context.responseStyle = 'concise'; } } @@ -414,7 +469,7 @@ export class EmailProcessor { const profile = await getEntityProfile(sender.id); // Personalize response based on communication style - const responseStyle = profile.communication_style || 'balanced'; + const responseStyle = profile.facts.communication_style || 'balanced'; return { sender, @@ -600,8 +655,8 @@ export class SignalBot { entity, profile, sessionId, - timezone: profile.timezone || 'UTC', - style: profile.communication_style || 'balanced' + timezone: profile.facts.timezone || 'UTC', + style: profile.facts.communication_style || 'balanced' }; // Process message with context @@ -661,7 +716,7 @@ app.get('/api/profile', (req, res) => { res.json({ entity: req.entity, profile: req.entityProfile, - timezone: req.entityProfile.timezone || 'UTC' + timezone: req.entityProfile.facts.timezone || 'UTC' }); }); ``` @@ -693,9 +748,9 @@ export class EmailHandler { // Adapt response based on communication style const responseConfig = { - formal: profile.communication_style === 'formal', - timezone: profile.timezone || 'UTC', - expertise: profile.expertise?.split(',') || [] + formal: profile.facts.communication_style === 'formal', + timezone: profile.facts.timezone || 'UTC', + expertise: profile.facts.expertise?.split(',') || [] }; // Generate personalized response @@ -783,8 +838,8 @@ describe('Entity Resolution', () => { const entity = await resolveEntity({ email: 'john@example.com' }); const profile = await getEntityProfile(entity!.id); - expect(profile.timezone).toBeDefined(); - expect(profile.communication_style).toBeDefined(); + expect(profile.facts.timezone).toBeDefined(); + expect(profile.facts.communication_style).toBeDefined(); }); }); ``` diff --git a/relationships/CHANGELOG.md b/relationships/CHANGELOG.md index 3b3764a..d4c6bdf 100644 --- a/relationships/CHANGELOG.md +++ b/relationships/CHANGELOG.md @@ -2,6 +2,19 @@ ## Unreleased +### Added +- **Pronouns + relationship stats** (nova-mind#543) โ€” `resolveEntity()`/`resolveEntityByIdentifiers()` now also select `entities.pronouns`, `entities.trust_level`, `entities.last_seen`, and `entities.created_at` via a new shared `mapDbEntity()` helper, so the returned `Entity` optionally carries `pronouns`, `trustLevel`, `lastSeen` (pre-rendered `YYYY-MM-DD HH24:MI UTC`), and `createdAt` (pre-rendered `YYYY-MM-DD`) โ€” omitted when the underlying column is NULL. `getEntityProfile()`'s return type changed from a bare `EntityFacts` map to `EntityProfile` (`{ facts, stats }`, new `EntityRelationshipStats` type in `types.ts`): `stats.factCount` is an unfiltered `entity_facts` row count, and `stats.lastMessage` is the most recent `channel_transcripts` row (timestamp + `provider:external_chat_id` ref), both from a single aggregate query with two `LEFT JOIN LATERAL`s designed to stay inside the existing 1s timeout budget the `turn-context` plugin races it against. Consumed by `formatEntityContext()`/`resolveEntityContext()` in `memory/plugins/turn-context/src/entity-resolver.ts` โ€” see root `CHANGELOG.md` (batch `entity-resolver-relationship-stats-543`) for the full consumer-side behavior change and `ARCHITECTURE-entity-resolver.md` for the updated API reference. **Breaking for direct consumers of `getEntityProfile()`:** code reading `profile.timezone` etc. directly must be updated to `profile.facts.timezone`. + +### Fixed +- **`entities.last_seen` timezone-shift bug** (nova-mind#543) โ€” rendering `last_seen` with `AT TIME ZONE 'UTC'` silently shifted the value under non-UTC sessions; fixed to use `to_char()` directly with no conversion, matching `created_at`. See RS-062 in `test.ts`. + +### Changed +- **`entities.trust_level` column comment** (nova-mind#543, `database/schema.sql`) โ€” updated from an enum-style description to reflect that the column is free-text and not enforced by a CHECK constraint. + +### Tests +- `relationships/lib/entity-resolver/test.ts` (nova-mind#543) โ€” integration scaffold plus RS-062 timezone regression test. +- `memory/plugins/turn-context/src/entity-resolver.test.ts` (nova-mind#543) โ€” RS-001โ€“RS-062 formatting matrix for `formatEntityContext()` (pure function). + ### Added - **IRC identifier support: `ircUsername` โ†’ `irc_username`** (nova-mind#522) โ€” `ircUsername` added to `EntityIdentifiers` (`types.ts`) and `IDENTIFIER_TO_DB_KEY` (`resolver.ts`), mapping to the `irc_username` entity_facts key. This library only stores/matches the already-composed `/` value โ€” host/nick parsing and lowercasing happen upstream in the `turn-context` plugin (see `memory/CHANGELOG.md`). Companion pre-migration `database/pre-migrations/006-lowercase-irc-username-values.sql` normalizes existing mixed-case values. See root `CHANGELOG.md` (batch `irc-entity-resolver-522`) and `relationships/ARCHITECTURE-entity-resolver.md` for the full identifier-mapping table. diff --git a/relationships/README.md b/relationships/README.md index 4c3c7cd..0498a2f 100644 --- a/relationships/README.md +++ b/relationships/README.md @@ -27,6 +27,7 @@ The NOVA Relationships System is a unified platform that merged the original Ent 1. **Entity Resolver Library** (`lib/entity-resolver/`) - **Identity Resolution**: `resolveEntity()` looks up an entity across multiple identifiers (phone, email, UUID, certificates, Discord ID, Telegram ID, Slack member ID, Signal UUID/username, IRC username, `deviceId`) via the `IDENTIFIER_TO_DB_KEY` mapping (`ircUsername` โ†’ `irc_username`, added for #522). - Conflict detection via `resolveEntityByIdentifiers()` โ€” flags when identifiers match different entities + - **Relationship stats & pronouns** (#543): the resolved `Entity` optionally carries `pronouns`, `trustLevel` (free-text `entities.trust_level`), `lastSeen`, and `createdAt`; `getEntityProfile()` returns `{ facts, stats }`, where `stats` is an unfiltered fact count plus the most recent channel-transcript timestamp/ref โ€” all inside the existing 1s timeout budget consumed by `turn-context`. See `ARCHITECTURE-entity-resolver.md` for the full API. - (Note: `find_entity_id()`/`is_plausible_entity()` โ€” alternate-spelling matching and ghost-entity heuristics โ€” are implemented in the memory domain's `extract_memories.py`, not this library. The IRC `/` composite value itself is derived in the `turn-context` plugin's `entity-resolver.ts`, not in this library โ€” see `memory/plugins/turn-context/src/entity-resolver.ts`.) - Session-aware caching for performance - Profile management and fact storage @@ -313,8 +314,8 @@ if (entity) { // Load profile for personalization const profile = await getEntityProfile(entity.id); console.log(`Found: ${entity.name}`); - console.log(`Style: ${profile.communication_style}`); - console.log(`Timezone: ${profile.timezone}`); + console.log(`Style: ${profile.facts.communication_style}`); + console.log(`Timezone: ${profile.facts.timezone}`); } ``` diff --git a/relationships/docs/integration-guide.md b/relationships/docs/integration-guide.md index 063a611..e13d4f2 100644 --- a/relationships/docs/integration-guide.md +++ b/relationships/docs/integration-guide.md @@ -18,7 +18,7 @@ async function handleUserRequest(userIdentifiers: any) { if (entity) { const profile = await getEntityProfile(entity.id); console.log(`Welcome back, ${entity.name}!`); - console.log(`Your timezone: ${profile.timezone || 'UTC'}`); + console.log(`Your timezone: ${profile.facts.timezone || 'UTC'}`); return { entity, profile }; } else { console.log('New user - consider onboarding'); @@ -72,8 +72,8 @@ class NOVASignalBot { } } - // Load user profile for personalization - const profile = entity ? await getEntityProfile(entity.id) : {}; + // Load user profile for personalization (EntityProfile shape as of #543: { facts, stats }) + const profile = entity ? await getEntityProfile(entity.id) : { facts: {}, stats: { factCount: 0, lastMessage: null } }; // Create response context const context = this.createResponseContext(entity, profile, message); @@ -87,9 +87,9 @@ class NOVASignalBot { return { entity, profile, - timezone: profile.timezone || 'UTC', - communicationStyle: profile.communication_style || 'balanced', - expertise: profile.expertise?.split(',') || [], + timezone: profile.facts.timezone || 'UTC', + communicationStyle: profile.facts.communication_style || 'balanced', + expertise: profile.facts.expertise?.split(',') || [], isKnownUser: !!entity, platform: 'signal', isGroupChat: !!message.groupId @@ -190,8 +190,8 @@ export async function entityResolverMiddleware( req.entityProfile = await getEntityProfile(entity.id); // Add timezone to request for time-aware responses - if (req.entityProfile.timezone) { - req.timezone = req.entityProfile.timezone; + if (req.entityProfile.facts.timezone) { + req.timezone = req.entityProfile.facts.timezone; } } } catch (error) { @@ -225,14 +225,14 @@ app.get('/api/profile', (req, res) => { }, profile: req.entityProfile, personalization: { - timezone: req.entityProfile?.timezone || 'UTC', - communicationStyle: req.entityProfile?.communication_style || 'balanced' + timezone: req.entityProfile?.facts?.timezone || 'UTC', + communicationStyle: req.entityProfile?.facts?.communication_style || 'balanced' } }); }); app.get('/api/dashboard', (req, res) => { - const style = req.entityProfile?.communication_style || 'balanced'; + const style = req.entityProfile?.facts?.communication_style || 'balanced'; if (style === 'direct') { res.json({ @@ -308,9 +308,9 @@ class NOVAEmailProcessor { entity: sender, profile, facts: allFacts, - communicationStyle: profile.communication_style || 'formal', - timezone: profile.timezone || 'UTC', - expertise: profile.expertise?.split(',') || [], + communicationStyle: profile.facts.communication_style || 'formal', + timezone: profile.facts.timezone || 'UTC', + expertise: profile.facts.expertise?.split(',') || [], lastInteraction: this.getLastEmailInteraction(sender.id) }; @@ -603,7 +603,7 @@ class SessionEntityManager { } } - return profile || {}; + return profile || { facts: {}, stats: { factCount: 0, lastMessage: null } }; } } ``` diff --git a/relationships/lib/entity-resolver/COMPLETED.md b/relationships/lib/entity-resolver/COMPLETED.md index 141eabd..e436b08 100644 --- a/relationships/lib/entity-resolver/COMPLETED.md +++ b/relationships/lib/entity-resolver/COMPLETED.md @@ -151,7 +151,7 @@ async function handleMessage(sessionId: string, senderId: string) { // Load profile if needed if (entity) { const profile = await getEntityProfile(entity.id); - console.log(`User: ${entity.name}, Timezone: ${profile.timezone}`); + console.log(`User: ${entity.name}, Timezone: ${profile.facts.timezone}`); } return entity; diff --git a/relationships/lib/entity-resolver/README.md b/relationships/lib/entity-resolver/README.md index bf3e8ea..32847ea 100644 --- a/relationships/lib/entity-resolver/README.md +++ b/relationships/lib/entity-resolver/README.md @@ -4,9 +4,10 @@ A reusable TypeScript library for resolving and caching entity information from ## Features -- **Multi-identifier resolution**: Resolve entities by phone, UUID, certificate CN, email, or platform-specific IDs (Discord, Telegram, Slack, Signal) +- **Multi-identifier resolution**: Resolve entities by phone, UUID, certificate CN, email, or platform-specific IDs (Discord, Telegram, Slack, Signal, IRC, device pairing) - **Conflict detection**: Detect when identifiers resolve to different entities (data integrity issues) - **Session-aware caching**: Cache entity lookups per session with configurable TTL +- **Relationship stats**: `getEntityProfile()` also returns an unfiltered fact count and the most recent channel transcript (timestamp + `provider:external_chat_id` ref), alongside pronouns/trust level carried on the `Entity` itself (nova-mind#543) - **Profile loading**: Load entity facts (timezone, preferences, etc.) - **Connection pooling**: Efficient database connection management - **Dynamic import support**: Installable to `~/.openclaw/lib/` for use by hooks at runtime @@ -66,6 +67,10 @@ if (entity) { - `slackMemberId` - Slack member ID - `signalUuid` - Signal UUID (dedicated field, maps to `signal_uuid` in DB) - `signalUsername` - Signal username (maps to `signal_username` in DB) +- `deviceId` - OpenClaw device pairing ID (maps to `nova_app_device_id` in DB) +- `ircUsername` - Composite `/`, e.g. `late.sh/druidian` (maps to `irc_username` in DB) + +**Returned `Entity` also carries relationship metadata when present on the row** (nova-mind#543): `pronouns`, `trustLevel` (free-text `entities.trust_level`), `lastSeen` (rendered `YYYY-MM-DD HH24:MI UTC`), and `createdAt` (rendered `YYYY-MM-DD`). These are populated by `mapDbEntity()` and are omitted from the object entirely when the underlying column is NULL โ€” consumers should treat them as optional. #### `resolveEntityByIdentifiers(identifiers: EntityIdentifiers): Promise` @@ -97,19 +102,29 @@ type ResolveResult = This function is preferred over `resolveEntity()` when conflict detection matters (e.g., the semantic-recall hook uses it to avoid injecting the wrong entity context). -#### `getEntityProfile(entityId: number, factKeys?: string[]): Promise` +#### `getEntityProfile(entityId: number, factKeys?: string[]): Promise` -Load entity profile facts. +Load entity profile facts **plus cheap relationship stats**, in a single aggregate query designed to stay inside the 1s timeout budget the `turn-context` plugin races it against (nova-mind#543). As of #543 this no longer returns a bare `EntityFacts` map โ€” it returns `{ facts, stats }`. ```typescript const profile = await getEntityProfile(entityId); -// Returns: { timezone: 'America/New_York', communication_style: 'direct', ... } - -// Or load specific facts: -const timezone = await getEntityProfile(entityId, ['timezone', 'current_timezone']); +// Returns: +// { +// facts: { timezone: 'America/New_York', communication_style: 'direct', ... }, +// stats: { +// factCount: 48, // unfiltered entity_facts row count +// lastMessage: { // most recent channel_transcripts row, or null +// timestamp: '2026-07-27 02:11 UTC', +// ref: 'discord:1513392492651872306', // `${provider}:${external_chat_id}` +// }, +// }, +// } + +// Or load specific facts (still affects only `facts`, not `stats`): +const { facts } = await getEntityProfile(entityId, ['timezone', 'current_timezone']); ``` -**Default fact keys:** +**Default fact keys** (unchanged โ€” still the 7-key allowlist, capped at 20 rows): - `timezone`, `current_timezone` - `communication_style` - `expertise` @@ -117,6 +132,8 @@ const timezone = await getEntityProfile(entityId, ['timezone', 'current_timezone - `location` - `occupation` +**`stats.factCount`** counts ALL `entity_facts` rows for the entity, regardless of `factKeys` โ€” it is not filtered by the allowlist. **`stats.lastMessage`** comes from a `LEFT JOIN LATERAL` against `channel_transcripts`/`channel_sessions` ordered by `timestamp DESC LIMIT 1`; it is `null` when the entity has no transcript rows. On any query error or timeout, the whole call resolves to `{ facts: {}, stats: { factCount: 0, lastMessage: null } }` rather than throwing โ€” matching the library's existing fail-closed error contract. + #### `getAllEntityFacts(entityId: number): Promise` Load all facts for an entity (including custom facts). @@ -179,12 +196,28 @@ interface Entity { name: string; fullName?: string; type: string; + pronouns?: string; // added #543 + trustLevel?: string; // added #543 โ€” free-text entities.trust_level + lastSeen?: string; // added #543 โ€” pre-rendered 'YYYY-MM-DD HH24:MI UTC' + createdAt?: string; // added #543 โ€” pre-rendered 'YYYY-MM-DD' } interface EntityFacts { [key: string]: string; } +// Relationship stats returned alongside facts by getEntityProfile() (#543) +interface EntityRelationshipStats { + factCount: number; + lastMessage: { timestamp: string; ref: string } | null; +} + +// getEntityProfile()'s actual return type as of #543 (was EntityFacts before) +interface EntityProfile { + facts: EntityFacts; + stats: EntityRelationshipStats; +} + interface EntityIdentifiers { phone?: string; uuid?: string; @@ -195,6 +228,8 @@ interface EntityIdentifiers { slackMemberId?: string; signalUuid?: string; signalUsername?: string; + deviceId?: string; // OpenClaw device pairing ID + ircUsername?: string; // composite /, e.g. late.sh/druidian } /** @@ -259,9 +294,10 @@ async function handleMessage(sessionId: string, senderId: string) { // Cache for future use setCachedEntity(sessionId, entity); - // Load profile + // Load profile (facts + relationship stats, see EntityProfile above) const profile = await getEntityProfile(entity.id); - console.log(`User ${entity.name} (${profile.timezone || 'unknown timezone'})`); + console.log(`User ${entity.name} (${profile.facts.timezone || 'unknown timezone'})`); + console.log(`Known for ${profile.stats.factCount} facts`); } } diff --git a/relationships/lib/entity-resolver/REFACTORING.md b/relationships/lib/entity-resolver/REFACTORING.md index b9f4d90..550c971 100644 --- a/relationships/lib/entity-resolver/REFACTORING.md +++ b/relationships/lib/entity-resolver/REFACTORING.md @@ -194,7 +194,7 @@ import { const entity = await resolveEntity({ phone: '+1234567890' }); if (entity) { const profile = await getEntityProfile(entity.id); - console.log(profile.timezone, profile.communication_style); + console.log(profile.facts.timezone, profile.facts.communication_style); } ``` diff --git a/relationships/lib/entity-resolver/index.ts b/relationships/lib/entity-resolver/index.ts index 87c111c..a03f0b9 100644 --- a/relationships/lib/entity-resolver/index.ts +++ b/relationships/lib/entity-resolver/index.ts @@ -49,6 +49,8 @@ export { export type { Entity, EntityFacts, + EntityProfile, + EntityRelationshipStats, EntityIdentifiers, ResolveResult, } from "./types.ts"; diff --git a/relationships/lib/entity-resolver/package.json b/relationships/lib/entity-resolver/package.json index 53f9919..5f191d7 100644 --- a/relationships/lib/entity-resolver/package.json +++ b/relationships/lib/entity-resolver/package.json @@ -7,6 +7,7 @@ "types": "index.ts", "scripts": { "typecheck": "tsc --noEmit", + "build": "tsc", "test": "tsx test.ts --irc-tests" }, "dependencies": { diff --git a/relationships/lib/entity-resolver/resolver.ts b/relationships/lib/entity-resolver/resolver.ts index c0b6c5f..b7940ff 100644 --- a/relationships/lib/entity-resolver/resolver.ts +++ b/relationships/lib/entity-resolver/resolver.ts @@ -4,7 +4,7 @@ import pg from "pg"; import * as os from "os"; -import type { Entity, EntityFacts, EntityIdentifiers, DbEntity, DbEntityFact, ResolveResult } from "./types.ts"; +import type { Entity, EntityFacts, EntityIdentifiers, DbEntity, DbEntityFact, ResolveResult, EntityProfile } from "./types.ts"; import { join } from "path"; const { Pool } = pg; @@ -74,6 +74,25 @@ const IDENTIFIER_TO_DB_KEY: Record = { ircUsername: 'irc_username', }; +/** + * Map a raw database entity row to the public Entity shape. + */ +function mapDbEntity(row: DbEntity): Entity { + const entity: Entity = { + id: row.id, + name: row.name, + fullName: row.full_name || undefined, + type: row.type || "unknown", + }; + + if (row.pronouns) entity.pronouns = row.pronouns; + if (row.trust_level) entity.trustLevel = row.trust_level; + if (row.last_seen) entity.lastSeen = row.last_seen; + if (row.created_at) entity.createdAt = row.created_at; + + return entity; +} + /** * Resolve an entity by various identifiers (original single-entity return). * Preserves backward compatibility โ€” returns the first matched entity or null. @@ -129,7 +148,13 @@ export async function resolveEntity(identifiers: EntityIdentifiers): Promise(query, values); if (result.rows.length > 0) { - const dbEntity = result.rows[0]; - return { - id: dbEntity.id, - name: dbEntity.name, - fullName: dbEntity.full_name || undefined, - type: dbEntity.type || "unknown", - }; + return mapDbEntity(result.rows[0]); } return null; @@ -207,7 +226,14 @@ export async function resolveEntityByIdentifiers( // Fetch ALL matching entities (no LIMIT) plus their matched facts const query = ` - SELECT DISTINCT e.id, e.name, e.full_name, e.type, ef.key AS fact_key, ef.value AS fact_value + SELECT DISTINCT e.id, e.name, e.full_name, e.type, + e.pronouns, e.trust_level, + -- entities.last_seen is plain timestamp (naive, no tz); do NOT apply + -- AT TIME ZONE 'UTC' because that interprets the value in the session + -- timezone and converts it. See nova-mind#543 / RS-062. + to_char(e.last_seen, 'YYYY-MM-DD HH24:MI "UTC"') AS last_seen, + to_char(e.created_at, 'YYYY-MM-DD') AS created_at, + ef.key AS fact_key, ef.value AS fact_value FROM entities e JOIN entity_facts ef ON e.id = ef.entity_id WHERE ${conditions.join(' OR ')} @@ -224,12 +250,7 @@ export async function resolveEntityByIdentifiers( for (const row of result.rows) { if (!entitiesById.has(row.id)) { entitiesById.set(row.id, { - entity: { - id: row.id, - name: row.name, - fullName: row.full_name || undefined, - type: row.type || 'unknown', - }, + entity: mapDbEntity(row), facts: [], }); } @@ -257,15 +278,21 @@ export async function resolveEntityByIdentifiers( } /** - * Get entity profile facts by entity ID + * Get entity profile facts by entity ID, plus cheap relationship stats. + * Returns allowlist facts, an unfiltered fact count, and the most recent + * channel transcript (timestamp + provider:external_chat_id ref) in a single + * round trip. See nova-mind#543. + * * @param entityId - Entity database ID * @param factKeys - Optional array of specific fact keys to retrieve - * @returns Object with fact key-value pairs + * @returns EntityProfile with facts and stats */ export async function getEntityProfile( entityId: number, factKeys?: string[] -): Promise { +): Promise { + const emptyProfile: EntityProfile = { facts: {}, stats: { factCount: 0, lastMessage: null } }; + try { const pool = getDbPool(); @@ -280,25 +307,76 @@ export async function getEntityProfile( "occupation", ]; + // Single aggregate query: fact count (unfiltered), last transcript, + // and filtered allowlist facts. Designed to stay inside the 1s race + // budget consumed by turn-context. const query = ` - SELECT key, value - FROM entity_facts - WHERE entity_id = $1 - AND key = ANY($2) - LIMIT 20 + SELECT + fc.count AS fact_count, + to_char(lm.timestamp AT TIME ZONE 'UTC', 'YYYY-MM-DD HH24:MI "UTC"') AS last_message_time, + lm.provider AS last_message_provider, + lm.external_chat_id AS last_message_external_chat_id, + facts.facts_json + FROM (SELECT COUNT(*) AS count FROM entity_facts WHERE entity_id = $1) fc + LEFT JOIN LATERAL ( + SELECT ct.timestamp, cs.provider, cs.external_chat_id + FROM channel_transcripts ct + JOIN channel_sessions cs ON ct.session_id = cs.id + WHERE ct.sender_entity_id = $1 + ORDER BY ct.timestamp DESC + LIMIT 1 + ) lm ON true + LEFT JOIN LATERAL ( + SELECT COALESCE( + jsonb_agg(jsonb_build_object('key', sub.key, 'value', sub.value)), + '[]'::jsonb + ) AS facts_json + FROM ( + SELECT key, value + FROM entity_facts + WHERE entity_id = $1 AND key = ANY($2) + LIMIT 20 + ) sub + ) facts ON true `; - const result = await pool.query(query, [entityId, keysToFetch]); + interface ProfileRow { + fact_count: string; + last_message_time: string | null; + last_message_provider: string | null; + last_message_external_chat_id: string | null; + facts_json: Array<{ key: string; value: string }>; + } + + const result = await pool.query(query, [entityId, keysToFetch]); + if (result.rows.length === 0) { + return emptyProfile; + } + + const row = result.rows[0]; const facts: EntityFacts = {}; - for (const row of result.rows) { - facts[row.key] = row.value; + for (const fact of row.facts_json || []) { + facts[fact.key] = fact.value; } + + const lastMessage = row.last_message_time + ? { + timestamp: row.last_message_time, + ref: `${row.last_message_provider}:${row.last_message_external_chat_id}`, + } + : null; - return facts; + return { + facts, + stats: { + factCount: parseInt(row.fact_count, 10) || 0, + lastMessage, + }, + }; } catch (err) { console.error("[entity-resolver] Profile loading error:", err instanceof Error ? err.message : String(err)); - return {}; + return emptyProfile; } } diff --git a/relationships/lib/entity-resolver/test.ts b/relationships/lib/entity-resolver/test.ts index 2fd6c66..9a7bee3 100755 --- a/relationships/lib/entity-resolver/test.ts +++ b/relationships/lib/entity-resolver/test.ts @@ -71,6 +71,11 @@ async function test() { return; } + if (identifier === "--relationship-stats-tests") { + await runRelationshipStatsTests(); + return; + } + console.log(`\n๐Ÿ” Testing entity resolution for: ${identifier}\n`); // Test 1: Resolve entity @@ -347,6 +352,255 @@ async function runIrcTests() { } } +/** + * RS-0xx: Relationship stats integration tests for nova-mind#543. + * + * These tests hit the live database (intended for nova-staging) and seed + * disposable rows in entities, entity_facts, channel_sessions, and + * channel_transcripts. IDs are chosen in the 99101โ€“99110 range to avoid + * collision with the IRC tests (99001โ€“99007). + */ +async function runRelationshipStatsTests() { + const client = await getDbClient(); + const testEntityIds = [99101, 99102, 99103]; + const testSessionIds: number[] = []; + + async function cleanup() { + await client.query( + `DELETE FROM channel_transcripts WHERE session_id = ANY($1)`, + [testSessionIds] + ); + await client.query( + `DELETE FROM channel_sessions WHERE id = ANY($1)`, + [testSessionIds] + ); + await client.query(`DELETE FROM entity_facts WHERE entity_id = ANY($1)`, [testEntityIds]); + await client.query(`DELETE FROM entities WHERE id = ANY($1)`, [testEntityIds]); + } + + async function seed() { + // Clean any stale test rows from a previous aborted run + await cleanup(); + + // RS-003/RS-044 rich entity: mixed visibility facts + transcript + await client.query( + `INSERT INTO entities ( + id, name, full_name, type, pronouns, trust_level, last_seen, created_at + ) VALUES ($1, 'Tabatha', 'Tabatha Janell Wilson', 'person', 'she/her', 'friend', + '2026-07-27 08:00:00'::timestamp, '2026-01-30'::timestamp)`, + [testEntityIds[0]] + ); + // 19 public + 15 trusted + 13 private + 1 timezone = 48 facts (unfiltered count test) + const visibilityFacts: Array<{ key: string; value: string; visibility: string; privacy_scope: number[] | null }> = []; + for (let i = 0; i < 19; i++) { + visibilityFacts.push({ key: `public_fact_${i}`, value: `v${i}`, visibility: "public", privacy_scope: null }); + } + for (let i = 0; i < 15; i++) { + visibilityFacts.push({ key: `trusted_fact_${i}`, value: `v${i}`, visibility: "trusted", privacy_scope: null }); + } + for (let i = 0; i < 13; i++) { + visibilityFacts.push({ key: `private_fact_${i}`, value: `v${i}`, visibility: "private", privacy_scope: null }); + } + // Bulk insert + const valuesSql = visibilityFacts + .map((_, idx) => `($1, $${idx * 4 + 2}, $${idx * 4 + 3}, $${idx * 4 + 4}, $${idx * 4 + 5})`) + .join(","); + const params = [testEntityIds[0], ...visibilityFacts.flatMap((f) => [f.key, f.value, f.visibility, f.privacy_scope])]; + await client.query( + `INSERT INTO entity_facts (entity_id, key, value, visibility, privacy_scope) VALUES ${valuesSql}`, + params + ); + // Allowlist fact for RS-004 regression + await client.query( + `INSERT INTO entity_facts (entity_id, key, value) VALUES ($1, 'timezone', 'America/Chicago')`, + [testEntityIds[0]] + ); + + // Create a channel session + transcript for last-message lookup + const sessionRes = await client.query( + `INSERT INTO channel_sessions (provider, external_chat_id, chat_type) + VALUES ('discord', '1513392492651872306', 'direct') + RETURNING id` + ); + const sessionId = sessionRes.rows[0].id; + testSessionIds.push(sessionId); + await client.query( + `INSERT INTO channel_transcripts ( + session_id, external_message_id, timestamp, sender_entity_id, content + ) VALUES ($1, 'msg-001', '2026-07-27 02:11:00+00', $2, 'hello')`, + [sessionId, testEntityIds[0]] + ); + // Older transcript โ€” should not be returned + await client.query( + `INSERT INTO channel_transcripts ( + session_id, external_message_id, timestamp, sender_entity_id, content + ) VALUES ($1, 'msg-002', '2026-07-26 10:00:00+00', $2, 'earlier')`, + [sessionId, testEntityIds[0]] + ); + + // RS-014 degenerate entity: zero facts, no transcripts + await client.query( + `INSERT INTO entities (id, name, type, trust_level, created_at) + VALUES ($1, 'Brand New User', 'person', 'unknown', '2026-07-28'::timestamp)`, + [testEntityIds[1]] + ); + + // RS-015 entity: last_seen populated but no transcripts + await client.query( + `INSERT INTO entities (id, name, type, last_seen, created_at) + VALUES ($1, 'No Transcript User', 'person', '2026-07-26 08:00:00'::timestamp, '2026-07-01'::timestamp)`, + [testEntityIds[2]] + ); + } + + let passed = 0; + let failed = 0; + + function check(name: string, condition: boolean, details?: string) { + if (condition) { + console.log(`โœ… ${name}`); + passed++; + } else { + console.log(`โŒ ${name}${details ? ` โ€” ${details}` : ""}`); + failed++; + } + } + + try { + await seed(); + console.log("\n๐Ÿ” RS-0xx relationship stats integration tests\n"); + + // RS-003 / RS-044: rich profile with unfiltered count + const profile = await getEntityProfile(testEntityIds[0]); + check( + "RS-003/RS-044: fact count is unfiltered (48) and allowlist facts present", + profile.stats.factCount === 48 && profile.facts.timezone === "America/Chicago" + ); + check( + "RS-003: last message timestamp + two-part ref", + profile.stats.lastMessage != null && + profile.stats.lastMessage.timestamp === "2026-07-27 02:11 UTC" && + profile.stats.lastMessage.ref === "discord:1513392492651872306" + ); + + // RS-002 / RS-001: pronouns and trust_level fetched in identifier query + const r002 = await resolveEntityByIdentifiers({ discordId: "1513392492651872306" }); + // The seeded entity has no discord_id fact, so this will not resolve by discordId. + // Instead resolve by the explicit entity id fact we add here: + await client.query( + `INSERT INTO entity_facts (entity_id, key, value) VALUES ($1, 'discord_id', 'rs543-tabatha')`, + [testEntityIds[0]] + ); + const r002b = await resolveEntityByIdentifiers({ discordId: "rs543-tabatha" }); + check( + "RS-001/RS-002: pronouns and trust_level resolved with identifier query", + r002b != null && + r002b.ok === true && + r002b.entity.pronouns === "she/her" && + r002b.entity.trustLevel === "friend" && + r002b.entity.createdAt === "2026-01-30" + ); + + // RS-062 regression: entities.last_seen is plain timestamp (naive). Applying + // AT TIME ZONE 'UTC' under a non-UTC session timezone silently shifts the + // displayed value. Verify direct formatting is stable. + await client.query("SET timezone = 'America/Chicago'"); + const r062 = await resolveEntity({ discordId: "rs543-tabatha" }); + check( + "RS-062: last_seen formatting is stable under non-UTC session timezone", + r062 != null && r062.lastSeen === "2026-07-27 08:00 UTC", + `got lastSeen=${r062?.lastSeen}` + ); + // channel_transcripts.timestamp is timestamptz, so its UTC rendering should + // also remain correct under the shifted session timezone. + const profileTz = await getEntityProfile(testEntityIds[0]); + check( + "RS-062: last_message timestamp remains UTC-labeled under non-UTC session timezone", + profileTz.stats.lastMessage?.timestamp === "2026-07-27 02:11 UTC", + `got lastMessage.timestamp=${profileTz.stats.lastMessage?.timestamp}` + ); + await client.query("RESET timezone"); + + // RS-014: degenerate entity + const profileNew = await getEntityProfile(testEntityIds[1]); + check( + "RS-014: zero facts + no transcripts โ†’ empty profile, zero count", + profileNew.stats.factCount === 0 && profileNew.stats.lastMessage === null && Object.keys(profileNew.facts).length === 0 + ); + + // RS-015: last_seen fallback scenario (no transcripts but last_seen set) + const profileFallback = await getEntityProfile(testEntityIds[2]); + check( + "RS-015: no transcripts โ†’ lastMessage null (last_seen is an entity field, not a profile stat)", + profileFallback.stats.factCount === 0 && profileFallback.stats.lastMessage === null + ); + + // RS-050: simple timing check (informational, generous ceiling) + const start = process.hrtime.bigint(); + await getEntityProfile(testEntityIds[0]); + const elapsedMs = Number(process.hrtime.bigint() - start) / 1_000_000; + check( + `RS-050: aggregate query completes within budget (${elapsedMs.toFixed(2)}ms)`, + elapsedMs < 200, + `took ${elapsedMs.toFixed(2)}ms` + ); + + // RS-051: EXPLAIN confirms index usage + const explainResult = await client.query( + `EXPLAIN (FORMAT JSON) + SELECT + fc.count AS fact_count, + to_char(lm.timestamp AT TIME ZONE 'UTC', 'YYYY-MM-DD HH24:MI "UTC"') AS last_message_time, + lm.provider AS last_message_provider, + lm.external_chat_id AS last_message_external_chat_id, + facts.facts_json + FROM (SELECT COUNT(*) AS count FROM entity_facts WHERE entity_id = $1) fc + LEFT JOIN LATERAL ( + SELECT ct.timestamp, cs.provider, cs.external_chat_id + FROM channel_transcripts ct + JOIN channel_sessions cs ON ct.session_id = cs.id + WHERE ct.sender_entity_id = $1 + ORDER BY ct.timestamp DESC + LIMIT 1 + ) lm ON true + LEFT JOIN LATERAL ( + SELECT COALESCE( + jsonb_agg(jsonb_build_object('key', sub.key, 'value', sub.value)), + '[]'::jsonb + ) AS facts_json + FROM ( + SELECT key, value + FROM entity_facts + WHERE entity_id = $1 AND key = ANY($2) + LIMIT 20 + ) sub + ) facts ON true`, + [testEntityIds[0], ["timezone"]] + ); + const planText = JSON.stringify(explainResult.rows[0]["QUERY PLAN"]); + const hasSeqScanEntityFacts = planText.includes('"Node Type": "Seq Scan"') && planText.includes("entity_facts"); + const hasSeqScanTranscripts = planText.includes('"Node Type": "Seq Scan"') && planText.includes("channel_transcripts"); + check( + "RS-051: EXPLAIN shows no seq scan on entity_facts or channel_transcripts", + !hasSeqScanEntityFacts && !hasSeqScanTranscripts, + planText + ); + + console.log(`\n${passed} passed, ${failed} failed\n`); + if (failed > 0) { + process.exitCode = 1; + } + } catch (err) { + console.error("\nโŒ Relationship stats tests failed:", err instanceof Error ? err.message : String(err)); + console.error((err as Error).stack); + process.exitCode = 1; + } finally { + await cleanup(); + await client.end(); + await closeDbPool(); + } +} + test().catch(async (err) => { console.error("\nโŒ Test failed:", err.message); console.error(err.stack); diff --git a/relationships/lib/entity-resolver/types.ts b/relationships/lib/entity-resolver/types.ts index 857fb61..ccd44df 100644 --- a/relationships/lib/entity-resolver/types.ts +++ b/relationships/lib/entity-resolver/types.ts @@ -7,12 +7,36 @@ export interface Entity { name: string; fullName?: string; type: string; + pronouns?: string; + trustLevel?: string; + lastSeen?: string; + createdAt?: string; } export interface EntityFacts { [key: string]: string; } +/** + * Relationship stats for an entity. + */ +export interface EntityRelationshipStats { + factCount: number; + lastMessage: { + timestamp: string; + ref: string; + } | null; +} + +/** + * Entity profile returned by getEntityProfile(). + * Combines the existing allowlist facts with cheap relationship stats. + */ +export interface EntityProfile { + facts: EntityFacts; + stats: EntityRelationshipStats; +} + /** * Identifiers that can be used to resolve an entity */ @@ -47,6 +71,10 @@ export interface DbEntity { name: string; full_name: string | null; type?: string; + pronouns?: string | null; + trust_level?: string | null; + last_seen?: string | null; + created_at?: string | null; } /** diff --git a/tests/DOC-AUDIT-543.md b/tests/DOC-AUDIT-543.md new file mode 100644 index 0000000..742cb76 --- /dev/null +++ b/tests/DOC-AUDIT-543.md @@ -0,0 +1,80 @@ +# Documentation Audit โ€” nova-mind#543 (SE Run #513, Step 9, Technical Writing) + +**Scope:** Document the entity-resolver pronouns + relationship-stats feature (commits `8ba578e` +feat + `bfdbe39` fix) and audit all project documentation against current source for staleness +introduced or exposed by this change. + +**Branch:** `feature/543-entity-resolver-relationship-stats`. + +--- + +## 1. Source Verified + +Read in full before editing any docs: +- `relationships/lib/entity-resolver/resolver.ts` (`mapDbEntity()`, `getEntityProfile()` aggregate + query, timezone-safe `last_seen`/`created_at` rendering) +- `relationships/lib/entity-resolver/types.ts` (`Entity`, `EntityProfile`, + `EntityRelationshipStats`) +- `relationships/lib/entity-resolver/index.ts` (exports) +- `memory/plugins/turn-context/src/entity-resolver.ts` (`formatEntityContext()`, + `resolveEntityContext()` timeout/race behavior, honorific-guard non-interaction) +- `database/schema.sql` (`entities.trust_level` column comment, actual column list) +- Both commit diffs (`git show 8ba578e`, `git show bfdbe39`) + +## 2. Files Edited (and why) + +| File | Reason | +|------|--------| +| `relationships/lib/entity-resolver/README.md` | Added `deviceId`/`ircUsername` to identifier list; documented `Entity`'s new optional `pronouns`/`trustLevel`/`lastSeen`/`createdAt` fields; rewrote `getEntityProfile()` section for its new `EntityProfile` (`{facts, stats}`) return type (was a bare `EntityFacts` map โ€” breaking signature change); updated Types section with `EntityRelationshipStats`/`EntityProfile`; fixed a stale usage example reading `profile.timezone` directly. | +| `relationships/ARCHITECTURE-entity-resolver.md` | Updated `resolveEntity()`'s documented `Entity` return shape to include the four new optional fields; rewrote `getEntityProfile()` API reference for the new `EntityProfile` shape with full field semantics (`factCount` unfiltered, `lastMessage` nullable); updated the "Query Strategy" SQL example to show the actual `pronouns`/`trust_level`/`last_seen`/`created_at` columns and added a dedicated note explaining the `last_seen` timezone-shift bug and its fix (RS-062); fixed ~10 stale `profile.timezone`/`profile.communication_style`/`profile.expertise` usages across integration examples (now `profile.facts.*`) that would otherwise silently break under the new return type. | +| `relationships/README.md` | Added a relationship-stats/pronouns bullet under the Entity Resolver Library key-component description, pointing to the full API doc. | +| `relationships/CHANGELOG.md` | Added `## Unreleased` entries for #543: pronouns/trust/stats feature (Added), the `last_seen` timezone fix (Fixed), the `trust_level` column-comment change (Changed), and test coverage (Tests). | +| `memory/plugins/turn-context/README.md` | Updated the Entity Resolver subsystem table row to mention pronouns/trust/stats rendering; added a new "Entity Context Formatting" section documenting `formatEntityContext()`'s exact 3-line output format (header/stats/facts), the `'unknown'`-trust suppression rule, the zero-facts-suppression rule for the stats line, and the timeout/data-source split that lets pronouns/trust survive a `getEntityProfile()` timeout while the stats line does not. | +| `memory/docs/semantic-recall.md` | Updated the Entity Resolution section's illustrative output block to show pronouns + the new stats line (was missing both, would otherwise look like #543 was never applied to this integration point), with a pointer to the new turn-context README section for full rules. | +| `psyche/ARCHITECTURE-entities-users.md` | Corrected the `trust_level` column description, which asserted a closed enum of `owner/admin/user/unknown/untrusted` โ€” the live `database/schema.sql` column comment (updated by #543's `8ba578e`) now explicitly documents it as free-text with no CHECK constraint. Clarified that the enum values are only meaningful to `confidence_helper.py`'s scoring table, not an enforced value set, and cross-referenced the new turn-context trust-suffix rendering. | +| `CHANGELOG.md` (root) | Added a full `### Batch: entity-resolver-relationship-stats-543` entry (Added/Fixed/Tests/Issues Closed), following the existing batch-entry convention used for #508/#522/#506, since those precedents document root-level, cross-cutting behavior changes at this granularity. | +| `tests/DOC-AUDIT-543.md` | This file. | + +## 3. Files Audited โ€” Already Correct, No Change Needed + +| File | Why it's clean | +|------|-----------------| +| `relationships/CONTRIBUTING.md` | No `getEntityProfile()`/`Entity` shape documentation to go stale; general contribution guidance only. | +| `relationships/docs/algorithms.md` | Explicitly marked "design phase โ€” not yet implemented" and unrelated to the resolver's actual shipped API; the design-phase types shown are not `EntityProfile`/`Entity`. | +| `relationships/docs/web-of-trust.md` | Its `trustLevel` field is an unrelated cert-vouch concept (`'full' \| 'limited' \| 'vouch-only'`), not `entities.trust_level`. Verified by reading the surrounding context โ€” no accidental collision. | +| `relationships/docs/integration-guide.md` | Fixed the direct `profile.timezone`-style accesses (see edits above) โ€” after those fixes, remainder of the doc (identifier lists, caching patterns, certificate/CA examples) is accurate. | +| `relationships/lib/entity-resolver/COMPLETED.md` | Dated historical completion snapshot (2025-02-08, Task #38, pre-dates #543 by over a year). Left as a historical record except for the one line reading `profile.timezone` directly, fixed since it would silently break if anyone copy-pasted it today. | +| `relationships/lib/entity-resolver/REFACTORING.md` | Same treatment as COMPLETED.md โ€” historical Task #38 snapshot; fixed the one stale `profile.timezone, profile.communication_style` line for the same reason. | +| `database/schema-reference.md` | `entities` row still correctly lists 22 columns โ€” #543 added no new columns (it only changed a column *comment*), so no count drift. Already-flagged staleness in this auto-generated file (agent_chat, portfolio tables, etc.) predates #543 and is out of scope. | +| `memory/docs/fact-judgement-model.md` | Its `trust_level`/confidence discussion concerns `confidence_helper.py`'s scoring table, not the turn-context rendering behavior #543 changed. Confirmed no reference to the injected-context format. | +| `memory/docs/SOURCE-AUTHORITY.md` | Same as above โ€” `trust_level` confidence-scoring content is orthogonal to #543's display-layer change. Already carries its own (pre-existing, accurate) staleness caveats re: pre-#174 authority pipeline; not touched further here. | +| `memory/docs/memory-extraction-pipeline.md` | Single `trust_level` mention is about `confidence_helper.py` initial-confidence scoring, unrelated to #543. | +| `ARCHITECTURE.md` (root) | Mentions of `turn-context`/`entity-resolver`/`getEntityProfile()` are all high-level architecture pointers that don't assert a specific return shape or rendering format โ€” not made stale by #543. | +| `memory/docs/database-schema-guide.md` | Its `entities` table SQL example was already a simplified/pre-existing illustration (missing `pronouns`, `trust_level`, and most real columns) that predates #543 and was never accurate to the live schema. Not a regression caused by this change โ€” flagged as an out-of-scope pre-existing gap below rather than fixed here. | + +## 4. Out-of-Scope Gaps Found (flagged for follow-up, not fixed here) + +- **`memory/docs/database-schema-guide.md`'s `entities` table example is stale independent of + #543** โ€” it shows a minimal illustrative `CREATE TABLE entities` (6 columns: id, name, type, + full_name, description, created_at/updated_at) that doesn't match the live 22-column schema at + all (no `pronouns`, `trust_level`, `last_seen`, `nicknames`, etc., and even has columns like + `description`/`updated_at` that don't exist on the real table). This predates #543 and is a + general schema-guide accuracy issue, not something #543 introduced or worsened โ€” recommend a + separate doc-accuracy issue rather than expanding this PR's scope. + +## 5. Consistency Check: `trust_level` Free-Text Semantics + +Per the task brief's instruction to verify trust_level free-text semantics wherever documented: +confirmed consistent across `database/schema.sql` (column comment, updated by #543's `8ba578e`), +`relationships/ARCHITECTURE-entity-resolver.md` (new `Entity.trustLevel` doc), `memory/plugins/ +turn-context/README.md` (new Entity Context Formatting section), and `psyche/ +ARCHITECTURE-entities-users.md` (corrected in this pass). `memory/scripts/confidence_helper.py`'s +hardcoded five-value scoring table (`owner`/`admin`/`user`/`unknown`/`untrusted`) is unaffected by +#543 and continues to work as a *scoring* lookup with a default fallback for unrecognized values โ€” +it does not enforce a closed set at the schema level, matching the corrected documentation. + +## Conclusion + +Documentation is now consistent with the merged `8ba578e`/`bfdbe39` behavior. No blocking gaps +remain in scope for #543. One pre-existing, unrelated staleness item is flagged in ยง4 for a +separate follow-up issue.