Transcript viewer final #1603
Transcript viewer final #1603
Conversation
📝 WalkthroughWalkthroughMoves chat interactive and trigger elements into React portals with fixed positioning and higher z-index; adds tooltip/disabled styling for floating controls; refactors transcript viewer to segment-based rendering with auto-scroll and a new segmentation API and tests; simplifies seed transcript generation; adjusts API path prefixes in client/server; removes an unused banner in auth. Changes
Sequence Diagram(s)sequenceDiagram
participant Viewer as TranscriptViewer (Component)
participant Hook as useSegments (Hook)
participant Splitter as splitIntoSegments (Logic)
participant Grouper as groupIntoTurns (Logic)
participant Builder as buildSegments (Logic)
participant SegmentComp as Segment (Component)
participant AutoScroll as useAutoScroll (Hook)
Note over Viewer,AutoScroll: Segment-based transcript rendering (new)
Viewer->>Hook: request segments(sessionId)
Hook->>Splitter: split words into candidate groups
Splitter-->>Hook: word groups
Hook->>Grouper: group words by channel into turns
Grouper-->>Hook: segments [{channel, words}, ...]
Hook->>Builder: build final segments (merge finals + partials)
Builder-->>Hook: Segment[]
Hook-->>Viewer: return Segment[]
Viewer->>SegmentComp: render each Segment
Viewer->>AutoScroll: register container & deps
AutoScroll-->>Viewer: scroll on updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/simple.mdc)
Files:
🧠 Learnings (1)📚 Learning: 2025-10-26T00:28:22.016ZApplied to files:
🧬 Code graph analysis (1)apps/desktop/src/devtool/seed/shared/transcript.ts (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (4)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (12)
apps/desktop/src/components/main/sidebar/profile/auth.tsx (1)
115-175: Consider removing unused code or deferring to a future PR.The
TryProBannerfunction is currently unused and marked with a broad@ts-expect-errorannotation. Consider:
- Removing it until actually needed, or
- If keeping for near-term use, replace
@ts-expect-errorwith a more specific@ts-expect-error [reason]or track implementation in an issueUnused code with error suppression can hide type issues and creates maintenance overhead.
apps/desktop/src/devtool/seed/shared/transcript.ts (2)
25-74: Hoist static word lists to module scope to avoid per-call allocations.Move starters/bridges out of generateSentence() and reuse them. Tiny perf win and cleaner.
Example:
// top-level const STARTERS = ["yeah","so","honestly","right","okay","look","listen","alright"] as const; const BRIDGES = ["you know","I mean","kind of","sort of","at the moment","for example","basically","on our side"] as const; // inside generateSentence() if (faker.datatype.boolean({ probability: 0.5 })) { appendPhrase(sentenceWords, faker.helpers.arrayElement(STARTERS)); } ... appendPhrase(sentenceWords, faker.helpers.arrayElement(BRIDGES));
75-82: Optional: allow deterministic seeds for repeatable transcripts.Add an optional seed param and call faker.seed(seed) at the start. Helpful for debugging/tests.
Example:
export const generateTranscript = (seed?: number) => { if (seed != null) faker.seed(seed); // ... };apps/desktop/src/components/main/body/sessions/note-input/transcript/viewer/segment.test.ts (3)
7-28: buildSegments tests: solid coverage of merge+turnsHappy path looks good. Consider adding a case with >2 channel alternations (e.g., 0→1→0→1→0) to ensure turn boundaries remain stable with more interleaving.
153-254: groupIntoTurns: tie-breaking and zero-length wordsConsider tests for:
- Ties: two words with identical start_ms on different channels.
- Zero-length or overlapping words to confirm the sort + grouping still produce intended turns.
56-151: Add three recommended test cases formergeWordsByChannelfunctionThe test file is missing three important edge case tests:
Equal
start_msacross channels — Verify deterministic ordering when words from different channels have identical start times. Current sorting happens per-channel only; this tests stability when merging across channels ingroupIntoTurns.Boundary adjacency — Add test where one word's
end_msequals the next word'sstart_ms(e.g., word A ends at 300ms, word B starts at 300ms). This validates handling of continuous/adjacent timing.Multi-key partial words with channel mismatch — Expand the existing single-key test ("uses word.channel property...") to use multiple partial word keys where some keys don't match their contained words'
channelvalues, confirming robust channel-over-key priority.These tests ensure the function's sorting and channel assignment logic handles edge cases correctly before reaching downstream consumers like
groupIntoTurnsandbuildSegments.apps/desktop/src/components/main/body/sessions/note-input/transcript/viewer/index.tsx (1)
25-27: Use stable keys for segmentsIndex keys can mis-associate on insertion/reordering. Prefer a timestamp‑based key derived from first/last word.
- (segment, i) => <Segment key={i} segment={segment} />, + (segment) => { + const first = segment.words[0]; + const last = segment.words[segment.words.length - 1] ?? first; + const stableKey = `${segment.channel}:${first?.start_ms ?? 0}-${last?.end_ms ?? 0}`; + return <Segment key={stableKey} segment={segment} />; + },apps/desktop/src/components/main/body/sessions/note-input/transcript/viewer/segment.ts (5)
71-77: Transcript selection may be non-deterministicPicking transcriptIds?.[0] could select an older transcript if index order changes. Consider selecting the most recent or explicit active transcript.
If “transcriptBySession” isn’t time‑sorted, adjust:
- const transcriptId = transcriptIds?.[0]; + const transcriptId = transcriptIds?.[transcriptIds.length - 1]; // most recentVerify index ordering before applying.
115-117: Sentence boundary heuristic: include common non‑ASCII punctuationSupport “。” “！ ” “？” “…” to improve multi‑language segmentation.
-function isSentenceEnding(text: string): boolean { - return /[.!?]$/.test(text.trim()); -} +function isSentenceEnding(text: string): boolean { + // ASCII . ! ? and common CJK/ellipsis sentence terminators + return /[.!?…。！？]$/.test(text.trim()); +}
137-141: Defensive ordering for splitIntoSegmentsThe function assumes chronological input. Either assert or sort defensively to avoid pathological splits with unsorted input.
export function splitIntoSegments( words: MaybePartialWord[], options: SplitOptions = {}, ): MaybePartialWord[][] { const { maxWordsPerSegment = 30, minGapMs = 2000 } = options; + // Ensure chronological order + const sorted = words.length > 1 && words[0].start_ms > words[1].start_ms + ? [...words].sort((a, b) => a.start_ms - b.start_ms) + : words; - if (words.length === 0) { + if (sorted.length === 0) { return []; } - if (words.length === 1) { - return [words]; + if (sorted.length === 1) { + return [sorted]; } - const segments: MaybePartialWord[][] = []; - let currentSegment: MaybePartialWord[] = [words[0]]; + const segments: MaybePartialWord[][] = []; + let currentSegment: MaybePartialWord[] = [sorted[0]]; - for (let i = 1; i < words.length; i++) { - const prevWord = words[i - 1]; - const currentWord = words[i]; + for (let i = 1; i < sorted.length; i++) { + const prevWord = sorted[i - 1]; + const currentWord = sorted[i];
250-255: Consider splitting long turns into readable segmentsbuildSegments currently returns channel turns only. For long monologues, UI may show very large blocks. Optionally split each turn by time/size.
export function buildSegments( finalWords: Record<string, persisted.Word>, partialWords: Record<number, Array<{ text: string; start_ms: number; end_ms: number; channel: number }>>, ): Segment[] { - return groupIntoTurns(mergeWordsByChannel(finalWords, partialWords)); + const turns = groupIntoTurns(mergeWordsByChannel(finalWords, partialWords)); + // Split each turn into sub‑segments for readability + const segmented: Segment[] = []; + for (const turn of turns) { + const pieces = splitIntoSegments(turn.words); + for (const words of pieces) { + segmented.push({ channel: turn.channel, words }); + } + } + return segmented; }
100-108: Memoized useSegments: good shapeHook composes final/partial words cleanly. If you adopt turn splitting, the memoization boundaries remain valid.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
apps/desktop/src/components/chat/interactive.tsx(3 hunks)apps/desktop/src/components/chat/trigger.tsx(2 hunks)apps/desktop/src/components/main/body/sessions/floating/generate.tsx(1 hunks)apps/desktop/src/components/main/body/sessions/floating/index.tsx(1 hunks)apps/desktop/src/components/main/body/sessions/floating/shared.tsx(1 hunks)apps/desktop/src/components/main/body/sessions/note-input/index.tsx(1 hunks)apps/desktop/src/components/main/body/sessions/note-input/transcript/viewer/index.tsx(1 hunks)apps/desktop/src/components/main/body/sessions/note-input/transcript/viewer/segment.test.ts(6 hunks)apps/desktop/src/components/main/body/sessions/note-input/transcript/viewer/segment.ts(1 hunks)apps/desktop/src/components/main/sidebar/profile/auth.tsx(1 hunks)apps/desktop/src/devtool/seed/shared/transcript.ts(2 hunks)crates/am/src/client.rs(5 hunks)plugins/local-stt/src/server/external.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/simple.mdc)
After a substantial amount of TypeScript changes, run
pnpm -r typecheck
Files:
apps/desktop/src/components/main/body/sessions/floating/index.tsxapps/desktop/src/components/main/body/sessions/note-input/transcript/viewer/index.tsxapps/desktop/src/components/main/body/sessions/note-input/index.tsxapps/desktop/src/components/main/body/sessions/note-input/transcript/viewer/segment.test.tsapps/desktop/src/devtool/seed/shared/transcript.tsapps/desktop/src/components/chat/interactive.tsxapps/desktop/src/components/main/body/sessions/floating/generate.tsxapps/desktop/src/components/main/body/sessions/note-input/transcript/viewer/segment.tsapps/desktop/src/components/main/body/sessions/floating/shared.tsxapps/desktop/src/components/main/sidebar/profile/auth.tsxapps/desktop/src/components/chat/trigger.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/simple.mdc)
**/*.{tsx,jsx}: When many classNames have conditional logic in React components, usecnfrom@hypr/utils
When usingcn, always pass an array of class segments
When usingcn, split entries by logical grouping for readability
Usemotion/reactinstead offramer-motion
Files:
apps/desktop/src/components/main/body/sessions/floating/index.tsxapps/desktop/src/components/main/body/sessions/note-input/transcript/viewer/index.tsxapps/desktop/src/components/main/body/sessions/note-input/index.tsxapps/desktop/src/components/chat/interactive.tsxapps/desktop/src/components/main/body/sessions/floating/generate.tsxapps/desktop/src/components/main/body/sessions/floating/shared.tsxapps/desktop/src/components/main/sidebar/profile/auth.tsxapps/desktop/src/components/chat/trigger.tsx
🧬 Code graph analysis (6)
apps/desktop/src/components/main/body/sessions/note-input/transcript/viewer/index.tsx (1)
apps/desktop/src/components/main/body/sessions/note-input/transcript/viewer/segment.ts (2)
useSegments(100-108)Segment(207-210)
apps/desktop/src/components/main/body/sessions/note-input/index.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/main/body/sessions/note-input/transcript/viewer/segment.test.ts (2)
apps/desktop/src/components/main/body/sessions/note-input/transcript/viewer/test-utils.ts (1)
word(66-81)apps/desktop/src/components/main/body/sessions/note-input/transcript/viewer/segment.ts (4)
buildSegments(250-255)mergeWordsByChannel(11-51)groupIntoTurns(212-248)splitIntoSegments(137-205)
apps/desktop/src/devtool/seed/shared/transcript.ts (3)
packages/db/src/schema.ts (1)
words(127-138)apps/desktop/src/store/tinybase/persisted.ts (1)
Word(124-124)apps/desktop/src/utils/index.ts (1)
DEFAULT_USER_ID(6-6)
apps/desktop/src/components/main/body/sessions/note-input/transcript/viewer/segment.ts (1)
apps/desktop/src/components/main/body/sessions/note-input/transcript/viewer/test-utils.ts (1)
word(66-81)
apps/desktop/src/components/chat/trigger.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
🔇 Additional comments (13)
apps/desktop/src/components/main/body/sessions/note-input/index.tsx (1)
32-39: LGTM! Clean implementation following coding guidelines.The dynamic className computation using
cncorrectly follows the coding guidelines: array syntax, logical grouping (layout → spacing → conditional overflow), and clear conditional logic for transcript vs. editor tabs.apps/desktop/src/components/main/body/sessions/floating/shared.tsx (1)
31-31: LGTM! Disabled state styling improves UX.The explicit disabled state styling ensures disabled buttons remain clearly visible with full opacity and a distinct neutral background, providing better visual feedback than the default disabled appearance.
apps/desktop/src/components/main/body/sessions/floating/index.tsx (1)
42-42: LGTM! Appropriate z-index for floating UI.The z-50 value elevates the floating button container above standard content while remaining below portaled elements (which use z-[100]), establishing a clear stacking hierarchy.
apps/desktop/src/components/main/body/sessions/floating/generate.tsx (1)
113-118: LGTM! Excellent UX improvement.The conditional tooltip provides clear feedback when the language model is not configured, helping users understand why regeneration is unavailable. This aligns well with the existing guard in
onRegenerate.apps/desktop/src/components/chat/interactive.tsx (1)
3-3: LGTM! Correct portal implementation.The portal-based rendering is properly implemented:
- Content renders to
document.bodyviacreatePortal- Fixed positioning with z-[100] ensures proper stacking above other UI elements
- All existing resizable functionality is preserved
This aligns with the broader UI layering strategy seen in other components.
Also applies to: 20-23, 67-69
apps/desktop/src/components/chat/trigger.tsx (1)
1-1: LGTM! Correct portal implementation and propercnusage.The portal-based rendering follows the same pattern as
interactive.tsx, with fixed positioning and z-[100] for proper stacking. Thecnusage correctly follows the coding guidelines by using an array with logical grouping (positioning, sizing, styling, transitions).Also applies to: 6-26
apps/desktop/src/components/main/sidebar/profile/auth.tsx (1)
105-112: LGTM! Cleaner authentication flow.The simplified unauthenticated state removes banner complexity while maintaining the sign-in functionality through
handleStartSignIn.crates/am/src/client.rs (1)
26-26: LGTM! API versioning centralized in base URL.All five endpoint paths have been consistently updated to remove the
/v1prefix. This refactoring centralizes the API version in thebase_urlparameter (as seen inplugins/local-stt/src/server/external.rsline 50), making it easier to manage versioning across the client.The final URLs remain unchanged:
- Previously:
base_url+/v1/status- Now:
base_url/v1+/statusAlso applies to: 41-41, 54-54, 65-65, 76-76
plugins/local-stt/src/server/external.rs (1)
50-51: All Client instantiations are consistent with the /v1 prefix in base URLs.The comprehensive search confirms there is only one instantiation of
hypr_am::Client::new()in the entire codebase, and it's the one under review atplugins/local-stt/src/server/external.rs:51. This instantiation correctly includes/v1in the base URL on line 50 viaformat!("http://localhost:{}/v1", port).The centralization of API versioning from individual endpoint paths to the base URL is complete and consistently applied.
apps/desktop/src/components/main/body/sessions/note-input/transcript/viewer/index.tsx (2)
20-23: cn usage LGTMArray form and logical grouping follow guidelines. No action.
2-3: Correct the verification command for this workspace's turbo configurationThe suggested verification commands don't work in this workspace. The workspace uses Turbo for task orchestration and doesn't have a workspace-wide
vitesttask. To properly verify TypeScript changes inapps/desktop, use:turbo run typecheck pnpm -F @hypr/desktop testAlternatively, from the
apps/desktopdirectory:pnpm typecheck pnpm testLikely an incorrect or invalid review comment.
apps/desktop/src/components/main/body/sessions/note-input/transcript/viewer/segment.ts (1)
11-51: mergeWordsByChannel implementation looks correctMerges, preserves isFinal, and per‑channel sorting are straightforward and efficient. Nice.
apps/desktop/src/components/main/body/sessions/note-input/transcript/viewer/segment.test.ts (1)
256-415: Add three test cases for edge cases and preconditions in splitIntoSegmentsAll three suggested tests are missing and valid:
- Gap equality test (gap === 2000ms): Existing tests use gaps of 2900ms and 2500ms; no boundary test for exact minGapMs. Verifies ">=" behavior.
- Unsorted input test: The implementation assumes sorted input but doesn't sort internally. This precondition should be verified to document expected behavior or catch silent failures.
- Long unpunctuated run with small gaps: No test covers the fallback split logic when no sentence boundaries exist and gaps stay below minGapMs. Ensures maxWordsPerSegment limits are respected cleanly.
No description provided.