Stream info in view history. #1643
Conversation
WalkthroughAdds per-stream metrics to connection sync history: introduces a StreamStat type and stream_stats on ConnectionSyncJobObject, plus a collapsible StreamStatsTable UI rendered per sync row showing per-stream recordsCommitted and other metrics. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SyncHistoryRow
participant StreamStatsTable
User->>SyncHistoryRow: Click stream stats toggle
SyncHistoryRow->>SyncHistoryRow: set showStreamStats = true/false
alt stream_stats present and showStreamStats true
SyncHistoryRow->>StreamStatsTable: render(stream_stats)
else no stream_stats
SyncHistoryRow-->>User: toggle disabled
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Out-of-scope changes
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Graph Analysis (1)src/components/Connections/ConnectionSyncHistory.tsx (1)
⏰ 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)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (2)
src/components/Connections/ConnectionSyncHistory.tsx (2)
366-366: Actions column width increaseThe 350px width resolves space for the new icon. Consider using theme spacing to keep consistency, but this is fine as-is.
- width: '350px', + width: 350,
430-434: Render per-stream stats: good conditional; consider column span in detail rowThe conditional render is correct. Minor: the detail row above uses colSpan={5} while the table has 6 columns. Using columns.length keeps it resilient and avoids layout quirks if headers change.
Apply this diff (on the detail row’s TableCell):
- <TableCell colSpan={5} style={{ paddingBottom: 0, paddingTop: 0 }}> + <TableCell colSpan={columns.length} style={{ paddingBottom: 0, paddingTop: 0 }}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Connections/ConnectionSyncHistory.tsx(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/components/Connections/ConnectionSyncHistory.tsx (1)
src/config/constant.ts (1)
AIRBYTE_JOB_STATUS_FAILED(43-43)
🔇 Additional comments (4)
src/components/Connections/ConnectionSyncHistory.tsx (4)
25-25: LGTM: icon imports for expand/collapseImports are correct and match usage below.
214-215: State for stream stats toggle looks goodLocal UI state to control expand/collapse is appropriate. No issues.
106-114: Does stream_stats include per-stream status/last-synced? Objective gapIssue #1414 asks for per-stream: Name, Number of Records Pulled, Last Synced Time, and Status. The current UI shows Name + Records Committed only. If the backend provides per-stream status and timestamps, we can extend the table easily; if not, we may need API changes.
Would you confirm the backend payload shape for stream_stats? If fields like status or lastSyncedAt exist (or can be derived), I can follow up with a patch to display them.
Also applies to: 170-195, 403-417
106-114: Ensure JSON key mapping forstream_stats
TheConnectionSyncJobObject.stream_statstype uses camelCase properties (streamName,recordsEmitted, etc.), but the raw Airbyte API returns snake_case JSON (stream_name,records_emitted, etc.). Without a casing conversion step, those fields will beundefinedat runtime.• Verify the
httpGethelper inhelpers/http.ts(or wherever it’s defined) to see if it converts snake_case keys to camelCase.
• If it doesn’t, add a transformation either insidehttpGetor immediately after callingfetchAirbyteSyncsto mapstream_statskeys from snake_case → camelCase.
• Audit any other code paths or DTOs that consumestream_statsto ensure they all expect the same shape.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1643 +/- ##
==========================================
- Coverage 75.96% 75.94% -0.03%
==========================================
Files 102 102
Lines 7769 7789 +20
Branches 1867 1886 +19
==========================================
+ Hits 5902 5915 +13
- Misses 1753 1842 +89
+ Partials 114 32 -82 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…improved clarity and reusability
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
src/components/Connections/ConnectionSyncHistory.tsx (4)
171-205: Improve StreamStatsTable: stable keys and right-align numeric column
- Use a stable key (streamName) instead of array index.
- Right-align numeric values for readability.
- Optional: remove the global center text-align to let per-column alignment apply cleanly.
-const StreamStatsTable = ({ streamStats }: { streamStats: StreamStat[] }) => { +const StreamStatsTable = ({ streamStats }: { streamStats: StreamStat[] }) => { return ( <Box sx={{ p: 2, display: 'flex', width: '100%' }}> <Table size="small" sx={{ width: '100%', border: '1px solid #ccc', borderCollapse: 'collapse', - '& td, & th': { - border: '1px solid #ccc', - textAlign: 'center', - }, + '& td, & th': { + border: '1px solid #ccc', + }, }} > <TableHead> <TableRow> <TableCell sx={{ fontWeight: 700, color: '#00897B' }}>Stream Name</TableCell> - <TableCell sx={{ fontWeight: 700, color: '#00897B' }}>Records Committed</TableCell> + <TableCell sx={{ fontWeight: 700, color: '#00897B' }} align="right"> + Records Committed + </TableCell> </TableRow> </TableHead> <TableBody> - {streamStats.map((stream, index) => ( - <TableRow key={index}> + {streamStats.map((stream) => ( + <TableRow key={stream.streamName}> <TableCell sx={{ fontWeight: 500 }}>{stream.streamName}</TableCell> - <TableCell sx={{ fontWeight: 500 }}> + <TableCell sx={{ fontWeight: 500 }} align="right"> {stream.recordsCommitted.toLocaleString()} </TableCell> </TableRow> ))} </TableBody> </Table> </Box> ); };
413-427: Add accessibility: aria-label/aria-expanded and optional tooltip for the stream stats toggleThe toggle IconButton lacks an accessible label and state. Add aria-label and aria-expanded. Consider a Tooltip; wrap in a span so tooltips work when disabled.
Apply this diff:
- <IconButton + <IconButton size="small" onClick={() => setShowStreamStats(!showStreamStats)} disabled={ !connectionSyncJob.stream_stats || connectionSyncJob.stream_stats.length === 0 } sx={{ color: '#00897B', '&.Mui-disabled': { color: '#ccc', }, }} + aria-label={showStreamStats ? 'Hide stream stats' : 'Show stream stats'} + aria-expanded={showStreamStats} > {showStreamStats ? <ExpandLess /> : <ExpandMore />} </IconButton>Optional Tooltip usage:
// add with other imports import { Tooltip } from '@mui/material';<Tooltip title={showStreamStats ? 'Hide stream stats' : 'Show stream stats'}> <span> <IconButton // ...same props as above aria-label={showStreamStats ? 'Hide stream stats' : 'Show stream stats'} aria-expanded={showStreamStats} > {showStreamStats ? <ExpandLess /> : <ExpandMore />} </IconButton> </span> </Tooltip>Optional: connect the button to content for a11y:
- Add aria-controls on the IconButton: aria-controls={
stream-stats-${connectionSyncJob.job_id}}- Add id on the container rendering StreamStatsTable: id={
stream-stats-${connectionSyncJob.job_id}}
115-115: Make stream_stats optional to match UI guards and avoid type/usage mismatchThe UI checks for undefined stream_stats, but the interface marks it as required. Align typing with usage.
Apply this diff:
-interface ConnectionSyncJobObject { +interface ConnectionSyncJobObject { job_type: ConnectionJobType; last_attempt_no: number; bytes_committed: string; created_at: string; job_id: number; logs: string[]; records_committed: number; status: string; duration_seconds: number; reset_config: any | null; - stream_stats: StreamStat[]; + stream_stats?: StreamStat[]; }
392-397: Guard ToggleButtonGroup onChange null and narrow handleLogActions typeToggleButtonGroup onChange can pass null. Currently you call handleLogActions unguarded and its signature expects string. Guard and narrow the type to avoid calling with null and to benefit from strict typing.
Apply these diffs:
- const [action, setAction] = useState<'detail' | 'summary' | null>(null); + const [action, setAction] = useState<'detail' | 'summary' | null>(null); - const handleLogActions = (action: string) => { + const handleLogActions = (action: 'detail' | 'summary') => { if (action === 'summary' && summarizedLogs.length < 1) { summarizeLogs(); trackAmplitudeEvent('[ai-summary] Button clicked'); } else if (action === 'detail' && detailedLogs.length < 1) { getDetailedLogs(); if (connectionSyncJob.status === AIRBYTE_JOB_STATUS_RUNNING) { pollForDetailedSyncLogs(); } } };- onChange={(event, newAction) => { - setAction(newAction); - handleLogActions(newAction); - trackAmplitudeEvent('[connection-logs] Button clicked'); - }} + onChange={(event, newAction: 'detail' | 'summary' | null) => { + setAction(newAction); + if (newAction) { + handleLogActions(newAction); + trackAmplitudeEvent('[connection-logs] Button clicked'); + } + }}Also applies to: 304-316
🧹 Nitpick comments (2)
src/components/Connections/ConnectionSyncHistory.tsx (2)
95-103: StreamStat type addition is good; verify API field casing alignmentType is clear and usable. Ensure backend returns camelCase for these fields (streamName, recordsCommitted, etc.). If API uses snake_case, add a transformation layer or adapt the type accordingly.
440-444: Conditional stream stats rendering is good; ties back to optional typingRendering is correctly guarded against undefined/empty. This further supports marking stream_stats optional on the type to match usage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Connections/ConnectionSyncHistory.tsx(7 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/components/Connections/ConnectionSyncHistory.tsx (1)
src/config/constant.ts (1)
AIRBYTE_JOB_STATUS_FAILED(43-43)
⏰ 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)
- GitHub Check: checks
🔇 Additional comments (1)
src/components/Connections/ConnectionSyncHistory.tsx (1)
25-25: Imports for expand/collapse icons look goodNo issues; consistent with MUI icon usage.
Fixes: #1414
Summary by CodeRabbit
New Features
UI