Skip to content

stream chat input#1212

Open
Zibbp wants to merge 1 commit into
mainfrom
chat-update-memory-fix
Open

stream chat input#1212
Zibbp wants to merge 1 commit into
mainfrom
chat-update-memory-fix

Conversation

@Zibbp

@Zibbp Zibbp commented Jun 14, 2026

Copy link
Copy Markdown
Owner

No description provided.

@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

ConvertTwitchLiveChatToTDLChat is rewritten to stream-convert Twitch live chat JSON incrementally using os.Open, os.Create, a buffered writer, and a json.Decoder/json.Encoder pair instead of building a full in-memory document. Comment conversion logic is extracted into a new convertLiveCommentToTDLComment helper. A new unit test validates the full pipeline output.

Changes

Streaming Twitch-to-TDL Conversion

Layer / File(s) Summary
Streaming I/O setup and JSON scaffolding
internal/utils/tdl.go
Adds bufio import, switches input handling from a loader call to os.Open, creates the output file with os.Create, sets up a buffered writer with deferred flush, and emits the initial JSON streamer field and start-of-comments structure.
Token-by-token decode loop and output finalization
internal/utils/tdl.go
Incrementally decodes the input JSON array via json.Decoder tokens, calls convertLiveCommentToTDLComment per comment, writes comma separators between encoded comments, validates the closing array token, computes tdlChat.Video.End from the last offset, and writes the closing JSON fields.
convertLiveCommentToTDLComment helper
internal/utils/tdl.go
Introduces convertLiveCommentToTDLComment, which encapsulates per-comment conversion: timestamp-to-offset calculation, highlighted message param injection, fragment and emote parsing with bounds validation and mismatch recovery via findSubstringPositions, fragment sorting and tail assembly, badge mapping, and default user color assignment.
End-to-end conversion test
internal/utils/tdl_test.go
Adds TestConvertTwitchLiveChatToTDLChatStreamsValidOutput, which writes fixture input chat JSON (including an emote-highlighted comment) to a temp file, runs the conversion, unmarshals the output TDLChat, and asserts streamer identity, video end time, comment count, offset, user-notice params, and emote fragment structure.
🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (1 warning, 2 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'stream chat input' is vague and does not clearly convey the main change, which is refactoring chat conversion to use streaming/incremental processing instead of loading the entire document. Use a more descriptive title that captures the primary change, such as 'Refactor chat conversion to stream input' or 'Use incremental processing for Twitch chat conversion'.
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess whether a description exists and relates to the changeset. Add a pull request description explaining the motivation, approach, and benefits of the refactoring to help reviewers understand the change.
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chat-update-memory-fix

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread internal/utils/tdl.go
log.Debug().Str("chat_file", path).Msg("Converting live Twitch chat to TDL chat for rendering")

liveComments, err := OpenLiveChatFile(path)
liveChatJsonFile, err := os.Open(path)
Comment thread internal/utils/tdl.go
tdlChat.Video.Start = 0

tdlComments := []Comment{}
outFile, err := os.Create(outPath)

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
internal/utils/tdl.go (1)

278-278: 💤 Low value

Non-idiomatic variable naming.

message_is_offset uses snake_case; Go convention prefers camelCase (messageIsOffset).

🤖 Prompt for 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.

In `@internal/utils/tdl.go` at line 278, The variable name `message_is_offset`
uses snake_case which is not idiomatic Go. Rename this variable to use camelCase
following Go conventions by changing it to `messageIsOffset` throughout the code
wherever it appears.
🤖 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 `@internal/utils/tdl.go`:
- Line 380: The slice operation on
Body[emoteFragments[i-1].Pos2:emoteFragment.Pos1] can panic if the end position
of the previous emote fragment (emoteFragments[i-1].Pos2) is greater than the
start position of the current emote fragment (emoteFragment.Pos1), resulting in
a negative slice length. Add a bounds check before this slicing operation to
verify that emoteFragments[i-1].Pos2 is less than or equal to
emoteFragment.Pos1. If this condition is false (indicating overlapping emotes),
skip processing that fragment iteration or handle the overlap appropriately
based on the intended behavior.

---

Nitpick comments:
In `@internal/utils/tdl.go`:
- Line 278: The variable name `message_is_offset` uses snake_case which is not
idiomatic Go. Rename this variable to use camelCase following Go conventions by
changing it to `messageIsOffset` throughout the code wherever it appears.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cbcb3f37-68fc-4edf-9f79-afc680f0a26e

📥 Commits

Reviewing files that changed from the base of the PR and between 9b9ee05 and 93fc277.

📒 Files selected for processing (2)
  • internal/utils/tdl.go
  • internal/utils/tdl_test.go

Comment thread internal/utils/tdl.go
log.Warn().Str("message_id", liveComment.MessageID).Msg("skipping invalid emote position")
continue
}
fragmentText := tdlComment.Message.Body[emoteFragments[i-1].Pos2:emoteFragment.Pos1]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Potential panic if emotes overlap.

If two emotes have overlapping positions (e.g., emote1 ends at pos 5, emote2 starts at pos 3), slicing Body[emoteFragments[i-1].Pos2:emoteFragment.Pos1] with negative length will panic. Consider adding a bounds check or skipping overlapping emotes.

Suggested defensive guard
 		} else {
 			if emoteFragment.Pos1 == 0 {
 				log.Warn().Str("message_id", liveComment.MessageID).Msg("skipping invalid emote position")
 				continue
 			}
+			if emoteFragments[i-1].Pos2 > emoteFragment.Pos1 {
+				log.Warn().Str("message_id", liveComment.MessageID).Msg("overlapping emote positions, skipping")
+				continue
+			}
 			fragmentText := tdlComment.Message.Body[emoteFragments[i-1].Pos2:emoteFragment.Pos1]
🤖 Prompt for 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.

In `@internal/utils/tdl.go` at line 380, The slice operation on
Body[emoteFragments[i-1].Pos2:emoteFragment.Pos1] can panic if the end position
of the previous emote fragment (emoteFragments[i-1].Pos2) is greater than the
start position of the current emote fragment (emoteFragment.Pos1), resulting in
a negative slice length. Add a bounds check before this slicing operation to
verify that emoteFragments[i-1].Pos2 is less than or equal to
emoteFragment.Pos1. If this condition is false (indicating overlapping emotes),
skip processing that fragment iteration or handle the overlap appropriately
based on the intended behavior.

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.

2 participants