feat: Proposal A v3 - Dynamic Importance Feedback Signals (Phase 1) by jlin53882 · Pull Request #507 · CortexReach/memory-lancedb-pro

jlin53882 · 2026-04-04T07:11:53Z

Summary

Proposal A Phase 1: Dynamic Importance Feedback Signals. Implements per-recall usage tracking and importance scoring via hooks on agent lifecycle events.

What was built

Hooks

agent_end (priority 20): Captures agent response text into pendingRecall
before_prompt_build (priority 5): Scores recall usage on next turn
session_end (priority 20): Cleans up pendingRecall entries

isRecallUsed() AND Logic

Checks for specific memory ID presence AND generic usage markers (both required):

const hasSpecificRecall = injectedIds.some(id => response.includes(id));
if (hasSpecificRecall) {
  for (const marker of usageMarkers) {
    if (response.includes(marker)) return true;
  }
}

Per-Recall Scoring

Each recallId scored individually — no cross-contamination between memories.

Key Fixes (from #493 review)

AND logic prevents generic phrases from boosting all injected memories
bad_recall_count increments correctly in error/miss paths
sessionKey:agentId composite key prevents cross-agent interference
Session_end properly cleans composite keys (no memory leak when autoCapture=false)

Files changed

index.ts: +272 lines (hooks, scoring loop, pendingRecall map)
src/reflection-slices.ts: +95 lines (isRecallUsed function)
src/auto-capture-cleanup.ts: +11 lines (boilerplate stripping)
src/smart-extractor.ts: +2 lines (regex fix)
src/retriever.ts: +9 lines (recency double-boost guard)

- Add pendingRecall Map for tracking session recalls - Add agent_end hook to store response text for usage scoring - Add before_prompt_build hook (priority 5) to score recall usage - Add session_end hook to clean up pending recalls - Add isRecallUsed function to reflection-slices.ts - Guard: skip scoring for empty responseText (<=24 chars) Implements: recall usage tracking for Proposal A Phase 1

1. Bug 1 (CRITICAL): injectedIds regex in feedback hook never matched - The feedback hook used a regex /\[([a-f0-9]{8,})\]/gi to parse IDs from prependContext, but auto-recall injects memories in format [preferences:global], [facts:dc-channel], NOT [hex-id]. - Fix: read recallIds directly from pendingRecall (which is populated by auto-recall's before_prompt_build from the previous turn). Also added code in auto-recall to store selected IDs into pendingRecall[sessionKey].recallIds before returning. 2. Bug 2 (MAJOR): stripEnvelopeMetadata regex had literal backspace (0x08) - In src/smart-extractor.ts line 76, a literal backspace character (byte 0x08) was embedded in the regex pattern between 'agent' and '.', producing 'agent[0x08].*?' instead of 'agent\b.*?'. - Fix: replaced the 0x08 byte with the proper \b word boundary. 3. Bug 3 (MAJOR): WeakSet.clear() does not exist - In index.ts resetRegistration(), _registeredApis.clear() was called, but WeakSet has no clear() method. - Fix: removed the .clear() call per the comment's own note.

…g, parseSmartMetadata, importance row update) Bug 1 (P1): pendingRecall was written with recallIds from Turn N but responseText from Turn N-1, causing feedback to score the wrong memories. Fix: before_prompt_build (auto-recall) now CREATES pendingRecall with recallIds. agent_end now only WRITES responseText to an existing entry (never creates). Bug 2 (P2): parseSmartMetadata was called with empty placeholder metadata, returning fallback values instead of real entry data. Fix: use store.getById(recallId) to get the real entry before parsing. Bug 3 (P2): patchMetadata only updates the metadata JSON blob, not the entry.importance ROW column. applyImportanceWeight reads entry.importance, so importance adjustments never affected ranking. Fix: use store.update(id, { importance: newValue }) to update the row directly.

Bug 1 [P1]: pendingRecall.delete() moved from session_end to feedback hook finally block — prevents repeated scoring of the same recallIds/ responseText pair when subsequent turns skip auto-recall (greeting, short input). Now deleted immediately after scoring completes. Bug 2 [P2]: confirmed use now resets bad_recall_count to 0 — so penalty threshold (3) only applies to truly consecutive misses, not interleaved confirmed-use/miss patterns. Bug 3 [P3]: retrieveWithTrace now forwards source to hybridRetrieval(), aligning debug/trace retrieval with real manual-recall behavior.

…anup, env-resolve gate, recency double-boost)

P1-1 (isRecallUsed): Add direct injected-ID check - The function accepted injectedIds but never used them - Added loop to check if response contains any injected memory ID - This complements the existing stock-phrase check P1-2 (rerank env vars): Add rerank-enabled guard - Only resolve \ placeholders when rerank is actually enabled - Prevents startup failure when rerankApiKey has unresolved placeholder but reranking is disabled (rerank='none') P2 (multi-line wrapper stripping): Strip boilerplate continuation lines - stripLeadingRuntimeWrappers now also strips lines matching AUTO_CAPTURE_RUNTIME_WRAPPER_BOILERPLATE_RE (e.g. 'Results auto-announce to your requester.', 'Do not use any memory tools.') while strippingLeadIn is still true, preventing these lines from being kept when they appear right after the wrapper prefix line

…d configurable feedback amplitudes

…er prompt extraction, parsePluginConfig feedback, bad_recall_count double-increment) Bug 1 (P1): isRecallUsed() only checked stock phrases and raw IDs, but auto-recall injects [category:scope] summary format text. Fix: store injectedSummaries (item.line) in pendingRecall on auto-recall injection; pass them to isRecallUsed() which now checks if the response contains any of the injected summary text verbatim. Bug 2 (P1): confirm/error keywords were checked against pending.responseText (previous-turn assistant response) instead of the current-turn user prompt. Fix: read event.prompt (array of {role, content} messages) in the before_prompt_build feedback hook and check keywords against the last user message in that array. Bug 3 (P2): parsePluginConfig() never copied cfg.feedback to the returned config object, so all deployments fell back to hardcoded defaults. Fix: add feedback block to the return object in parsePluginConfig. Bug 4 (P2): bad_recall_count was incremented in BOTH the auto-recall injection path AND the feedback hook, causing double-counting that made the 3-consecutive-miss penalty trigger after only 2 actual misses. Fix: remove +1 from the feedback hook; counter now only increments once (in the auto-recall injection path where staleInjected is evaluated).

…ssages user prompt, agentId keying Bug 1 (P1): Score each recall independently instead of one usedRecall for the whole batch. - Build summaryMap: recallId -> injected summary - Call isRecallUsed per recallId with its specific summary - Prevents unused memories from being boosted or used ones penalized Bug 2 (P2): Extract user prompt from event.messages array, not event.prompt. - event.prompt is a plain string (confirmed by codebase usage), not an array - Extract last user message from event.messages (same pattern as agent_end) Bug 3 (P2): pendingRecall key includes agentId to avoid cross-agent overwrite. - Key format: sessionKey:agentId (both in auto-recall and feedback/agent_end hooks)

P1 fix: replace single-char CJK keywords (是/對/不/錯) with longer phrases (是對的/確認/錯誤/更正) to avoid false positives on ordinary conversation. P3 fix: session_end hook was not cleaning pendingRecall at all. Add cleanup of all pendingRecall entries that match the sessionId or sessionKey:agentId composite key pattern.

…ach#445)

…ory leak When config.autoCapture === false, the auto-capture session_end (priority 10) was skipped, leaving only the Phase 1 session_end (priority 20) to clean up. The old code only deleted pendingRecall[sessionKey] - a simple key - but not composite keys (sessionKey:agentId). Now uses pattern matching (startsWith) to clean all related keys regardless of format. Fixes: P1 issue from Phase 1 audit

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 649e90631b

ℹ️ 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".

chatgpt-codex-connector · 2026-04-04T07:19:03Z

index.ts

      api.on("agent_end", agentEndAutoCaptureHook);
-    }
+
+    // ========================================================================


Restore autoCapture block boundary before feedback hooks

The if (config.autoCapture !== false) block is no longer closed before the new feedback-hook section, so subsequent registrations (e.g., selfImprovement hooks starting at if (config.selfImprovement?.enabled !== false)) are now conditionally skipped whenever autoCapture is disabled. This is a behavioral regression for configs that set autoCapture: false but still rely on command/session hooks, and it changes feature wiring far beyond recall feedback.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-04-04T07:19:03Z

index.ts

+    feedback: typeof cfg.feedback === "object" && cfg.feedback !== null
+      ? { ...(cfg.feedback as Record<string, unknown>) }


Validate feedback config field types before consumption

parsePluginConfig now forwards cfg.feedback as an untyped object, but the scoring hook later assumes confirmKeywords/errorKeywords are arrays and calls .some(...) on them. If a deployment supplies these as strings (a common JSON/YAML misconfiguration), the hook throws and the catch path disables recall scoring for that turn, so custom feedback settings can silently break the new feature.

Useful? React with 👍 / 👎.

AliceLJY

The feedback signal design is conceptually sound -- per-recall scoring with AND logic is a real improvement over naive batch scoring. However, this PR chain has structural and code issues that need addressing:

Branch topology problem

PRs #507 (Phase 1), #505 (Phase 3), and #506 (Phase 4) all target master independently. They modify the same files (index.ts, reflection-slices.ts) with overlapping changes. Merging them independently will produce conflicts.

Fix: Either stack them as a chain (#507 -> #505 -> #506 with each targeting the predecessor branch), or collapse all three into a single PR.

Code issues in Phase 1 (#507)

autoCapture block boundary -- the feedback scoring code may be placed outside the intended block scope. Verify the indentation and braces.
Unrelated bug fixes bundled -- please separate them into their own commits or PRs.
isRecallUsed reverse-check has a false positive risk -- a user saying "that's not right" could be matched as recall usage confirmation.

Phase 3 (#505)

Keyword defaults need to be reconciled with Phase 1 hardcoded values.
Must rebase onto Phase 1 branch.

Phase 4 (#506)

Should import real FeedbackConfigManager instead of mocking it.
Needs integration tests for the hook lifecycle (pendingRecall -> agent_end -> scoring).
Must rebase onto Phase 3 branch.

Recommendation: Fix the branch topology first, then address the code issues. Please discuss the approach in issue #445 before re-submitting.

jlin53882 and others added 13 commits April 3, 2026 19:08

fix(proposal-a): 4 final bugs (importance fallback, pendingRecall cle…

469e069

…anup, env-resolve gate, recency double-boost)

feat(proposal-a): add user confirmation signals, min_recall_count, an…

33b721a

…d configurable feedback amplitudes

fix: change bad_recall threshold from 3 to 2 per spec (issue CortexRe…

b92c47c

…ach#445)

fix(recall): AND logic in isRecallUsed + bad_recall_count increment fix

bd0c582

chatgpt-codex-connector bot reviewed Apr 4, 2026

View reviewed changes

AliceLJY requested changes Apr 4, 2026

View reviewed changes

This was referenced Apr 4, 2026

feat: Proposal A v3 - Configurable Feedback Amplitudes (Phase 3) #505

Closed

test: Proposal A v3 - Unit Tests for Feedback Scoring (Phase 4) #506

Open

jlin53882 mentioned this pull request Apr 4, 2026

[RFC] Proposal A & B Implementation Analysis #445

Open

rwmjhb closed this Apr 5, 2026

This was referenced Apr 9, 2026

[RFC] Proposal A Phase 1-4 Stack Chain 順序確認 #569

Open

feat(proposal-a): Phase 1 recall governance (Issue #569) #597

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Proposal A v3 - Dynamic Importance Feedback Signals (Phase 1)#507

feat: Proposal A v3 - Dynamic Importance Feedback Signals (Phase 1)#507
jlin53882 wants to merge 13 commits intoCortexReach:masterfrom
jlin53882:feat/proposal-a-v3-clean

jlin53882 commented Apr 4, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Apr 4, 2026

Uh oh!

chatgpt-codex-connector bot Apr 4, 2026

Uh oh!

AliceLJY left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		feedback: typeof cfg.feedback === "object" && cfg.feedback !== null
		? { ...(cfg.feedback as Record<string, unknown>) }

Conversation

jlin53882 commented Apr 4, 2026

Summary

What was built

Hooks

isRecallUsed() AND Logic

Per-Recall Scoring

Key Fixes (from #493 review)

Files changed

Related

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

AliceLJY left a comment

Choose a reason for hiding this comment

Branch topology problem

Code issues in Phase 1 (#507)

Phase 3 (#505)

Phase 4 (#506)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants