Skip to content

feat: Add assistant messages to get_session_messages (closes #68)#70

Merged
evansenter merged 3 commits into
mainfrom
feat/issue-68-assistant-messages
Jan 10, 2026
Merged

feat: Add assistant messages to get_session_messages (closes #68)#70
evansenter merged 3 commits into
mainfrom
feat/issue-68-assistant-messages

Conversation

@evansenter

Copy link
Copy Markdown
Owner

Summary

  • Add unified message_text column for all entry types (user, assistant, tool_result, summary) with FTS indexing
  • Add entry_types parameter to filter by message type (default: ["user", "assistant"])
  • Add max_message_length parameter for truncation (default: 500, 0=no limit)
  • Update search_messages to search across all message types with optional filtering

Test plan

  • All 308 tests pass
  • Benchmark shows excellent performance (<1ms for get_session_messages, 8ms for search_messages)
  • CLI commands tested with new flags (--entry-types, --max-length)
  • Re-ingested all session logs to populate message_text column

🤖 Generated with Claude Code

Add unified `message_text` column for all entry types (user, assistant,
tool_result, summary) with FTS indexing across all message types.

Key changes:
- Add migration v8 for message_text column and rebuilt FTS index
- Update ingestion to extract text from assistant, tool_result, summary entries
- Add entry_types parameter to filter by message type (default: user, assistant)
- Add max_message_length parameter for truncation (default: 500, 0=no limit)
- Update search_messages to search across all message types
- Update CLI with --entry-types and --max-length flags
- Add 5 new tests for the new functionality

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude

claude Bot commented Jan 10, 2026

Copy link
Copy Markdown

Prompt: evansenter/dotfiles/.../claude-review.md

Code Review

Summary

This PR adds assistant message support to get_session_messages and search_messages by introducing a unified message_text column that captures text from all entry types (user, assistant, tool_result, summary). It includes a schema migration, FTS index rebuild, new filtering parameters (entry_types, max_message_length), and comprehensive updates to CLI/MCP/documentation.

Issues Found

Critical

None

Important

  • src/session_analytics/ingest.py:25-87 - New public functions extract_text_from_content() and extract_tool_result_content() lack direct unit tests. These functions have multiple branches (handling strings, lists, dicts, image placeholders) that should be tested to ensure correct extraction behavior. While they're exercised indirectly through ingestion, edge cases could be missed.

Suggestions

  • src/session_analytics/ingest.py:22 - MESSAGE_TEXT_MAX_LENGTH = None is defined but never used. Consider either using it as a configurable limit or removing the unused constant to avoid confusion.
  • src/session_analytics/guide.md:79-82 - The documentation mentions entry_types defaults but doesn't specify that search_messages defaults to searching all types (no filter) while get_session_messages defaults to ["user", "assistant"]. This asymmetry could be made more explicit.
  • src/session_analytics/queries.py:730-731 - When max_message_length=0, the truncation is skipped, but the condition if message_text and max_message_length > 0 could be clearer with a comment or by using a named constant like NO_TRUNCATION = 0.

Verdict

REQUEST_CHANGES - Important issue with missing tests for new extraction functions, plus minor suggestions for code clarity.


Automated review by Claude Code

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude

claude Bot commented Jan 10, 2026

Copy link
Copy Markdown

Prompt: evansenter/dotfiles/.../claude-review.md

Code Review

Summary

This PR adds assistant, tool_result, and summary message support to get_session_messages and search_messages by introducing a unified message_text column. It includes schema migration v8, FTS index rebuild, new filtering parameters (entry_types, max_message_length), and comprehensive updates across CLI/MCP/documentation.

Issues Found

Critical

None

Important

  • src/session_analytics/ingest.py:25-87 - New public functions extract_text_from_content() and extract_tool_result_content() lack direct unit tests in test_ingest.py. These functions handle multiple content formats (strings, lists, dicts with various types, image placeholders) with non-trivial branching logic. While exercised indirectly through integration tests, dedicated unit tests would catch edge cases like empty lists returning None, mixed string/dict lists, or malformed content structures. Consider adding targeted tests to test_ingest.py.

Suggestions

  • src/session_analytics/ingest.py:22 - MESSAGE_TEXT_MAX_LENGTH = None is defined but never used anywhere in the code. Either apply it as a configurable limit in the extraction functions or remove the unused constant.
  • src/session_analytics/guide.md:79-82 - The documentation states entry_types default for get_session_messages is ["user", "assistant"] but does not explicitly clarify that search_messages defaults to all types (no filter). This asymmetry could confuse users; consider adding a note like "search_messages: Default: all types (no filter)".

Verdict

REQUEST_CHANGES - Important issue with missing dedicated unit tests for new extraction functions, plus minor suggestions for code cleanup and documentation clarity.


Automated review by Claude Code

- Add 21 unit tests for extract_text_from_content() and
  extract_tool_result_content() covering edge cases (empty lists,
  mixed content, image placeholders, etc.)
- Remove unused MESSAGE_TEXT_MAX_LENGTH constant from ingest.py
- Clarify entry_types defaults in guide.md: get_session_messages
  defaults to ["user", "assistant"], search_messages defaults to
  all types (no filter)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@evansenter

Copy link
Copy Markdown
Owner Author

Feedback Addressed

Implemented

  • [Important] ingest.py:25-87 - Added 21 unit tests for extract_text_from_content() and extract_tool_result_content() covering edge cases: empty lists, mixed string/dict content, image placeholders, tool blocks, malformed structures
  • [Suggestion] ingest.py:22 - Removed unused MESSAGE_TEXT_MAX_LENGTH constant
  • [Suggestion] guide.md:79-82 - Clarified that get_session_messages defaults to ["user", "assistant"] while search_messages defaults to all types (no filter)

Skipped

  • [Suggestion] queries.py:730-731 - The max_message_length=0 condition is self-documenting; adding a constant like NO_TRUNCATION = 0 would be over-engineering for a standard pattern

@claude

claude Bot commented Jan 10, 2026

Copy link
Copy Markdown

Prompt: evansenter/dotfiles/.../claude-review.md

Code Review

Summary

This PR adds assistant, tool_result, and summary message support to get_session_messages and search_messages by introducing a unified message_text column. It includes schema migration v8, FTS index rebuild, new filtering parameters (entry_types, max_message_length), and comprehensive updates across CLI/MCP/documentation.

Previous Feedback Status

All previously raised items have been addressed:

  • ✅ Unit tests added for extract_text_from_content() and extract_tool_result_content() (21 tests)
  • ✅ Unused MESSAGE_TEXT_MAX_LENGTH constant removed
  • ✅ Documentation clarified for entry_types defaults

Issues Found

Critical

None

Important

None

Suggestions

None

Verdict

APPROVE - Previous feedback has been addressed. The implementation is solid with comprehensive test coverage, safe migration preserving existing data, and proper backward compatibility via the search_user_messages wrapper.


Automated review by Claude Code

@evansenter evansenter merged commit 3d6bfee into main Jan 10, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant