[MM-69856] Append Zoom chat continuation lines to prior message - #927
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughZoom chat subtitle parsing now preserves non-timestamp continuation lines by appending them to the latest subtitle item. Tests cover timestamp formats, continuation handling, empty input, and scanner errors. ChangesZoom chat subtitle parsing
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
🤖 LLM Evaluation ResultsOpenAI
❌ Failed EvaluationsShow 7 failuresOPENAI1. TestReactEval/[openai]_react_cat_message
2. TestConversationMentionHandling/[openai]_conversation_from_attribution_long_thread.json
3. TestConversationMentionHandling/[openai]_conversation_from_attribution_long_thread.json
4. TestConversationMentionHandling/[openai]_conversation_from_attribution_long_thread.json
5. TestConversationMentionHandling/[openai]_conversation_from_attribution_long_thread.json
6. TestConversationMentionHandling/[openai]_conversation_from_attribution_long_thread.json
7. TestDirectMessageConversations/[openai]_bot_dm_tool_introspection
Anthropic
❌ Failed EvaluationsShow 7 failuresANTHROPIC1. TestReactEval/[anthropic]_react_cat_message
2. TestConversationMentionHandling/[anthropic]_conversation_from_attribution_long_thread.json
3. TestConversationMentionHandling/[anthropic]_conversation_from_attribution_long_thread.json
4. TestConversationMentionHandling/[anthropic]_conversation_from_attribution_long_thread.json
5. TestConversationMentionHandling/[anthropic]_conversation_from_attribution_long_thread.json
6. TestConversationMentionHandling/[anthropic]_conversation_from_attribution_long_thread.json
7. TestDirectMessageConversations/[anthropic]_bot_dm_tool_introspection
This comment was automatically generated by the eval CI pipeline. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@subtitles/subtitles.go`:
- Around line 27-48: Update the subtitle parsing flow around the scanner loop to
check scanner.Err() after scanning completes. If an error is present, propagate
it instead of returning partial storage with a nil error; otherwise preserve the
existing storage return behavior.
- Around line 31-36: In the timestamp parsing branch, require the character at
line[8] to be a space before creating the astisub.Item; update the condition
around time.Parse so malformed separators such as “X” are rejected while valid
timestamp lines continue using line[9:] for text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: d142c68b-bcf9-492f-a930-6de9d3bbd5e3
📒 Files selected for processing (2)
subtitles/subtitles.gosubtitles/subtitles_test.go
|
@jgheithcock @ogi-m merging this to get it into release cut FYI |
* [MM-69856] Safely parse short Zoom chat lines during summarization (#906) * webapp: polish Agents list and agent configuration form UX (#887) * webapp: polish Agents list and agent configuration form UX Co-authored-by: Cursor <cursoragent@cursor.com> * webapp: address PR review feedback and fix lint/i18n drift Co-authored-by: Asaad Mahmood <asaadmahmood@users.noreply.github.com> * e2e: update agent provider config tests for advanced section UX Co-authored-by: Asaad Mahmood <asaadmahmood@users.noreply.github.com> * e2e: fix Reasoning Effort select locator for nested form layout Co-authored-by: Asaad Mahmood <asaadmahmood@users.noreply.github.com> * e2e: expand advanced section before toggling Enable Tools in CRUD test Co-authored-by: Asaad Mahmood <asaadmahmood@users.noreply.github.com> * e2e: rename agent page helper and provider config spec to snake_case Co-authored-by: Asaad Mahmood <asaadmahmood@users.noreply.github.com> * e2e: revert snake_case renames to keep PR diff focused Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> * Exempt embedded Mattermost MCP tools from the MCP license gate (#924) * Exempt embedded Mattermost MCP tools from the MCP license gate Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * webapp: mark remote MCP servers as a licensed feature in the system console Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Gate remote MCP auto-execution on license and base approval gate on acceptance Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Enforce remote MCP licensing at tool supply time and hide remote server config UI when unlicensed Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * Address review feedback: plan-neutral chip title, shared license-mock helpers Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> * [MM-69973] Validate the mention reminder's target post id before building the loop-in request (#931) * [MM-69973] Harden post id handling in the agent mention reminder Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * [MM-69973] Harden webapp handling of values read from post props Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * [MM-69973] Validate and bound search source props in the webapp Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * [MM-69973] Add e2e coverage for the agent mention reminder loop-in link Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * webapp: use a transient styled-components prop for the sources toggle Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * webapp: use unique post ids in the search source cap test fixture Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * [MM-69973] Simplify the props validation changes and trim test duplication Co-authored-by: nick.misasi <nick.misasi@mattermost.com> * chore: retrigger CodeRabbit review after feedback triage Co-authored-by: mattermost-code <matty-code@mattermost.com> * MM-69973: Trim scope to the mention reminder post id validation Co-authored-by: nick.misasi <nick.misasi@mattermost.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: mattermost-code <matty-code@mattermost.com> * [MM-69856] Append Zoom chat continuation lines to prior message (#927) * [MM-69856] Append Zoom chat continuation lines to prior message * Address review: require timestamp separator and propagate scanner errors * fix: guard nil map in web search context to prevent plugin crash (#935) Unmarshaling the literal JSON "null" from the web_search_context post prop succeeds and leaves the target map nil. The unconditional writes that followed panicked with "assignment to entry in nil map", crashing the plugin process and taking all AI routes offline. Any authenticated user could trigger this by DMing a bot with props.web_search_context set to "null". Guard against a nil map after unmarshaling and add a table-driven test covering null, empty-object, valid, and invalid inputs. Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> --------- Co-authored-by: JG Heithcock <jgheithcock@gmail.com> Co-authored-by: Asaad Mahmood <asaadmahmood@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Christopher Speller <crspeller@gmail.com> Co-authored-by: Christopher Speller <crspeller@users.noreply.github.com> Co-authored-by: mattermost-code <matty-code@mattermost.com>
Summary
Follow-up to #906: treat a valid
HH:MM:SStimestamp as the discriminator for Zoom chat lines. Non-timestamp lines (short or long) append to the previous message instead of being skipped or failing the entire parse.Ticket Link
https://mattermost.atlassian.net/browse/MM-69856
Release Note
Summary by CodeRabbit
HH:MM:SS <text>lines, with consistent subtitle durations.