fix: auto-capture cumulative turn counting for smart extraction (issue #417)

[jlin53882]

Summary

Fixes the cumulative turn counting for smart extraction in auto-capture. Resolves issue #417.

Problem

With extractMinMessages: 2 + smartExtraction: true, single-turn DM conversations (1 user message per agent_end) always fall through to regex fallback, writing dirty data (l0_abstract == text, no LLM distillation).

Root cause: autoCaptureSeenTextCount was being overwritten per-event (always set to current event's message count = 1), never accumulating. Two fix paths identified:

• Path A: autoCaptureSeenTextCount diffing (fix: accumulate instead of overwrite)
• Path B: buildAutoCaptureConversationKeyFromIngress returned null for DM (no conversationId), so pendingIngressTexts was never written (fix: fallback to channel for DM)

Changes

Fix 1: buildAutoCaptureConversationKeyFromIngress (Path B)

DM: conversationId is undefined → returns channel instead of null. Now matches the key extracted by regex in agent_end.

Fix 2: autoCaptureSeenTextCount cumulative counting (Path A)

Changed from set(sessionKey, eligibleTexts.length) (overwrite) to set(sessionKey, currentCumulativeCount) (accumulate).

Fix 3: Smart extraction threshold uses cumulative count

currentCumulativeCount >= minMessages instead of cleanTexts.length >= minMessages.

Fix 4: isExplicitRememberCommand guard

Preserves explicit remember command behavior in DM context.

Fix 5: extractMinMessages cap

Math.min(config.extractMinMessages ?? 4, 100) — prevents misconfiguration.

Fix 6: MAX_MESSAGE_LENGTH = 5000 guard

Prevents OOM from super-long messages in pendingIngressTexts.

Semantic Change (Needs Discussion)

The fix changes extractMinMessages semantics from "per-event message count" to "cumulative conversation turns". This aligns with the design intent (AliceLJY confirmed this is the correct direction for beta.11), but existing users who configured extractMinMessages based on the old semantics may see different behavior.

Open Questions for Maintainer Review

Q1: extractMinMessages semantic change acceptable?

Before: extractMinMessages=N = each agent_end event needs N messages
After: extractMinMessages=N = smart extraction triggers at turn N

AliceLJY confirmed this is the correct direction. Is this semantic change acceptable as a bug fix, or should it be a separate config option?

Q2: isExplicitRememberCommand behavior after Fix 3

When pendingIngressTexts.length > 0 (multi-turn), texts.length is typically > 1, so texts.length === 1 && isExplicitRememberCommand(...) guard rarely triggers. The fix preserves this by checking lastPending fallback. Is this the intended behavior?

Q3: Window size for cumulative counting

The rolling window for pendingIngressTexts is -6 messages. For DM with extractMinMessages=2, this is sufficient. But if extractMinMessages is set to a very large value (e.g., 50), the window may not hold enough history. Should there be a dynamic adjustment?

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[rwmjhb]

Review: fix: auto-capture cumulative turn counting for smart extraction (#417)

问题定位准确——单轮 DM 因为 autoCaptureSeenTextCount 被每次覆写而永远触发 regex fallback，导致脏数据。但实现有几个需要关注的点：

Must Fix

1. 语义混淆: autoCaptureSeenTextCount 现在混合了 transcript length 和 cumulative-turn 两种语义，currentCumulativeCount 在整个 session 生命周期内单调递增无 reset——超过 turn N 之后每轮都会触发 smart extraction。

2. Build 失败: TypeScript 编译不过，需要先修。

Questions

• autoCapturePendingIngressTexts.delete(conversationKey) 被移除但 PR description 没说明——是有意为之吗？如果是，什么机制防止 stale pending texts 被重复消费？
• isExplicitRememberCommand guard 在 multi-turn DM（这个 PR 的目标场景）下不可达——是设计意图还是 bug？
• 新测试的 createMockApi 传了第 5 个参数但函数签名不接受。

[jlin53882]

Phase 2: Design Questions for Maintainer Input

Phase 1 cleanup (PR #534) is ready — fixes createMockApi interface and removes dead code.

Before Phase 2 (semantic fixes), we need maintainer input on these 5 questions:

---

Q1: Smart Extraction Re-trigger Policy

Issue: currentCumulativeCount is monotonic (no reset). Once it crosses extractMinMessages=N, every subsequent turn triggers smart extraction.

Current behavior (with Phase 1):

• Turn 1: cumulative=1, skip
• Turn 2: cumulative=2, trigger ✓
• Turn 3: cumulative=3, trigger again ⚠️
• Turn 4: cumulative=4, trigger again ⚠️

Is this the intended behavior? Should there be a session-level once-only flag?

---

Q2: REPLACE vs APPEND Strategy

PR #518 chose REPLACE (newTexts = pendingIngressTexts) to avoid deduplication issues when the same text appears in both pendingIngressTexts and eligibleTexts.

Alternative: Keep APPEND but deduplicate before joining.

Which strategy is preferred?

---

Q3: Stale pendingIngressTexts Prevention

Issue: autoCapturePendingIngressTexts.delete(conversationKey) was removed in PR #518, relying on REPLACE + slice(-6) to prevent stale data.

Concern: If agent_end fires without a preceding message_received (e.g., direct trigger, retry/replay), stale texts could be consumed.

Is there a mechanism to ensure message_received always precedes agent_end for a given conversationKey?

---

Q4: extractMinMessages Semantic Change

PR #518 changes semantics from "per-event message count" to "cumulative conversation turns".

This is a breaking change for users who configured extractMinMessages based on the old semantics.

Is this semantic change acceptable as a bug fix, or should it be a separate config option?

---

Q5: isExplicitRememberCommand Guard (Dead Code)

Phase 1 removes this guard as unreachable under REPLACE, but I want to confirm this is correct:

Original intent: Handle single "請記住..." messages without enough context, by appending priorRecentTexts.

Why unreachable now: Under REPLACE, texts = pendingIngressTexts (rolling window of up to 6 messages), so texts.length === 1 can never be true in multi-turn scenarios.

Should this behavior be preserved? If so, where should the context-enrichment logic live?

---

PR #534 (Phase 1): #534
PR #518 (main PR): #518

[jlin53882]

This PR has been superseded by PR #549, which resolves all blocking concerns from the PR #534 review (Must Fix #1, #2, #3, and Minor item #4) and additionally corrects a placement bug in Fix #1 identified during adversarial review.

Summary of fixes in #549:

• Fix [BUG] Ollama nomic-embed-text dimension mismatch (expected 768, got 192) #8: pendingIngressTexts.delete() — clears consumed pending texts
• Fix fix: respect embedding.dimensions for Ollama nomic-embed-text #9: currentCumulativeCount reset moved inside success block — resets only on successful extraction (not on failure), preventing re-trigger loops
• Fix CLI Commands 無效 #4: pluginConfigOverrides comment explaining embedding override behavior

Please review #549 instead.