Repository navigation
refactor(conversation-store): drop triple-encoded sessions JSON + duplicate events (closes #1052) - #1054
Merged
Conversation
This was referenced May 23, 2026
Closed
…licate events (closes #1052) Builds on #1051. Three storage paths in conversation.sqlite were triple- encoding the same atoms — per #1052's table. This PR collapses them so surface tables (voice / phone / discord_voice) and session_events become the single sources of truth; the sessions table keeps only the per- session rollup metadata. Schema migration (idempotent, init-time): - ALTER TABLE sessions DROP COLUMN tool_calls (now in surface tables as kind='tool_call' rows, written real-time per #1051) - ALTER TABLE sessions DROP COLUMN events (now in session_events as lifecycle-only rows, see below) - Guarded via pragma_table_info — no-op on re-run. Writer-side (3 surface servers + store): - recordSession() INSERT drops the 2 JSON columns (13→11 cols) - recordSession()'s session_events fan-out now skips user:/caller:/ sutando:/assistant:/tool_call:/tool_result: prefixes (defense-in-depth filter for any external caller still passing them in m.events) - voice-agent.ts removes 3 voiceEvents.push: tool_call: (onToolCall), tool_result: (onToolResult), and the user:/sutando: utterance loop in the turn-handler. logConversation() + recordToolCall() are the canonical writes for those. - conversation-server.ts (phone) removes the same 4: tool_call:, tool_result:, caller:, sutando: - discord-voice-server.ts removes the same 4: tool_call:, tool_result:, user:, sutando: - Lifecycle pushes (session_started, session_ended, error, task_*, channel_result, rec_indicator, call_ended) are preserved. Reader-side: - diagnose-call.py (skills/regression-search) rebuilds the tool_calls list from surface tables (UNION ALL 3 with kind='tool_call' filter by session_id) and the events list from session_events directly. The output shape is unchanged. SessionMetrics interface kept toolCalls + events fields for backwards compat with existing callers — they're now silently ignored (toolCalls) or filtered (events) instead of persisted as JSON blobs. Verified end-to-end: - Migration applied to live workspace db; sessions now has 11 columns; pre-existing data preserved. - All 3 surface servers restart cleanly under new code. - tsc --noEmit clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
liususan091219
force-pushed
the
feat/conversation-store-drop-redundancy
branch
from
May 23, 2026 13:43
ff625b8 to
c7b314e
Compare
sonichi
approved these changes
May 23, 2026
sonichi
left a comment
Owner
There was a problem hiding this comment.
LGTM — clean redundancy cleanup after #1051's per-surface tables landed. Per #1052's analysis:
- Drops the triple-encoded
sessions.eventsJSON column + duplicatesession_eventsrows (event_name='user:<text>'/'sutando:<text>'/'tool_call:<name>') that the #1051 per-surface tables made redundant. - Single source of truth post-merge: per-surface row in
voice/phone/discord_voicetables.session_eventskeeps only system events (SESSION_ENDetc.), not utterance/tool-call duplicates. - Now-clean diff after Maddy's rebase: 1 commit (
c7b314e), +130/-49 — visible in the PR diff. Rebase resolved the stacking-on-#1051 conflict.
Per Maddy at 13:51Z. Ready for merge.
liususan091219
added a commit
to liususan091219/sutando-fork
that referenced
this pull request
May 28, 2026
…licate events (closes sonichi#1052) (sonichi#1054) Builds on sonichi#1051. Three storage paths in conversation.sqlite were triple- encoding the same atoms — per sonichi#1052's table. This PR collapses them so surface tables (voice / phone / discord_voice) and session_events become the single sources of truth; the sessions table keeps only the per- session rollup metadata. Schema migration (idempotent, init-time): - ALTER TABLE sessions DROP COLUMN tool_calls (now in surface tables as kind='tool_call' rows, written real-time per sonichi#1051) - ALTER TABLE sessions DROP COLUMN events (now in session_events as lifecycle-only rows, see below) - Guarded via pragma_table_info — no-op on re-run. Writer-side (3 surface servers + store): - recordSession() INSERT drops the 2 JSON columns (13→11 cols) - recordSession()'s session_events fan-out now skips user:/caller:/ sutando:/assistant:/tool_call:/tool_result: prefixes (defense-in-depth filter for any external caller still passing them in m.events) - voice-agent.ts removes 3 voiceEvents.push: tool_call: (onToolCall), tool_result: (onToolResult), and the user:/sutando: utterance loop in the turn-handler. logConversation() + recordToolCall() are the canonical writes for those. - conversation-server.ts (phone) removes the same 4: tool_call:, tool_result:, caller:, sutando: - discord-voice-server.ts removes the same 4: tool_call:, tool_result:, user:, sutando: - Lifecycle pushes (session_started, session_ended, error, task_*, channel_result, rec_indicator, call_ended) are preserved. Reader-side: - diagnose-call.py (skills/regression-search) rebuilds the tool_calls list from surface tables (UNION ALL 3 with kind='tool_call' filter by session_id) and the events list from session_events directly. The output shape is unchanged. SessionMetrics interface kept toolCalls + events fields for backwards compat with existing callers — they're now silently ignored (toolCalls) or filtered (events) instead of persisted as JSON blobs. Verified end-to-end: - Migration applied to live workspace db; sessions now has 11 columns; pre-existing data preserved. - All 3 surface servers restart cleanly under new code. - tsc --noEmit clean. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1052. Stacks on top of #1051 — the diff currently includes #1051's commits; once that merges this PR rebases to the clean redundancy-cleanup-only diff.
Why
Per the table in #1052, three storage paths encode the same atoms:
voice/phone/discord_voicerow (per-utt)session_eventsevent_name='user:<text>'/'sutando:<text>'sessions.eventsJSONkind='tool_call'(realtime, #1051)session_eventsevent_name='tool_call:<name>'/'tool_result:...'sessions.tool_callsJSONRow-count proof on the live workspace db pre-this-PR:
voice3673 rows butsession_events20027 rows (the pre-#1051v_voicequery reading 'user:'/'sutando:'/'tool_call:' prefixes returned ~10574 rows — 3× the surface table for the same data). Cost: more disk, more confused readers (you can SELECT from any of 3 places and get the same number).What
Single source of truth per atom:
voice/phone/discord_voiceonly.kind='tool_call', written real-time viarecordToolCallfrom refactor(conversation-store): per-surface tables + realtime tool_call writes #1051).session_eventsonly.sessionskeeps just the per-session rollup metadata (duration, transcript_lines, tool_count, etc.) — no more JSON columns.Schema change
Guarded via
PRAGMA table_infoso re-init is a no-op. SQLite 3.35+ required (node:sqlite ships with newer than that).Writer-side changes
src/conversation-store.tstool_calls+eventscols in INSERT; session_events fan-out filter for user:/caller:/sutando:/assistant:/tool_call:/tool_result: prefixessrc/voice-agent.tstool_call:,tool_result:,${evtRole}:${content}pushesskills/phone-conversation/scripts/conversation-server.tstool_call:,tool_result:,caller:,sutando:pushesskills/discord-voice/scripts/discord-voice-server.tstool_call:,tool_result:,user:,sutando:pushesLifecycle pushes (
session_started,session_ended,error:,task_delegated:,task_result:,channel_result:,rec_indicator:,call_ended) preserved across all 3 surfaces.Reader-side changes
skills/regression-search/scripts/diagnose-call.py:_build_transcriptalready updated in refactor(conversation-store): per-surface tables + realtime tool_call writes #1051 to read surface tables (UNION ALL+kind != 'tool_call'filter).skills/regression-search/scripts/diagnose-call.py:find_metricsupdated here:UNION ALL+kind = 'tool_call' WHERE session_id = ?)session_events WHERE session_id = ? OR call_sid = ?{name, durationMs, timestamp}per tool call;{event, timestamp}per event) unchanged.SessionMetricsinterface keepstoolCalls+eventsfields for backwards compat with existing callers — silently ignored / filtered now instead of persisted.Verification
diagnose-call.pysyntax check + structural rewriteRisks
sessions.tool_callsorsessions.eventsJSON outside this repo will break post-merge. Mitigation: anyone affected can rebuild the same data via the surface tables (for tool_calls) or session_events (for events). Migration log line prints when columns are dropped so the operator notices.🤖 Generated with Claude Code