Repository navigation
Conversation
Bounding lineMarkerStart at the previous label also made "only whitespace since the previous label" count as the start of a line, so a short turn after a label was read as the next label's list marker and dropped with it. #71 fixed that for numbers (`Alice: 42. Bob:`), but punctuation-only turns were still lost: `Alice: ... Bob: Yes?`, `Alice: — Bob: Sorry, go on.` and `Alice: ?! Bob: What?` each sent only Bob's turn. Judge the line start on the whole text for every marker. The scan still stops at the previous label, so emoji names (`😀: 🤖:`) don't panic. The live API accepts annotated turns of only `...`, `—` or `-` (HTTP 200).
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request simplifies the logic in lineMarkerStart within internal/cli/custom/tts.go by removing the redundant check variable and directly evaluating the line start using the full text context. It also updates TestSpeakerTurns in internal/cli/custom/custom_test.go to include test cases verifying that punctuation-only turns following a label are correctly preserved. There are no review comments, so I have no feedback to provide.
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
1 participant
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.
Follow-up to #71. A speaker turn that contains only punctuation was dropped from the request when it came right after its label on the same line:
main(1f78c8a)Alice: ... Bob: Yes?..., BobYes?Alice: — Bob: Sorry, go on.—, BobAlice: ?! Bob: What??!, BobAlice: - Bob: yo.-, BobCause:
lineMarkerStartstops scanning at the end of the previous label (floor), which is what keeps emoji speaker names (😀: 🤖:) from panicking. The side effect was that "only whitespace since the previous label" also counted as the start of a line. A turn right after a label was then taken for the next label's list marker and dropped with it. #71 fixed this for numbers only (Alice: 42. Bob:).Fix: the scan still stops at
floor, but the start-of-line check now looks at the whole text for every kind of marker. This also removes the number-only special case.Verification:
...,—or-(HTTP 200 ongemini-3.8-flash-tts).go test ./...passes.42., numbered and bulleted lines, emoji names andश्रीराम.