Adaptive profile extraction + confidence_score 400 fix#122
Conversation
Makes the AU/real_life adaptive learning loop actually work instead of writing empty stub profiles. - Add deterministic adaptive-profile-extractor (no model calls / embeddings): classifies memory events into preferences, facts, decisions, open loops, risks and assigns a bounded confidence. - Wire refresh_adaptive_profiles to extract real content and upsert a populated versioned profile (was an empty "refresh requested" stub). - Fix hybrid retrieval 400: select confidence_score (the real Phase 5D column) instead of the non-existent `confidence`; map it back so ranking still uses the stored score. - Surface the extracted operating profile in get_adaptive_context (writing_rules/business_rules/decisions/facts + summary/confidence), additive so existing fields are unchanged. - Additive migration: add supersedes_profile_id/superseded_at to memory_profiles so versioned profile writes succeed on re-runs. - Tests prove the full loop: extract -> confidence -> save -> refresh -> retrieve via get_adaptive_context, plus the confidence_score fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds a deterministic adaptive-profile extractor, a refresh function that persists extracted profiles via versioned upsert, wires the MCP refresh tool to it, updates ChatGPT context building to surface extracted preferences/facts/decisions, switches hybrid retrieval to use confidence_score, adds a DB migration, and adds unit tests. ChangesAdaptive Profile Extraction and Refresh Loop
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed due to a network error. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72908e588f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (seen.has(key)) continue; | ||
| seen.add(key); | ||
|
|
||
| const item: AdaptiveProfileItem = { text: clip(raw), event_id: event.id ?? null, importance: event.importance ?? null }; |
There was a problem hiding this comment.
When a captured event contains a token/password (possible through direct MCP/bridge capture before candidate review), classifyContent returns secret, but the extractor still builds item from the raw event text and the switch below can persist it into patterns or risks via refreshAdaptiveProfileFromEvents. Since active memory_profiles are returned wholesale by hybrid context, a profile refresh can duplicate and expose credentials instead of honoring the existing secret-blocking contract; skip or redact category === "secret" before saving profile items.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/adaptive-profile-loop.test.ts (1)
137-149: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winStrengthen the
confidencevsconfidence_scoreregression check.
expect(eventsSelect).not.toContain(",confidence,")(Line 148) won't catch a regression where the bareconfidencecolumn appears as the first or last entry in the select list (e.g."confidence,id,..."or"...,id,confidence", missing one of the surrounding commas). Since this test's stated purpose is guarding against the exact 400-causing bug this PR fixes, a word-boundary regex is safer.🐛 Proposed fix
- expect(eventsSelect).not.toContain(",confidence,"); + expect(eventsSelect).not.toMatch(/\bconfidence\b(?!_score)/);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/adaptive-profile-loop.test.ts` around lines 137 - 149, Strengthen the regression assertion in the getHybridMemoryContext test so it catches any bare confidence column, not just the middle-of-list case. Replace the current eventsSelect containment check with a word-boundary style validation that fails whether confidence appears at the start, middle, or end of the select list, while still confirming confidence_score is requested. Keep the focus of the test on the fakeClient memory_events select capture and the hybrid retrieval behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/services/adaptive-profile-extractor.ts`:
- Around line 74-100: The switch in adaptive profile extraction allows
`"secret"` items from `classifyContent` to fall through into `patterns`, which
can persist sensitive text. Update the logic in the extraction flow around
`classifyContent`, `RISK_RE.test`, and the category switch to explicitly skip
any `"secret"` category before any array pushes. Ensure secret/credential items
are neither added to `patterns` nor to other profile buckets, even when they do
not match `RISK_RE` or `DECISION_RE`.
---
Nitpick comments:
In `@tests/unit/adaptive-profile-loop.test.ts`:
- Around line 137-149: Strengthen the regression assertion in the
getHybridMemoryContext test so it catches any bare confidence column, not just
the middle-of-list case. Replace the current eventsSelect containment check with
a word-boundary style validation that fails whether confidence appears at the
start, middle, or end of the select list, while still confirming
confidence_score is requested. Keep the focus of the test on the fakeClient
memory_events select capture and the hybrid retrieval behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 701d035e-1591-497f-89ba-e24944ab052e
📒 Files selected for processing (7)
lib/services/adaptive-chatgpt-context-service.tslib/services/adaptive-profile-extractor.tslib/services/memory-hybrid-retrieval-service.tslib/services/memory-profile-service.tslib/services/pandora-mcp-tools.tssupabase/migrations/20260702000000_adaptive_profile_versioning_columns.sqltests/unit/adaptive-profile-loop.test.ts
| const category = classifyContent({ | ||
| text: raw, | ||
| memory_type: event.memory_type ?? null, | ||
| importance: event.importance ?? null, | ||
| source: event.source ?? null, | ||
| }); | ||
| const sensitive = event.sensitivity === "high" || event.sensitivity === "private"; | ||
| const isRisk = RISK_RE.test(raw) || sensitive; | ||
| const isDecision = DECISION_RE.test(raw); | ||
|
|
||
| if (isRisk) risks.push(item); | ||
| if (isDecision) decisions.push(item); | ||
|
|
||
| switch (category) { | ||
| case "durable_preference": | ||
| preferences.push(item); | ||
| break; | ||
| case "production_fact": | ||
| facts.push(item); | ||
| break; | ||
| case "task_state": | ||
| openLoops.push(item); | ||
| break; | ||
| default: | ||
| if (!isRisk && !isDecision) patterns.push(item); | ||
| break; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Secret-classified content can leak into the stored patterns array.
classifyContent returns "secret" for text/memory_type matching credential patterns, and per its own contract Secrets/credentials always win and are blocked. Here, however, category "secret" falls through the default branch of the switch and is pushed into patterns unless the text also happens to match RISK_RE or DECISION_RE (Line 98). Since sensitivity and risk keywords are independent of secret detection, a credential/secret event can be silently stored (raw, unredacted item.text) in the profile's patterns field, which is then persisted via upsertVersionedMemoryProfile and returned as part of ctx.adaptive_profile from getHybridMemoryContext.
Add an explicit skip for the secret category before classification/push.
🔒 Proposed fix
const category = classifyContent({
text: raw,
memory_type: event.memory_type ?? null,
importance: event.importance ?? null,
source: event.source ?? null,
});
+ if (category === "secret") continue;
const sensitive = event.sensitivity === "high" || event.sensitivity === "private";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const category = classifyContent({ | |
| text: raw, | |
| memory_type: event.memory_type ?? null, | |
| importance: event.importance ?? null, | |
| source: event.source ?? null, | |
| }); | |
| const sensitive = event.sensitivity === "high" || event.sensitivity === "private"; | |
| const isRisk = RISK_RE.test(raw) || sensitive; | |
| const isDecision = DECISION_RE.test(raw); | |
| if (isRisk) risks.push(item); | |
| if (isDecision) decisions.push(item); | |
| switch (category) { | |
| case "durable_preference": | |
| preferences.push(item); | |
| break; | |
| case "production_fact": | |
| facts.push(item); | |
| break; | |
| case "task_state": | |
| openLoops.push(item); | |
| break; | |
| default: | |
| if (!isRisk && !isDecision) patterns.push(item); | |
| break; | |
| } | |
| const category = classifyContent({ | |
| text: raw, | |
| memory_type: event.memory_type ?? null, | |
| importance: event.importance ?? null, | |
| source: event.source ?? null, | |
| }); | |
| if (category === "secret") continue; | |
| const sensitive = event.sensitivity === "high" || event.sensitivity === "private"; | |
| const isRisk = RISK_RE.test(raw) || sensitive; | |
| const isDecision = DECISION_RE.test(raw); | |
| if (isRisk) risks.push(item); | |
| if (isDecision) decisions.push(item); | |
| switch (category) { | |
| case "durable_preference": | |
| preferences.push(item); | |
| break; | |
| case "production_fact": | |
| facts.push(item); | |
| break; | |
| case "task_state": | |
| openLoops.push(item); | |
| break; | |
| default: | |
| if (!isRisk && !isDecision) patterns.push(item); | |
| break; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/services/adaptive-profile-extractor.ts` around lines 74 - 100, The switch
in adaptive profile extraction allows `"secret"` items from `classifyContent` to
fall through into `patterns`, which can persist sensitive text. Update the logic
in the extraction flow around `classifyContent`, `RISK_RE.test`, and the
category switch to explicitly skip any `"secret"` category before any array
pushes. Ensure secret/credential items are neither added to `patterns` nor to
other profile buckets, even when they do not match `RISK_RE` or `DECISION_RE`.
2 participants
What
Makes the adaptive learning loop actually populate and surface real profile data, and fixes the
get_adaptive_context400. Follow-up #1 from the AU Memory Engine work.Changes
lib/services/adaptive-profile-extractor.ts: deterministic (no model calls / embeddings), classifies memory events into preferences / facts / decisions / open loops / risks and assigns a bounded confidence, reusing the Phase 5DclassifyContent.refreshAdaptiveProfileFromEventsreads namespace-scoped events, extracts, and upserts a populated versioned profile (previously an empty"MCP adaptive profile refresh requested."stub). MCP tool wired to it.confidencecolumn (the real Phase 5D column isconfidence_score) → PostgREST 400 that silently emptiedget_adaptive_context's event/profile data. Now selectsconfidence_scoreand maps it back so ranking still uses the stored score.writing_rules(au) /business_rules(real_life); decisions →decision_rules; facts →do_not_forget; plus newadaptive_profile_summary/adaptive_profile_confidence. Additive: every existing field preserved.Schema changes
One additive migration
supabase/migrations/20260702000000_adaptive_profile_versioning_columns.sql:upsertVersionedMemoryProfilewrites these supersession-chain columns, but they were never created (absent from every migration and from the deployed table) — so a non-dry profile write 400'd on re-runs. Fully additive; nothing dropped. Not auto-applied — apply via the reviewed migration step before enabling non-dry refresh in prod.Tests —
tests/unit/adaptive-profile-loop.test.ts(5/5)Proves the full loop with an in-memory client: extract canon/preference/business-fact/decision → assign confidence → save populated profile (non-dry) → dry-run previews without writing → retrieve via
get_adaptive_context→ confidence_score select fix.Verification
first-reviewed-memory-fixture→spawnSync npm ENOENT) is a pre-existing sandbox-only flake in an untouched file.Hard-rules compliance
real_lifemaster packs touched. No AU↔real_life mixing. Existing retrieval (get_memory_context/get_latest_context_pack) behavior intact (additive only). No gated features enabled — still model-free and embedding-free.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests