Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb90e004a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Greptile SummaryThis PR adds per-turn latency tracing to talk mode, threading timing anchors through the full pipeline from final transcript to speech output (ElevenLabs streaming and system TTS), and also introduces soft-boundary splitting in Key changes:
Issues found:
Confidence Score: 2/5
Last reviewed commit: eb90e00 |
apps/shared/OpenClawKit/Sources/OpenClawKit/TalkSystemSpeechSynthesizer.swift
Show resolved
Hide resolved
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d3f1fcaf4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| private func ensureLatencyTrace(transcriptChars: Int) { | ||
| guard self.activeLatencyTrace == nil else { return } |
There was a problem hiding this comment.
Reset latency traces when starting a new transcript turn
ensureLatencyTrace bails out if activeLatencyTrace is already set, but this commit introduced paths where a trace can be started and never finished (for example push-to-talk offline/cancel exits that do not call finishLatencyTrace). In that case, the next turn can reuse stale timestamps/anchors from a previous turn, which merges separate interactions into one trace and corrupts per-turn latency metrics.
Useful? React with 👍 / 👎.
| } else { | ||
| await self.playAssistant(text: assistantText) | ||
| } | ||
| self.finishLatencyTrace(outcome: "spoken", fields: ["runId=\(runId)"]) |
There was a problem hiding this comment.
Record playback failures as non-spoken latency outcomes
This unconditionally finalizes tracing as outcome=spoken, but playAssistant handles TTS/system-speech errors internally and can end with statusText = "Speak failed: ..." without any spoken output. When that happens, latency telemetry marks failed speech turns as successful, which skews success-rate and latency analysis for talk mode.
Useful? React with 👍 / 👎.
|
Thanks for the solid work here, @ngutman. Closing this original PR in favor of smaller, reviewable slices that preserve the reliability/UX improvements:
This keeps risk isolated and makes it easier to validate each behavior independently. |
Summary
onStartcallback toTalkSystemSpeechSynthesizer.speak(...)viaAVSpeechSynthesizerDelegate.didStartTesting
swift test --package-path apps/ios --filter TalkModeIncrementalSpeechBufferTests(fails in this repo layout:Could not find Package.swiftforapps/ios)