feat(tui): opt-in auto-attach Long-Term Memory to chats before send

[manuel-goepfi]

Summary

Chat-LTM lives on the conversation record server-side as a list of associated ranges. New chats start with an empty range list, so even when system LTM (the workstream pattern engine) is recording, the model receives no LTM-derived context until the user manually presses Ctrl+L on every conversation. Users who always want LTM context end up performing a per-chat ritual.

This adds two opt-in CLI preferences:

Field	Type	Default	Purpose
auto_enable_chat_ltm	bool	false	Master switch — gates the entire auto-attach behavior so privacy-sensitive users see no change.
auto_enable_chat_ltm_lookback_days	int	7	Width of the LTM range. The SDK's BasicRange.create() defaults to a 15-minute window — far too narrow for "long-term" memory. 7 days covers a typical work week. Setting 0 falls back to the SDK 15-minute default.

Why hook in ask_question (not on chat creation)

The activation hook lives in CopilotController.ask_question rather than at chat-creation time so it covers all three chat origin paths in a single call site:

1. Ctrl+N then send — chat exists with no ranges → activate, send.
2. Click an existing chat with no ranges then send — activate, send.
3. Type-and-send with no active chat — the helper itself creates the chat and attaches the range; stream_question then targets that chat.

A symmetric hook on ChatController.switch_chat(None) handles the explicit Ctrl+N path so the 🧠 indicator updates as soon as the new chat is created, not just when the first message goes out.

Why the lookback field exists

Without it, the auto-attached range covers only the last 15 minutes (the SDK default in BasicRange.create()). The model would have access to LTM data only from immediately before the message — not the "long-term" memory the feature implies. Repeated activations create multiple narrow windows that aggregate to only the periods the user happened to be active when toggling, leaving older work invisible.

The lookback field widens the range to the configured number of days at activation time, so a single auto-activation gives the chat genuinely useful coverage. 0 is preserved as the SDK-default escape hatch.

Activation helper

Both controllers route through a shared helper _activate_chat_ltm_with_lookback that mirrors the SDK's ltm.chat_enable_ltm() — creating a chat if needed, refreshing the local conversation cache afterwards — but builds the range with the configured lookback. Keeping the cache-refresh behavior preserves callers' immediate visibility into conversation.ranges.

UI sync

PiecesCopilot.on_chat_messages_switched now refreshes the status-bar LTM indicator on every switch, so the visible 🧠 state stays in sync with the per-chat flag after auto-activation.

Idempotency

Both hooks early-return when the chat already has ranges (is_chat_ltm_enabled), keeping repeated sends idempotent — no piling-on of redundant ranges across multiple messages in the same chat.

Failure handling

API failures are logged via Settings.logger and swallowed — a transient hiccup must not break either chat creation or the user's send.

Test plan

• With auto_enable_chat_ltm: true and auto_enable_chat_ltm_lookback_days: 7, sent first message in three chat origin paths (Ctrl+N, click existing, type-and-send-with-no-chat); LTM context arrived in the first response and pulled in activity from the last week.
• With auto_enable_chat_ltm: false (default), no behavior change vs current main.
• Repeated sends in the same chat do not add duplicate ranges (idempotent guard).
• When system LTM permissions are not granted, the auto-enable silently no-ops (does not surface a permission prompt mid-send).
• Setting auto_enable_chat_ltm_lookback_days: 0 falls back to the SDK 15-minute default.

[mark-at-pieces]

Detailed Review

Overall this is a well-designed, privacy-conscious feature with solid opt-in defaults, idempotency guards, and graceful error handling. The PR description is excellent. However, there are several issues — a real bug, race conditions, missing tests, and missing docs — that should be addressed before merge.

---

🐛 Bug: Orphan Chat Creation in _activate_chat_ltm_with_lookback()

Severity: Medium

chat = Settings.pieces_client.copilot.chat
if not chat:
    chat = Settings.pieces_client.copilot.create_chat("New Conversation")

When called from CopilotController.ask_question (where copilot.chat may be None), this creates a chat but never assigns it back to copilot.chat. Then stream_question may create another chat. The range gets attached to the orphaned first chat, not the one the message actually goes to.

Fix: Either set Settings.pieces_client.copilot.chat = chat after creation, or (better) remove the chat-creation side-effect from the helper entirely and require callers to ensure a chat exists before invoking it.

---

🏁 Race Condition: Unsynchronized Write to ConversationsSnapshot.identifiers_snapshot

Severity: Medium-High

ConversationsSnapshot.identifiers_snapshot[conv.id] = conv

This bypasses StreamedIdentifiersCache._lock. The WebSocket worker thread also writes to this dict under the lock. While CPython's GIL makes a single dict.__setitem__ atomic, there's a lost-update race: the worker could overwrite this fresher value with a stale one (or vice versa) because they don't coordinate.

Fix: Use ConversationsSnapshot's own update_identifier() method (which acquires _lock), or at minimum:

with ConversationsSnapshot._lock:
    ConversationsSnapshot.identifiers_snapshot[conv.id] = conv

---

🏁 Race Condition: Blocking HTTP Calls in the Send Path

Severity: Medium

CopilotController._maybe_auto_enable_chat_ltm() makes two synchronous HTTP calls (vision status + associate_range) that block every first-send. During this window, concurrent actions (chat switch, manual Ctrl+L toggle, WebSocket updates) can change the state the precondition checks relied upon.

Suggestion: Consider caching the vision-status result for a short TTL, or at minimum document that the pre-send hook adds latency on first message per chat.

---

🔀 DRY Violation: Two Divergent _maybe_auto_enable_chat_ltm Implementations

Severity: Medium

ChatController._maybe_auto_enable_chat_ltm() uses self.is_ltm_running() (calls update_ltm_cache() then reads ltm.is_enabled).

CopilotController._maybe_auto_enable_chat_ltm() does the same thing inline but directly via:

ltm.ltm_status = Settings.pieces_client.work_stream_pattern_engine_api.workstream_pattern_engine_processors_vision_status()
if not ltm.is_enabled:
    return

If update_ltm_cache() is ever enhanced (e.g., checks multiple processors), the CopilotController path won't pick up the change.

Fix: Have CopilotController delegate to the same is_ltm_running() / is_chat_ltm_enabled() methods, or extract the precondition check into _activate_chat_ltm_with_lookback() so both call sites are one-liners.

---

📏 Edge Case: No Upper Bound on lookback_days

Severity: Low

ge=0 prevents negatives, but a user could set auto_enable_chat_ltm_lookback_days: 99999 — creating a range spanning 273 years. The LTM engine only retains ~9 months of data.

Fix: Add le=270 (or similar) to match the engine's actual retention window.

---

📏 Edge Case: Implicitly-Created Chats Miss LTM on First Message

Severity: Low

When a user types and sends without pressing Ctrl+N first, the ask_question hook fires while copilot.chat is None — leading to the orphan-chat bug above. The fallback hook in _on_stream_message only fires after the first response, so the first message never gets LTM context in this path.

Resolving the orphan-chat bug (#1) would fix this too.

---

🧪 Tests: None

Severity: High

There are zero tests for any of the new logic. The precondition matrix is complex enough (opt-in flag × LTM running × already enabled × error paths × multiple chat origins) that manual testing alone is insufficient.

Needed:

• Unit tests for _activate_chat_ltm_with_lookback(): days > 0 vs days == 0 vs no active chat; cache refresh behavior.
• Unit tests for both _maybe_auto_enable_chat_ltm() methods: precondition early-returns, exception swallowing, idempotency.
• Integration tests for switch_chat(None): fresh chat → auto-enable fires; existing chat → does not fire.
• Integration tests for ask_question(): auto-enable fires before stream_question; already-enabled → no extra API calls.
• Config schema tests: negative value rejection, default values, round-trip serialization.

---

📚 Documentation: Not Updated

Severity: Medium

The PR adds two new user-facing config fields but updates no documentation:

• CHANGELOG — needs an entry for this feature.
• README — if it has a configuration section, the new fields should be listed with examples.
• CLI discoverability — without a pieces config set auto_enable_chat_ltm true command (or at least mentioning the fields in pieces config --help), users have no way to discover these settings exist.

---

💧 Memory (Pre-Existing, Amplified)

Severity: Low / Informational

RangeSnapshot.identifiers_snapshot grows with every BasicRange.create() and is never pruned. Auto-enable amplifies this by creating ranges the user never explicitly requested. Not a blocker, but worth flagging to the SDK team for long-running sessions.

---

✅ What's Good

• Privacy-first default=False — correct call.
• Idempotency guard (is_chat_ltm_enabled early-return) prevents redundant ranges.
• Error swallowing ensures API hiccups never break chat creation or sends.
• Lookback field with 0 escape hatch is a well-thought-out API.
• Status bar refresh on chat switch is a good UX detail.
• PR description is thorough and clearly explains the three chat origin paths.

---

Summary

Priority	Action Item
High	Add tests (precondition matrix, error paths, idempotency)
Medium	Fix orphan-chat-creation bug in _activate_chat_ltm_with_lookback
Medium	Synchronize identifiers_snapshot write with _lock
Medium	Deduplicate the two _maybe_auto_enable_chat_ltm methods
Medium	Add CHANGELOG entry and docs
Low	Add upper bound on lookback_days
Low	Consider caching or async for the pre-send HTTP calls

Happy to help iterate on any of these. Nice feature — looking forward to seeing it land! 🚀