feat(intake): reduce data loaded for nemo studio trace views#558
Conversation
Signed-off-by: Brian Newsom <[email protected]>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughAdds ChangesBackend session/span mode and truncation
Studio trace-linked span navigation
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
web/packages/studio/src/components/IntakeDetail/IntakeComponents/spanKeyValues.test.tsx (1)
132-145: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing coverage for the actual
parent_span_idchange.This new test asserts on
trace_id, but the segment that changed in this cohort isparent_span_id's resolve logic (spanKeyValues.tsx Lines 200-209), including the new raw-text fallback whentrace_idis absent. No test here exercises that branch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/studio/src/components/IntakeDetail/IntakeComponents/spanKeyValues.test.tsx` around lines 132 - 145, The new test is covering the wrong field: it checks trace_id instead of the updated parent_span_id resolution in spanKeyValues.tsx. Add or बदल the test in spanKeyValues.test.tsx to exercise the parent_span_id branch in buildSpanSummaryEntries, including the new raw-text fallback when trace_id is missing, and assert the rendered value/route for that specific behavior.web/packages/studio/src/components/IntakeDetail/IntakeComponents/spanKeyValues.tsx (1)
200-209: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
!parent_span_idguard.
include: (span) => isMeaningfulValue(span.parent_span_id)(Line 210) already filters out entries whereparent_span_idis falsy, so theif (!span.parent_span_id) return EMPTY_VALUE;branch inresolve(Line 201) is effectively unreachable. Harmless, but dead code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/studio/src/components/IntakeDetail/IntakeComponents/spanKeyValues.tsx` around lines 200 - 209, The resolve logic in spanKeyValues has a redundant parent_span_id null check because include already filters those entries via isMeaningfulValue(span.parent_span_id). Remove the dead if (!span.parent_span_id) return EMPTY_VALUE; branch from the resolve callback and keep the existing Link/span.parent_span_id rendering behavior unchanged.services/intake/src/nmp/intake/api/v2/experiments/endpoints.py (1)
659-665: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded "1000" in query description will drift from the constant.
EXPERIMENT_SESSION_SUMMARY_INPUT_CHAR_LIMITis imported here but the description text hardcodes the value instead of interpolating it, unlikeschemas.py'sinputfield description which uses an f-string.♻️ Proposed fix
mode: ExperimentSessionMode = Query( default="detailed", description=( - "Response payload mode. summary keeps the same session row fields but truncates root-span input " - "to 1000 characters; detailed returns the full root-span input." + "Response payload mode. summary keeps the same session row fields but truncates root-span input " + f"to {EXPERIMENT_SESSION_SUMMARY_INPUT_CHAR_LIMIT} characters; detailed returns the full root-span input." ), ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/intake/src/nmp/intake/api/v2/experiments/endpoints.py` around lines 659 - 665, The Query description for the experiment session mode hardcodes the summary input truncation length, which can drift from the imported constant. Update the description in the endpoint definition that uses Query for mode to interpolate EXPERIMENT_SESSION_SUMMARY_INPUT_CHAR_LIMIT instead of embedding a literal, matching the f-string approach used in schemas.py for the input field description. Keep the wording the same otherwise and ensure the constant remains the single source of truth.web/packages/studio/src/components/dataViews/ExperimentSessionsDataView/index.tsx (1)
48-53: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBrittle exact-string match for backend error detection.
Fallback only triggers on an exact match of
'Unsupported query parameter(s): mode'. Any backend wording/formatting change (different layer in this PR stack) silently breaks the fallback, turning a graceful degradation into a hard failure during rollout skew.♻️ Loosen the match
- const detail = (error.response.data as { detail?: unknown } | undefined)?.detail; - return typeof detail === 'string' && detail === 'Unsupported query parameter(s): mode'; + const detail = (error.response.data as { detail?: unknown } | undefined)?.detail; + return typeof detail === 'string' && detail.includes('mode');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/packages/studio/src/components/dataViews/ExperimentSessionsDataView/index.tsx` around lines 48 - 53, The unsupported-mode detection in isUnsupportedModeError is too brittle because it depends on an exact backend error string. Loosen the check so it still recognizes the same fallback case when the message wording varies slightly, while keeping the axios status and detail-based guard in place; update the matching logic inside isUnsupportedModeError in ExperimentSessionsDataView accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/packages/studio/src/components/IntakeDetail/TraceSpanAccordions.tsx`:
- Around line 264-277: The deep-link fallback state in linkedSpanEmptyContent is
only shown by SpanTreeView, so SpanListView still hides loading/error/mismatch
feedback for linked spans outside the loaded page. Update the list-view path in
TraceSpanAccordions to render the same linked-span status UI (spinner,
ErrorMessage, or mismatch notice) in whatever top-of-list/banner slot
SpanListView supports, using linkedSpanEmptyContent and the existing linked span
state variables so deep links remain visible in both view modes.
In `@web/packages/studio/src/components/IntakeLists/IntakeSpansTable.tsx`:
- Around line 140-144: The row click logic in IntakeSpansTable is now a silent
no-op when span.trace_id is missing, while the row still appears clickable.
Update the table row behavior in IntakeSpansTable so rows without trace_id are
either not registered with an onRowClick handler (or otherwise marked
non-clickable) or fall back to getIntakeSpanRoute instead of doing nothing. Use
the existing navigate and getIntakeTraceSpanRoute path for trace-backed rows,
and ensure the clickable affordance matches the actual behavior for each
SpanTableRow.
---
Nitpick comments:
In `@services/intake/src/nmp/intake/api/v2/experiments/endpoints.py`:
- Around line 659-665: The Query description for the experiment session mode
hardcodes the summary input truncation length, which can drift from the imported
constant. Update the description in the endpoint definition that uses Query for
mode to interpolate EXPERIMENT_SESSION_SUMMARY_INPUT_CHAR_LIMIT instead of
embedding a literal, matching the f-string approach used in schemas.py for the
input field description. Keep the wording the same otherwise and ensure the
constant remains the single source of truth.
In
`@web/packages/studio/src/components/dataViews/ExperimentSessionsDataView/index.tsx`:
- Around line 48-53: The unsupported-mode detection in isUnsupportedModeError is
too brittle because it depends on an exact backend error string. Loosen the
check so it still recognizes the same fallback case when the message wording
varies slightly, while keeping the axios status and detail-based guard in place;
update the matching logic inside isUnsupportedModeError in
ExperimentSessionsDataView accordingly.
In
`@web/packages/studio/src/components/IntakeDetail/IntakeComponents/spanKeyValues.test.tsx`:
- Around line 132-145: The new test is covering the wrong field: it checks
trace_id instead of the updated parent_span_id resolution in spanKeyValues.tsx.
Add or बदल the test in spanKeyValues.test.tsx to exercise the parent_span_id
branch in buildSpanSummaryEntries, including the new raw-text fallback when
trace_id is missing, and assert the rendered value/route for that specific
behavior.
In
`@web/packages/studio/src/components/IntakeDetail/IntakeComponents/spanKeyValues.tsx`:
- Around line 200-209: The resolve logic in spanKeyValues has a redundant
parent_span_id null check because include already filters those entries via
isMeaningfulValue(span.parent_span_id). Remove the dead if
(!span.parent_span_id) return EMPTY_VALUE; branch from the resolve callback and
keep the existing Link/span.parent_span_id rendering behavior unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 89dc0808-6e8e-4850-9e93-11ff67b9208f
⛔ Files ignored due to path filters (7)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/resources/experiments/sessions.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiments/experiment_session_response.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/experiments/session_list_params.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/intake/span.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/experiments/test_sessions.pyis excluded by!sdk/**web/packages/sdk/generated/agents/schema/DeploymentLogsResponse.tsis excluded by!**/generated/**
📒 Files selected for processing (40)
openapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlservices/intake/src/nmp/intake/api/v2/experiments/endpoints.pyservices/intake/src/nmp/intake/api/v2/experiments/schemas.pyservices/intake/src/nmp/intake/spans/api/spans_schemas.pyservices/intake/src/nmp/intake/spans/experiment_session_repository.pyservices/intake/tests/test_experiment_session_schemas.pyservices/intake/tests/test_spans_schemas.pyservices/studio/src/nmp/studio/studio_links.pyservices/studio/tests/unit/test_coding_agents.pyweb/packages/studio/src/components/IntakeDetail/IntakeComponents/AnnotationsPanel.test.tsxweb/packages/studio/src/components/IntakeDetail/IntakeComponents/SpanPayloadBlock.test.tsxweb/packages/studio/src/components/IntakeDetail/IntakeComponents/SpanPayloadBlock.tsxweb/packages/studio/src/components/IntakeDetail/IntakeComponents/spanKeyValues.test.tsxweb/packages/studio/src/components/IntakeDetail/IntakeComponents/spanKeyValues.tsxweb/packages/studio/src/components/IntakeDetail/IntakeComponents/traceKeyValues.tsxweb/packages/studio/src/components/IntakeDetail/README.mdweb/packages/studio/src/components/IntakeDetail/SpanDetailView.tsxweb/packages/studio/src/components/IntakeDetail/SpanTemplates/EvaluatorSpanTemplate.tsweb/packages/studio/src/components/IntakeDetail/SpanTemplates/GuardrailSpanTemplate.tsweb/packages/studio/src/components/IntakeDetail/TraceSpanAccordionContent.tsxweb/packages/studio/src/components/IntakeDetail/TraceSpanAccordions.tsxweb/packages/studio/src/components/IntakeDetail/TraceSpanTreeView.tsxweb/packages/studio/src/components/IntakeLists/IntakeSpansTable.test.tsxweb/packages/studio/src/components/IntakeLists/IntakeSpansTable.tsxweb/packages/studio/src/components/IntakeLists/IntakeTracesTable.test.tsxweb/packages/studio/src/components/dataViews/ExperimentSessionsDataView/index.test.tsxweb/packages/studio/src/components/dataViews/ExperimentSessionsDataView/index.tsxweb/packages/studio/src/constants/routes.tsweb/packages/studio/src/routes/IntakeSpanDetailRoute/index.tsxweb/packages/studio/src/routes/IntakeTraceDetailRoute/index.test.tsxweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/artifacts.test.tsweb/packages/studio/src/routes/agents/ClaudeCodeChatRoute/artifacts.tsweb/packages/studio/src/routes/constants.tsweb/packages/studio/src/routes/groups/intakeRoutes.tsxweb/packages/studio/src/routes/index.test.tsxweb/packages/studio/src/routes/utils.test.tsweb/packages/studio/src/routes/utils.tsweb/packages/studio/src/tests/title-change.test.tsx
💤 Files with no reviewable changes (6)
- web/packages/studio/src/routes/IntakeSpanDetailRoute/index.tsx
- web/packages/studio/src/components/IntakeDetail/SpanDetailView.tsx
- web/packages/studio/src/tests/title-change.test.tsx
- web/packages/studio/src/constants/routes.ts
- web/packages/studio/src/routes/groups/intakeRoutes.tsx
- web/packages/studio/src/routes/index.test.tsx
|
Signed-off-by: Brian Newsom <[email protected]>
Signed-off-by: Brian Newsom <[email protected]>
Signed-off-by: Brian Newsom <[email protected]>
* feat(intake): reduce data loaded for nemo studio trace views Signed-off-by: Brian Newsom <[email protected]> * fix(intake): cleanup and coderabbit fixes Signed-off-by: Brian Newsom <[email protected]> * chore(intake): lint Signed-off-by: Brian Newsom <[email protected]> * chore(intake): lint Signed-off-by: Brian Newsom <[email protected]> --------- Signed-off-by: Brian Newsom <[email protected]>
Summary by CodeRabbit
modeoption for experiment session listings (summaryvsdetailed), withsummarytruncating root-span input and span error messages.summaryisn’t supported by the backend.