Conversation
This was referenced Mar 27, 2026
Collaborator
Author
This was referenced Mar 27, 2026
Contributor
📝 WalkthroughWalkthroughAdds session-level log viewing and parent_request_id filtering, extracts detailed log UI to LogDetailView, introduces SessionDetailsSheet and a session API endpoint, adds realtime.turn request type, accepts single-string parent_request_id in filters, and removes metadata-driven log columns. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant LogsPage as LogsPage
participant LogDetailSheet as LogDetailSheet
participant LogDetailView as LogDetailView
participant SessionSheet as SessionDetailsSheet
participant API as API Server
User->>LogsPage: Click log row
LogsPage->>LogDetailSheet: open(logId)
LogDetailSheet->>API: GET /logs/:id
API-->>LogDetailSheet: full LogEntry
LogDetailSheet->>LogDetailView: render(full LogEntry)
User->>LogDetailView: Click "View Session"
LogDetailView->>LogsPage: onViewSession(sessionId, logId)
LogsPage->>SessionSheet: open(sessionId, highlightedLogId)
SessionSheet->>API: GET /logs/sessions/:sessionId?limit&offset&order
API-->>SessionSheet: session logs + aggregates
SessionSheet-->>User: display session logs
User->>SessionSheet: Click parent_request_id
SessionSheet->>LogsPage: onFilterByParentRequestId(parentId)
LogsPage->>API: GET /logs?parent_request_id=parentId
API-->>LogsPage: filtered logs
LogsPage-->>User: show filtered list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
|
Contributor
Confidence Score: 4/5Not safe to merge — the session details sheet cannot be closed due to an undefined handler reference All previously flagged P1 concerns from earlier review rounds are resolved. One new P1 remains: handleSessionSheetOpenChange at page.tsx:940 is undeclared, causing both a TypeScript compile failure and a ReferenceError at runtime on every sheet-close action. Once this one-function gap is filled the PR is otherwise well-structured. ui/app/workspace/logs/page.tsx — missing handleSessionSheetOpenChange definition before the return statement Important Files Changed
Reviews (16): Last reviewed commit: "feat: add realtime session browsing and ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/app/workspace/logs/sheets/logDetailsSheet.tsx (1)
31-42:⚠️ Potential issue | 🟠 MajorDon't block the detail sheet on the follow-up fetch.
displayLogalready falls back to the row payload, but Lines 37-41 still render only the spinner untilfullLog.id === log.id. IfuseLazyGetLogByIdQueryerrors, the sheet never exits loading even thoughlogis available.Suggested adjustment
- const isFullDataReady = fullLog?.id === log.id && !isFetching; - const displayLog = isFullDataReady ? fullLog : log; + const hasFetchedCurrentLog = fullLog?.id === log.id; + const displayLog = hasFetchedCurrentLog ? fullLog : log; @@ - {!isFullDataReady ? ( - <div className="flex h-full items-center justify-center"> - <Loader2 className="text-muted-foreground h-6 w-6 animate-spin" /> - </div> - ) : ( - <LogDetailView - log={displayLog} - handleDelete={handleDelete} - onClose={() => onOpenChange(false)} - onFilterByParentRequestId={onFilterByParentRequestId} - headerAction={ - displayLog.parent_request_id && onViewSession ? ( - <Button - variant="outline" - size="sm" - onClick={() => onViewSession(displayLog.parent_request_id as string, displayLog.id)} - > - View Session - </Button> - ) : null - } - /> - )} + <LogDetailView + log={displayLog} + loading={isFetching && !hasFetchedCurrentLog} + handleDelete={handleDelete} + onClose={() => onOpenChange(false)} + onFilterByParentRequestId={onFilterByParentRequestId} + headerAction={ + displayLog.parent_request_id && onViewSession ? ( + <Button + variant="outline" + size="sm" + onClick={() => onViewSession(displayLog.parent_request_id, displayLog.id)} + > + View Session + </Button> + ) : null + } + />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/logs/sheets/logDetailsSheet.tsx` around lines 31 - 42, The sheet is blocked waiting for the follow-up fetch because the JSX uses isFullDataReady to decide between Loader2 and LogDetailView; instead render LogDetailView whenever displayLog is available (displayLog = fullLog?.id === log.id && !isFetching ? fullLog : log) so the row payload shows immediately, and only show the spinner as an overlay/state when isFetching && fullLog?.id !== log.id (or when the lazy query is loading); also surface the lazy query error (from useLazyGetLogByIdQuery) to stop the spinner and allow displayLog to render, or pass isFetching/isError into LogDetailView so it can show field-level loading/error states.
🧹 Nitpick comments (4)
ui/app/workspace/providers/fragments/openaiConfigFormFragment.tsx (1)
15-15: Use@/...alias import instead of relative path inui/TSXLine 15 should use the project alias for consistency with the providers UI convention.
♻️ Suggested change
-import { buildProviderUpdatePayload } from "../views/utils"; +import { buildProviderUpdatePayload } from "@/app/workspace/providers/views/utils";Based on learnings: in
ui/app/workspace/providers/, TSX files should consistently use@/...alias imports instead of relative paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/providers/fragments/openaiConfigFormFragment.tsx` at line 15, Replace the relative import of buildProviderUpdatePayload with the project alias import used across the providers UI; update the import statement that references buildProviderUpdatePayload in openaiConfigFormFragment.tsx to use the "@/workspace/providers/views/utils" (or the correct `@/`... path matching your aliases) so it follows the providers UI convention and stays consistent with other TSX files.ui/app/workspace/providers/views/utils.ts (1)
18-18: Consider merging partial config updates to prevent future data lossThe current pattern
updates.openai_config ?? provider.openai_configreplaces the entire object when a partial update is provided. WhileOpenAIConfigcurrently has onlydisable_store, this approach is fragile if new properties are added.Note: This same pattern exists throughout the function for
network_config,proxy_config, andcustom_provider_config. If addressing this, consider applying the fix consistently to all config objects or deferring as part of a broader refactor.Suggested improvement
- openai_config: updates.openai_config ?? provider.openai_config, + openai_config: updates.openai_config + ? { ...(provider.openai_config ?? {}), ...updates.openai_config } + : provider.openai_config,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/providers/views/utils.ts` at line 18, The assignment using updates.openai_config ?? provider.openai_config will overwrite the whole config when a partial update is provided; change it to merge partial updates instead (e.g., set openai_config to a shallow merge of provider.openai_config and updates.openai_config) so fields not present in updates are preserved, and apply the same pattern for network_config, proxy_config, and custom_provider_config in the same function (use the spread/merge of provider.* with updates.*).ui/lib/store/apis/logsApi.ts (1)
85-139: Reduce filter param duplication to avoid drift.
getLogsandgetLogsStatsstill hand-build params whilebuildFilterParamsexists. This increases maintenance risk as filters evolve (likeparent_request_id).♻️ Suggested consolidation
getLogs: builder.query<...>({ query: ({ filters, pagination }) => { - const params: Record<string, string | number> = { + const params: Record<string, string | number> = { limit: pagination.limit, offset: pagination.offset, sort_by: pagination.sort_by, order: pagination.order, + ...buildFilterParams(filters), }; - - // Add filters to params if they exist - ... return { url: "/logs", params }; }, }), ... getLogsStats: builder.query<...>({ query: ({ filters }) => { - const params: Record<string, string | number> = {}; - // Add filters to params if they exist - ... + const params = buildFilterParams(filters); return { url: "/logs/stats", params }; }, }),Also applies to: 168-217
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/lib/store/apis/logsApi.ts` around lines 85 - 139, The query handler in getLogs manually recreates filter-to-param logic that buildFilterParams already implements, causing duplication and drift; replace the entire manual params construction in the getLogs query with a call to buildFilterParams (and reuse the same call in getLogsStats), passing the incoming filters and pagination, ensure the returned params still include pagination keys (limit, offset, sort_by, order) and special handling for metadata_filters/parent_request_id remains preserved by buildFilterParams, and update any dependent types or tests to use the consolidated buildFilterParams output instead of the duplicated params construction.ui/lib/types/logs.ts (1)
511-515: KeepPagination.sort_bynarrowed to the supported log sort keys.This shared type is still consumed by the main logs page/table as the four-key contract (
timestamp,latency,tokens,cost). Widening it tostringweakens that contract for every caller just to satisfy the session flow. A separate session-pagination shape is safer than loosening the shared one.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/lib/types/logs.ts` around lines 511 - 515, The Pagination interface's sort_by is too wide; change Pagination.sort_by from string to the union of supported log keys ("timestamp" | "latency" | "tokens" | "cost") to restore the four-key contract used by the logs page/table, and if session flow needs broader keys, introduce a separate type (e.g., SessionPagination with sort_by: string) for session-specific usage; update any callers that rely on the shared Pagination to use the narrowed keys or the new SessionPagination as appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/app/workspace/logs/sheets/logDetailsSheet.tsx`:
- Around line 47-55: Add a stable test selector to the new "View Session" Button
rendered in the headerAction conditional (the Button inside the block guarded by
displayLog.parent_request_id && onViewSession) by adding a data-testid attribute
following the project pattern, e.g. data-testid="session-button-view"
(entity=session, element=button, qualifier=view), so tests can reliably target
the onViewSession handler that uses displayLog.parent_request_id and
displayLog.id.
In `@ui/app/workspace/logs/sheets/logDetailView.tsx`:
- Around line 146-150: The Async badge currently checks
log.metadata?.isAsyncRequest by truthiness which renders for the string "false";
update the conditional in logDetailView (the JSX around
log.metadata?.isAsyncRequest) to compare the value explicitly (e.g.,
log.metadata?.isAsyncRequest === "true") or normalize the metadata value to a
boolean before rendering so the Badge only shows when the flag is actually
"true".
- Around line 127-140: Replace the non-focusable clickable <code> elements (the
Request ID copy block using log.id and navigator.clipboard.writeText with
toast.success/toast.error) with real interactive controls (e.g., <button> or <a
role="button">) so they are keyboard-focusable and accessible; update the click
handler from the inline onClick to the control's onClick and ensure proper
keyboard handling, and add data-testid attributes following the pattern
data-testid="request-id-copy-button" (and similarly for the parent-request
filter control referenced around lines 247-263, e.g.,
data-testid="parent-request-filter-button") to both new elements for testing.
Ensure the new controls still call navigator.clipboard.writeText(log.id) and
show toast success/error as before.
- Around line 210-219: The current rendering fabricates zero latency by using
`log.latency || 0`; change the End Timestamp and Latency render logic to treat
null/undefined separately: for `LogEntryDetailsView` labeled "End Timestamp"
only compute `moment(log.timestamp).add(log.latency, "ms")` when `log.latency`
is not null/undefined (use a nullish check) and otherwise show "N/A"; for the
"Latency" field use a nullish check on `log.latency` and render "N/A" when it is
null/undefined or NaN, otherwise format the numeric latency with `toFixed(2) +
"ms"`. Ensure you reference `LogEntryDetailsView`, `log.timestamp`, and
`log.latency` when making these changes.
- Around line 549-605: The copyRequestBody logic misclassifies realtime.turn (it
is treated as responses and only reads responses_input_history) and also uses
the wrong object type for embeddings; update the predicate checks and branching:
add a specific isRealtimeTurn flag (e.g., const isRealtimeTurn = log.object ===
"realtime.turn") and remove realtime.turn from isResponses, change isEmbedding
to match "embedding" instead of "list", and in the branch for realtime.turn
populate requestBody with the conversation input (use log.input_history/messages
similar to isChat) so turns include the actual input_history when serializing;
update the branches that set requestBody.input/messages to reference these
revised flags (isRealtimeTurn, isResponses, isEmbedding) and reuse
extractTextFromMessage/extractTextsFromMessage as needed.
In `@ui/app/workspace/logs/sheets/sessionDetailsSheet.tsx`:
- Around line 212-232: Replace the non-focusable clickable <code> element used
for filtering with a real interactive element (e.g., a <button> or <a>) so it
can be focused and activated by keyboard; wire its onClick to
onFilterByParentRequestId(sessionId) and keep the Tooltip wrapping behavior,
referencing sessionId and onFilterByParentRequestId to find the code. Add stable
data-testid attributes: session-details-filter-btn on the new filter button,
session-details-sort-btn on the sort Button that calls setSortOrder (and uses
sortOrder), and session-details-load-more-btn on the load-more control mentioned
around the 333-338 region; ensure these elements are semantic,
keyboard-accessible, and retain current visual styles and aria/tooltip behavior.
- Around line 188-201: Live socket updates modify sessionLogs, totalCount, and
hasMore but never update the derived summary fields or fetchedCount, causing UI
mismatch; update those derived values whenever you merge a live log. In the
setSessionLogs handler (and where operation === "create"/"update" is handled)
recalculate or adjust summary.totalCost, summary.totalTokens, summary.latestAt,
and summary.durationMs (either by applying a delta from the incoming log or by
aggregating the updated sessionLogs returned by sortSessionLogs), increment
fetchedCount when a new log is appended, and recompute hasMore using the updated
fetchedCount and totalCount; ensure you update the same state slice that renders
the cards (the summary object and fetchedCount) so cards and pager stay in sync
with setSessionLogs/setTotalCount changes.
- Around line 126-161: The fetch handler loadSessionPage currently ignores
triggerGetSession errors and treats them as empty results; update
loadSessionPage to explicitly check result.error after awaiting
triggerGetSession and handle it separately (e.g., set an error state or call a
provided onError handler, stop further processing, and avoid treating it like an
empty session or closing the sheet), so only successful responses update
setSummary, setSessionLogs, setTotalCount/setHasMore/setFetchedCount and only
the reset branch uses onOpenChange(false) when the result is a successful empty
session; reference loadSessionPage, triggerGetSession, result.error, setSummary,
setSessionLogs and onOpenChange when making the change.
In `@ui/app/workspace/logs/views/columns.tsx`:
- Around line 29-36: getRealtimeTurnMessages currently uses .find(...) which
returns only the first matching tool/user message, causing stale/incomplete
previews; update getRealtimeTurnMessages to collect all messages per role from
log.input_history (e.g., use .filter(...) for role === "tool" and role ===
"user"), map each to getMessageFromContent(message.content), and join them (or
alternatively pick the last matching message) to produce the tool and user
strings; keep the assistant behavior for log.output_message the same and ensure
empty-string fallback when no messages exist.
In `@ui/components/filters/filterPopover.tsx`:
- Around line 202-213: The new Input rendered when showParentRequestIdFilter is
true lacks a test selector; add a data-testid prop to that Input (the component
instance using value={filters.parent_request_id} and onChange={(e) =>
onFilterChange("parent_request_id", e.target.value)}) following the project
pattern entity-element-qualifier, e.g.
data-testid="request-parent-request-id-input", so tests can reliably target the
parent request ID control.
---
Outside diff comments:
In `@ui/app/workspace/logs/sheets/logDetailsSheet.tsx`:
- Around line 31-42: The sheet is blocked waiting for the follow-up fetch
because the JSX uses isFullDataReady to decide between Loader2 and
LogDetailView; instead render LogDetailView whenever displayLog is available
(displayLog = fullLog?.id === log.id && !isFetching ? fullLog : log) so the row
payload shows immediately, and only show the spinner as an overlay/state when
isFetching && fullLog?.id !== log.id (or when the lazy query is loading); also
surface the lazy query error (from useLazyGetLogByIdQuery) to stop the spinner
and allow displayLog to render, or pass isFetching/isError into LogDetailView so
it can show field-level loading/error states.
---
Nitpick comments:
In `@ui/app/workspace/providers/fragments/openaiConfigFormFragment.tsx`:
- Line 15: Replace the relative import of buildProviderUpdatePayload with the
project alias import used across the providers UI; update the import statement
that references buildProviderUpdatePayload in openaiConfigFormFragment.tsx to
use the "@/workspace/providers/views/utils" (or the correct `@/`... path matching
your aliases) so it follows the providers UI convention and stays consistent
with other TSX files.
In `@ui/app/workspace/providers/views/utils.ts`:
- Line 18: The assignment using updates.openai_config ?? provider.openai_config
will overwrite the whole config when a partial update is provided; change it to
merge partial updates instead (e.g., set openai_config to a shallow merge of
provider.openai_config and updates.openai_config) so fields not present in
updates are preserved, and apply the same pattern for network_config,
proxy_config, and custom_provider_config in the same function (use the
spread/merge of provider.* with updates.*).
In `@ui/lib/store/apis/logsApi.ts`:
- Around line 85-139: The query handler in getLogs manually recreates
filter-to-param logic that buildFilterParams already implements, causing
duplication and drift; replace the entire manual params construction in the
getLogs query with a call to buildFilterParams (and reuse the same call in
getLogsStats), passing the incoming filters and pagination, ensure the returned
params still include pagination keys (limit, offset, sort_by, order) and special
handling for metadata_filters/parent_request_id remains preserved by
buildFilterParams, and update any dependent types or tests to use the
consolidated buildFilterParams output instead of the duplicated params
construction.
In `@ui/lib/types/logs.ts`:
- Around line 511-515: The Pagination interface's sort_by is too wide; change
Pagination.sort_by from string to the union of supported log keys ("timestamp" |
"latency" | "tokens" | "cost") to restore the four-key contract used by the logs
page/table, and if session flow needs broader keys, introduce a separate type
(e.g., SessionPagination with sort_by: string) for session-specific usage;
update any callers that rely on the shared Pagination to use the narrowed keys
or the new SessionPagination as appropriate.
🪄 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: 87eb7b78-f7d3-4fd5-9d6f-e785e99f66cc
📒 Files selected for processing (16)
ui/app/workspace/dashboard/page.tsxui/app/workspace/logs/page.tsxui/app/workspace/logs/sheets/logDetailView.tsxui/app/workspace/logs/sheets/logDetailsSheet.tsxui/app/workspace/logs/sheets/sessionDetailsSheet.tsxui/app/workspace/logs/views/columns.test.tsui/app/workspace/logs/views/columns.tsxui/app/workspace/logs/views/filters.tsxui/app/workspace/logs/views/logsTable.tsxui/app/workspace/providers/fragments/openaiConfigFormFragment.tsxui/app/workspace/providers/views/utils.tsui/components/filters/filterPopover.tsxui/lib/constants/logs.test.tsui/lib/constants/logs.tsui/lib/store/apis/logsApi.tsui/lib/types/logs.ts
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_websocket_webrtc_and_client_secret_handlers
branch
from
33a3e91 to
2d1c18b
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_browsing_and_log_filtering_ui
branch
from
cc0cadd to
ecd583b
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ui/app/workspace/logs/page.tsx (1)
58-60: Remove shadowedselectedLogstate — it's never read.Line 58 declares
selectedLogstate, but line 139 shadows it with a derived value (selectedLogFromData ?? fetchedLog). All subsequent references toselectedLogresolve to the shadowed variable, so the calls tosetSelectedLog(lines 329, 914, 936, 954) update a value that is never consumed.Consider removing the state entirely and using a local variable or renaming to avoid confusion:
♻️ Suggested cleanup
- const [selectedLog, setSelectedLog] = useState<LogEntry | null>(null); const [selectedSessionId, setSelectedSessionId] = useState<string | null>(null); const [sessionHighlightedLogId, setSessionHighlightedLogId] = useState<string | null>(null);Then remove the now-unused
setSelectedLogcalls throughout the file (lines 329, 914, 936, 954), as they serve no purpose.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/logs/page.tsx` around lines 58 - 60, The file declares a useState hook const [selectedLog, setSelectedLog] = useState<LogEntry | null>(null) but later a derived variable selectedLogFromData ?? fetchedLog shadows and is actually used; remove the unused state to avoid confusion: delete the selectedLog/useState declaration and remove all calls to setSelectedLog (they have no effect), or alternatively if persistent state was intended, rename the hook (e.g., selectedLogState/setSelectedLogState) and update usages to consistently use that state instead of the derived selectedLogFromData/fetchedLog; ensure references to selectedLogFromData and fetchedLog remain unchanged and that no orphaned setSelectedLog calls remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ui/app/workspace/logs/page.tsx`:
- Around line 58-60: The file declares a useState hook const [selectedLog,
setSelectedLog] = useState<LogEntry | null>(null) but later a derived variable
selectedLogFromData ?? fetchedLog shadows and is actually used; remove the
unused state to avoid confusion: delete the selectedLog/useState declaration and
remove all calls to setSelectedLog (they have no effect), or alternatively if
persistent state was intended, rename the hook (e.g.,
selectedLogState/setSelectedLogState) and update usages to consistently use that
state instead of the derived selectedLogFromData/fetchedLog; ensure references
to selectedLogFromData and fetchedLog remain unchanged and that no orphaned
setSelectedLog calls remain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 98f294a7-9bb4-4ad2-8468-2f66a2ff5e4d
📒 Files selected for processing (5)
ui/app/workspace/dashboard/page.tsxui/app/workspace/logs/page.tsxui/app/workspace/logs/sheets/logDetailView.tsxui/app/workspace/logs/sheets/logDetailsSheet.tsxui/app/workspace/logs/sheets/sessionDetailsSheet.tsx
✅ Files skipped from review due to trivial changes (1)
- ui/app/workspace/logs/sheets/sessionDetailsSheet.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- ui/app/workspace/dashboard/page.tsx
- ui/app/workspace/logs/sheets/logDetailView.tsx
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_websocket_webrtc_and_client_secret_handlers
branch
from
2d1c18b to
b91d7ac
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_browsing_and_log_filtering_ui
branch
from
ecd583b to
839db86
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/app/workspace/logs/views/columns.tsx (1)
126-260:⚠️ Potential issue | 🟠 MajorDon't drop the metadata columns from
createColumns.After this change the logs table can only render the base columns plus actions, so every
metadata_*column disappears even though the rest of this stack still exposes metadata filtering. If the goal is to simplify the prop surface, move metadata-column generation elsewhere instead of removing it outright. Based on learnings,metadata_*columns inui/app/workspace/logs/views/logsTable.tsxare intentionally shown visible by default to aid discoverability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/logs/views/columns.tsx` around lines 126 - 260, The createColumns function removed generation of dynamic metadata_* columns causing those columns to vanish; restore or reintroduce the metadata column logic by generating metadata columns (those keyed as metadata_*) and appending them to baseColumns before returning (keep using the existing ColumnDef<LogEntry> shape), or move that generation into a helper used by createColumns so logsTable.tsx can continue to show metadata columns by default; update createColumns (and any helper it calls) to produce the metadata column definitions and include them in the returned array alongside actionsColumn so filtering in logsTable remains functional.
♻️ Duplicate comments (1)
ui/app/workspace/logs/sheets/logDetailView.tsx (1)
559-563:⚠️ Potential issue | 🟠 MajorMatch
embeddingin the copy-request-body guard.
isEmbeddingstill checks"list", so the embedding serializer never runs and the new copy action still rejects embedding logs as unsupported.🔧 Proposed fix
- const isEmbedding = log.object === "list"; + const isEmbedding = log.object === "embedding";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/logs/sheets/logDetailView.tsx` around lines 559 - 563, The guard variable isEmbedding is still matching "list" so embedding logs are treated as unsupported; update the check used to set isEmbedding (and any use in the copy-request-body guard) to match the correct embedding object types (e.g., check log.object === "embedding" and, if applicable, "embedding.chunk") instead of "list" so the embedding serializer runs for embedding logs (update the isEmbedding assignment and any related conditional that references isEmbedding).
🧹 Nitpick comments (1)
ui/app/workspace/providers/fragments/openaiConfigFormFragment.tsx (1)
45-49: Preserve existingopenai_configfields when updating one key.This currently sends only
{ disable_store }. With the helper’s replace semantics, any futureopenai_configkeys could be unintentionally dropped.♻️ Suggested hardening
const updatedProvider = buildProviderUpdatePayload(provider, { openai_config: { + ...provider.openai_config, disable_store: data.disable_store, }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/providers/fragments/openaiConfigFormFragment.tsx` around lines 45 - 49, When building the update payload in buildProviderUpdatePayload, preserve existing provider.openai_config keys instead of replacing the whole object; merge the current provider.openai_config with the new field { disable_store: data.disable_store } so other keys aren’t dropped. Locate the code that constructs updatedProvider (uses provider and openai_config) and change it to shallow-merge provider.openai_config with the new partial update before passing to buildProviderUpdatePayload so only the disable_store field is updated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/app/workspace/logs/views/columns.tsx`:
- Around line 13-26: getMessageFromContent currently only treats blocks with
type === "text" as carrying visible text, so structured blocks like "input_text"
and "output_text" are ignored; update getMessageFromContent to treat any block
that has a truthy block.text as text-bearing (or explicitly include "input_text"
and "output_text") and collect all such block.text values (e.g., push into an
array) and return them joined (e.g., with a space) instead of only returning the
last matching block; update references to lastTextContentBlock in the
getMessageFromContent function accordingly.
In `@ui/components/filters/filterPopover.tsx`:
- Around line 205-207: The parent_request_id Input currently calls
onFilterChange on every keystroke (Input value={filters.parent_request_id}
onChange={(e) => onFilterChange("parent_request_id", e.target.value)}), which
spams navigation/refetch; change this to use a local draft state for this field
(e.g., component state draftParentRequestId) and bind Input to that draft,
update the draft on every onChange, and only call
onFilterChange("parent_request_id", draft) when the user presses Enter
(onKeyDown) or onBlur; alternatively implement a debounced commit that uses
history.replace semantics when committing to avoid backstack clutter. Ensure you
update the Input handlers (onChange, onBlur, onKeyDown) and initialize the draft
from filters.parent_request_id when the popover mounts/shows.
---
Outside diff comments:
In `@ui/app/workspace/logs/views/columns.tsx`:
- Around line 126-260: The createColumns function removed generation of dynamic
metadata_* columns causing those columns to vanish; restore or reintroduce the
metadata column logic by generating metadata columns (those keyed as metadata_*)
and appending them to baseColumns before returning (keep using the existing
ColumnDef<LogEntry> shape), or move that generation into a helper used by
createColumns so logsTable.tsx can continue to show metadata columns by default;
update createColumns (and any helper it calls) to produce the metadata column
definitions and include them in the returned array alongside actionsColumn so
filtering in logsTable remains functional.
---
Duplicate comments:
In `@ui/app/workspace/logs/sheets/logDetailView.tsx`:
- Around line 559-563: The guard variable isEmbedding is still matching "list"
so embedding logs are treated as unsupported; update the check used to set
isEmbedding (and any use in the copy-request-body guard) to match the correct
embedding object types (e.g., check log.object === "embedding" and, if
applicable, "embedding.chunk") instead of "list" so the embedding serializer
runs for embedding logs (update the isEmbedding assignment and any related
conditional that references isEmbedding).
---
Nitpick comments:
In `@ui/app/workspace/providers/fragments/openaiConfigFormFragment.tsx`:
- Around line 45-49: When building the update payload in
buildProviderUpdatePayload, preserve existing provider.openai_config keys
instead of replacing the whole object; merge the current provider.openai_config
with the new field { disable_store: data.disable_store } so other keys aren’t
dropped. Locate the code that constructs updatedProvider (uses provider and
openai_config) and change it to shallow-merge provider.openai_config with the
new partial update before passing to buildProviderUpdatePayload so only the
disable_store field is updated.
🪄 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: 2df281fc-8a0f-4467-8dd3-2f2fba8659bc
📒 Files selected for processing (16)
ui/app/workspace/dashboard/page.tsxui/app/workspace/logs/page.tsxui/app/workspace/logs/sheets/logDetailView.tsxui/app/workspace/logs/sheets/logDetailsSheet.tsxui/app/workspace/logs/sheets/sessionDetailsSheet.tsxui/app/workspace/logs/views/columns.test.tsui/app/workspace/logs/views/columns.tsxui/app/workspace/logs/views/filters.tsxui/app/workspace/logs/views/logsTable.tsxui/app/workspace/providers/fragments/openaiConfigFormFragment.tsxui/app/workspace/providers/views/utils.tsui/components/filters/filterPopover.tsxui/lib/constants/logs.test.tsui/lib/constants/logs.tsui/lib/store/apis/logsApi.tsui/lib/types/logs.ts
✅ Files skipped from review due to trivial changes (3)
- ui/lib/constants/logs.test.ts
- ui/app/workspace/logs/views/columns.test.ts
- ui/app/workspace/logs/sheets/sessionDetailsSheet.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- ui/app/workspace/providers/views/utils.ts
- ui/app/workspace/logs/views/filters.tsx
- ui/lib/constants/logs.ts
- ui/app/workspace/dashboard/page.tsx
- ui/lib/store/apis/logsApi.ts
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_websocket_webrtc_and_client_secret_handlers
branch
from
b91d7ac to
c3a8913
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_browsing_and_log_filtering_ui
branch
from
839db86 to
c28e16f
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
ui/app/workspace/logs/sheets/sessionDetailsSheet.tsx (1)
203-237:⚠️ Potential issue | 🟠 MajorLive merges still drift totals and pagination for unseen logs.
summaryis seeded from whole-session backend totals, so when an off-page log arrives as an"update"andoldLogisundefined, the full cost/token values get added again. The"create"branch also computeshasMorefrom the pre-increment refs, so a fully loaded session can flipLoad Moreback on after appending the new row. Only mutate summary/counts when the id was actually inserted, and derivehasMorefrom the post-insert counts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/logs/sheets/sessionDetailsSheet.tsx` around lines 203 - 237, The live-merge logic updates totals and pagination even when an incoming "update" doesn't insert a new row (oldLog undefined), causing double-counting and incorrect hasMore; modify the block around setSessionLogs/setTotalCount/setFetchedCount/setHasMore/setSummary so you only apply totalCount/fetchedCount/summary deltas when the id was actually inserted (i.e., detect insertion inside setSessionLogs using the same findIndex logic or capture an "inserted" boolean when you build next), and compute hasMore from the post-insert counts (use fetchedCountRef.current and totalCountRef.current after increment or derive from the new counts) rather than the pre-increment refs; keep sortSessionLogs and existing cost/token delta logic but guard it behind the insertion check so updates that don't add a row don't change summary or counts.ui/app/workspace/logs/sheets/logDetailView.tsx (1)
683-689:⚠️ Potential issue | 🟠 Major
copyRequestBodystill excludes actual embedding requests.
isEmbeddingchecks"list", soobject === "embedding"logs still fall through the unsupported-type toast and never serialize their input.🩹 Suggested fix
- const isEmbedding = log.object === "list"; + const isEmbedding = log.object === "embedding";Also applies to: 716-757
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/logs/sheets/logDetailView.tsx` around lines 683 - 689, The embedding branch is using the wrong object value so embedding logs fall through as unsupported; update the log type checks and serialization in copyRequestBody to treat embedding objects correctly by changing the isEmbedding check from "list" to include "embedding" (e.g., const isEmbedding = log.object === "embedding" || log.object === "list") and then ensure copyRequestBody handles embedding logs by serializing their input payload (use the same serialization used for other request types) when isEmbedding is true so embedding requests are copied rather than triggering the unsupported-type toast.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/app/workspace/logs/sheets/sessionDetailsSheet.tsx`:
- Around line 130-168: loadSessionPage can apply stale responses because it
doesn't verify the request context after awaiting triggerGetSession; capture a
local request token (or the current sessionId and sortOrder) at the start of
loadSessionPage (e.g., const requestSession = sessionId; const requestSort =
sortOrder or increment a requestId), then after the await check that the
captured values still match the current sessionId/sortOrder (or that the
requestId is still the latest) before doing any state updates or calling
onOpenChange; bail out early if they differ so older responses cannot overwrite
setSessionLogs, setSummary, setTotalCount, setHasMore, setFetchedCount or call
onOpenChange.
- Around line 183-189: When starting a new session load (the block that calls
setSessionLogs([]), setFetchedCount(0), setTotalCount(0), sets
fetchedCountRef/totalCountRef to 0, setHasMore(false) and calls
loadSessionPage(0, true)), also reset the session summary state by calling the
summary setter (setSummary) to its initial/empty value so the summary cards
don't show stale totals/timestamps if the fetch fails; ensure this reset happens
before invoking loadSessionPage so the UI is cleared immediately.
---
Duplicate comments:
In `@ui/app/workspace/logs/sheets/logDetailView.tsx`:
- Around line 683-689: The embedding branch is using the wrong object value so
embedding logs fall through as unsupported; update the log type checks and
serialization in copyRequestBody to treat embedding objects correctly by
changing the isEmbedding check from "list" to include "embedding" (e.g., const
isEmbedding = log.object === "embedding" || log.object === "list") and then
ensure copyRequestBody handles embedding logs by serializing their input payload
(use the same serialization used for other request types) when isEmbedding is
true so embedding requests are copied rather than triggering the
unsupported-type toast.
In `@ui/app/workspace/logs/sheets/sessionDetailsSheet.tsx`:
- Around line 203-237: The live-merge logic updates totals and pagination even
when an incoming "update" doesn't insert a new row (oldLog undefined), causing
double-counting and incorrect hasMore; modify the block around
setSessionLogs/setTotalCount/setFetchedCount/setHasMore/setSummary so you only
apply totalCount/fetchedCount/summary deltas when the id was actually inserted
(i.e., detect insertion inside setSessionLogs using the same findIndex logic or
capture an "inserted" boolean when you build next), and compute hasMore from the
post-insert counts (use fetchedCountRef.current and totalCountRef.current after
increment or derive from the new counts) rather than the pre-increment refs;
keep sortSessionLogs and existing cost/token delta logic but guard it behind the
insertion check so updates that don't add a row don't change summary or counts.
🪄 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: 78c87a5d-935b-4f4a-9d06-dc2550f59b50
📒 Files selected for processing (16)
ui/app/workspace/dashboard/page.tsxui/app/workspace/logs/page.tsxui/app/workspace/logs/sheets/logDetailView.tsxui/app/workspace/logs/sheets/logDetailsSheet.tsxui/app/workspace/logs/sheets/sessionDetailsSheet.tsxui/app/workspace/logs/views/columns.test.tsui/app/workspace/logs/views/columns.tsxui/app/workspace/logs/views/filters.tsxui/app/workspace/logs/views/logsTable.tsxui/app/workspace/providers/fragments/openaiConfigFormFragment.tsxui/app/workspace/providers/views/utils.tsui/components/filters/filterPopover.tsxui/lib/constants/logs.test.tsui/lib/constants/logs.tsui/lib/store/apis/logsApi.tsui/lib/types/logs.ts
✅ Files skipped from review due to trivial changes (2)
- ui/lib/constants/logs.ts
- ui/app/workspace/logs/views/columns.test.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- ui/app/workspace/logs/views/filters.tsx
- ui/lib/constants/logs.test.ts
- ui/app/workspace/dashboard/page.tsx
- ui/app/workspace/logs/views/logsTable.tsx
- ui/components/filters/filterPopover.tsx
- ui/app/workspace/providers/views/utils.ts
- ui/lib/types/logs.ts
- ui/app/workspace/logs/views/columns.tsx
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_browsing_and_log_filtering_ui
branch
from
c28e16f to
bd6ac46
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_websocket_webrtc_and_client_secret_handlers
branch
from
c3a8913 to
13ed4ad
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/app/workspace/logs/sheets/logDetailsSheet.tsx (1)
56-69:⚠️ Potential issue | 🟠 MajorRender the fallback log when the detail refetch fails.
If
useGetLogByIdQuery()errors,isFullDataReadynever becomes true, so the sheet stays in the loading branch indefinitely even thoughdisplayLogalready falls back to the row data.💡 Suggested change
- {!isFullDataReady ? ( + {!isFullDataReady && !isError ? (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/logs/sheets/logDetailsSheet.tsx` around lines 56 - 69, The sheet currently shows a loader based only on isFullDataReady so when useGetLogByIdQuery() errors the UI never falls back to the row data; change the render condition to rely on displayLog (which already falls back to log) instead of strict isFullDataReady. Concretely, in the Sheet render block replace the check that shows the loader (currently using isFullDataReady) with a check that shows the loader only when there is no displayLog (or when loading and displayLog is undefined) so that when useGetLogByIdQuery() errors the component renders <LogDetailView displayLog={displayLog}> using the row data (references: isFullDataReady, fullLog, log, displayLog, useGetLogByIdQuery, LogDetailView).
♻️ Duplicate comments (1)
ui/app/workspace/logs/views/columns.tsx (1)
20-26:⚠️ Potential issue | 🟡 MinorThis still keeps only the last text block.
getMessageFromContent()now recognizesinput_text/output_text, but it still overwrites earlier segments inside the same content array. Multi-block prompts/responses will truncate to the final segment in both the main table and the session sheet.💡 Suggested change
- let lastTextContentBlock = ""; - for (const block of content) { - if ((block.type === "text" || block.type === "input_text" || block.type === "output_text") && block.text) { - lastTextContentBlock = block.text; - } - } - return lastTextContentBlock; + return content + .filter((block) => block.type === "text" || block.type === "input_text" || block.type === "output_text") + .map((block) => block.text ?? "") + .filter(Boolean) + .join(" ");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/logs/views/columns.tsx` around lines 20 - 26, getMessageFromContent currently only keeps the last matching text segment (it overwrites lastTextContentBlock), so multi-block prompts/responses are truncated; change the logic to accumulate matching block.text values in order (e.g., push them into an array or append with a space/separator) instead of assigning, ensuring you still check block.type === "text" || "input_text" || "output_text" and block.text; return the concatenated string (trimmed) rather than the final single segment so getMessageFromContent, the lastTextContentBlock variable, and any consumers get the full combined message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/app/workspace/logs/sheets/sessionDetailsSheet.tsx`:
- Around line 205-214: The bug is that update events for logs not already in
sessionLogs cause oldLog to be undefined and summary deltas get applied
incorrectly; fix by changing the setSessionLogs/update handling so you only
compute or apply costDelta/tokensDelta when oldLog is found: in the
setSessionLogs callback (the block that finds idx and sets oldLog), keep the
current append behavior for idx < 0 (return sortSessionLogs([...prev, log])) but
ensure any summary-delta adjustments are guarded with a check like if (oldLog
!== undefined) so unseen updates are treated as new adds without applying
summary deltas; apply the same guard in the analogous code path around the lines
222-237.
- Around line 216-220: The current live "create" handler increments fetchedCount
(via setFetchedCount) which wrongly shifts the paging index and causes
loadSessionPage(fetchedCount) to skip historical rows; modify the "create"
branch so it only updates totalCount (setTotalCount) and adjusts hasMore based
on refs, but do NOT increment fetchedCount—use fetchedCountRef.current <
totalCountRef.current (or fetchedCountRef.current < totalCountRef.current + 1 if
you intentionally want the +1 behavior) when calling setHasMore; update the
block around setTotalCount, setFetchedCount, and setHasMore (the symbols to
change) accordingly.
---
Outside diff comments:
In `@ui/app/workspace/logs/sheets/logDetailsSheet.tsx`:
- Around line 56-69: The sheet currently shows a loader based only on
isFullDataReady so when useGetLogByIdQuery() errors the UI never falls back to
the row data; change the render condition to rely on displayLog (which already
falls back to log) instead of strict isFullDataReady. Concretely, in the Sheet
render block replace the check that shows the loader (currently using
isFullDataReady) with a check that shows the loader only when there is no
displayLog (or when loading and displayLog is undefined) so that when
useGetLogByIdQuery() errors the component renders <LogDetailView
displayLog={displayLog}> using the row data (references: isFullDataReady,
fullLog, log, displayLog, useGetLogByIdQuery, LogDetailView).
---
Duplicate comments:
In `@ui/app/workspace/logs/views/columns.tsx`:
- Around line 20-26: getMessageFromContent currently only keeps the last
matching text segment (it overwrites lastTextContentBlock), so multi-block
prompts/responses are truncated; change the logic to accumulate matching
block.text values in order (e.g., push them into an array or append with a
space/separator) instead of assigning, ensuring you still check block.type ===
"text" || "input_text" || "output_text" and block.text; return the concatenated
string (trimmed) rather than the final single segment so getMessageFromContent,
the lastTextContentBlock variable, and any consumers get the full combined
message.
🪄 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: a8590533-e35c-4563-8d73-a2c34c4ee4b4
⛔ Files ignored due to path filters (1)
ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
ui/app/workspace/dashboard/page.tsxui/app/workspace/logs/page.tsxui/app/workspace/logs/sheets/logDetailView.tsxui/app/workspace/logs/sheets/logDetailsSheet.tsxui/app/workspace/logs/sheets/sessionDetailsSheet.tsxui/app/workspace/logs/views/columns.test.tsui/app/workspace/logs/views/columns.tsxui/app/workspace/logs/views/filters.tsxui/app/workspace/logs/views/logsTable.tsxui/app/workspace/providers/fragments/openaiConfigFormFragment.tsxui/app/workspace/providers/views/utils.tsui/components/filters/filterPopover.tsxui/lib/constants/logs.test.tsui/lib/constants/logs.tsui/lib/store/apis/logsApi.tsui/lib/types/logs.ts
✅ Files skipped from review due to trivial changes (3)
- ui/app/workspace/providers/fragments/openaiConfigFormFragment.tsx
- ui/lib/constants/logs.ts
- ui/app/workspace/logs/sheets/logDetailView.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- ui/lib/constants/logs.test.ts
- ui/app/workspace/providers/views/utils.ts
- ui/app/workspace/dashboard/page.tsx
- ui/app/workspace/logs/views/columns.test.ts
- ui/app/workspace/logs/views/logsTable.tsx
- ui/components/filters/filterPopover.tsx
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_websocket_webrtc_and_client_secret_handlers
branch
from
13ed4ad to
d86f3f5
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_browsing_and_log_filtering_ui
branch
from
bd6ac46 to
9f9abc4
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_browsing_and_log_filtering_ui
branch
from
72339b5 to
9c9d36e
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_browsing_and_log_filtering_ui
branch
from
9c9d36e to
8c3e708
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_websocket_webrtc_and_client_secret_handlers
branch
2 times, most recently
from
7ed9dd7 to
29fc633
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_browsing_and_log_filtering_ui
branch
2 times, most recently
from
37d588c to
d30e179
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_websocket_webrtc_and_client_secret_handlers
branch
from
29fc633 to
c1558b5
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_websocket_webrtc_and_client_secret_handlers
branch
from
c1558b5 to
b25fc77
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_browsing_and_log_filtering_ui
branch
2 times, most recently
from
8448b2e to
67cde0e
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_websocket_webrtc_and_client_secret_handlers
branch
from
b25fc77 to
5b980ac
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_websocket_webrtc_and_client_secret_handlers
branch
from
5b980ac to
5e43323
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_browsing_and_log_filtering_ui
branch
from
67cde0e to
3cb15fa
Compare
danpiths
force-pushed
the
feature/03-27-feat_add_realtime_session_browsing_and_log_filtering_ui
branch
from
3cb15fa to
7799ba5
Compare
Contributor
Merge activity
|
akshaydeo
changed the base branch from
feature/03-27-feat_add_realtime_websocket_webrtc_and_client_secret_handlers
to
graphite-base/2342
akshaydeo
dismissed
coderabbitai[bot]’s stale review
The base branch was changed.
| sessionId={selectedSessionId} | ||
| highlightedLogId={sessionHighlightedLogId} | ||
| open={selectedSessionId !== null} | ||
| onOpenChange={handleSessionSheetOpenChange} |
Contributor
There was a problem hiding this comment.
handleSessionSheetOpenChange is undefined — runtime crash on sheet close
handleSessionSheetOpenChange is passed as onOpenChange but is never declared anywhere in this file (no useCallback, no plain function, no import). TypeScript will fail to compile, and at runtime any code path that triggers onOpenChange(false) — pressing Escape, clicking outside the sheet, or the early-exit in loadSessionPage when a session returns zero logs — will throw a ReferenceError, crashing the logs page. The analogous handler for LogDetailSheet at line 924 uses an inline arrow function directly; this prop is the only one left dangling.
Add a definition before the return statement:
const handleSessionSheetOpenChange = useCallback(
(open: boolean) => {
if (!open) {
setSelectedSessionId(null);
setSessionHighlightedLogId(null);
}
},
[],
);
akshaydeo
deleted the
feature/03-27-feat_add_realtime_session_browsing_and_log_filtering_ui
branch
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
3 participants
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.
Summary
Adds the frontend for realtime session browsing and log filtering. Introduces a
session details sheet with summary polling, extracts log detail rendering into a
reusable component, adds parent_request_id filtering across the logs page, and
supports
realtime.turndisplay in columns with multi-role message rendering.Changes
server-side
getLogSessionSummaryByIdpolling (5s interval). Summary cardsshow logs count, cost, tokens, timestamps, and duration from polled data
logDetailsSheet.tsxas pure displaycomponent with parent request ID clickable link for session navigation. Fixed
overflow-x-hiddencausing 0-height header, addeddata-testidattributesto interactive elements
LogDetailViewLogMessageCellshared component,getRealtimeTurnMessagesfor multi-role display (Tool/User/Assistant),removed
metadataKeysparameter fromcreateColumnshandleFilterByParentRequestIdcallback, wired
SessionDetailsSheetand session navigationparent_request_idtext inputfilter,
showParentRequestIdFilterpropgetLogSessionByIdandgetLogSessionSummaryByIdqueryendpoints
LogSessionSummaryResponseinterface, addedparent_request_idtoLogEntry/LogFiltersrealtime.turntoRequestTypes,RequestTypeLabels,RequestTypeColorsType of change
Affected areas
How to test
Screenshots/Recordings
N/A — UI changes require running application
Breaking changes
Related issues
N/A
Security considerations
No new security implications — all data flows through existing authenticated API
endpoints.
Checklist
docs/contributing/README.mdand followed the guidelines