Temp branch#11934
Closed
Adam-Aghili wants to merge 33 commits into
Closed
Conversation
v0 for traces includes: - filters: status, token usage range and datatime - accordian rows per trace Could add: - more filter options. Ecamples: session_id, trace_id and latency range
add sidebar buttons for logs and trace remove lods canvas control
hopefully fix duplicate trace ID insertion on windows
update tests and alembic tables for uts
was flow_name - trace_id now flow_name - flow_id
address gabriel simple changes in traces.py and native.py
model name is now set using name = f"{operation} {model_name}" if model_name else operation
address cris I1 comment
address cris I5
address cris I6
address cris R7
Contributor
WalkthroughThis PR implements comprehensive native tracing support by introducing database models and migrations for Trace/Span tables, a persistence-focused tracer service with async database writes, API endpoints for trace retrieval and management, and an extensive frontend UI for viewing traces with hierarchical span visualization, filtering, and sidebar integration. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant Tracer as NativeTracer
participant LC as LangChain
participant Handler as NativeCallbackHandler
participant DB as Database
participant API as Traces API
participant Frontend as Frontend UI
App->>Tracer: initialize(flow_id, trace_name)
App->>Tracer: add_trace(component_id, inputs)
Tracer->>Tracer: store in-memory span
LC->>Handler: on_llm_start (via callback)
Handler->>Tracer: add_langchain_span(span_id, model)
Handler->>Tracer: accumulate tokens
LC->>Handler: on_llm_end (outputs, tokens)
Handler->>Tracer: end_langchain_span(span_id, outputs)
App->>Tracer: end_trace(component_id, outputs)
Tracer->>Tracer: compute latency, aggregate spans
Tracer->>Tracer: schedule async flush
Tracer->>DB: _flush_to_database (persist TraceTable + SpanTable)
DB-->>Tracer: write complete
Frontend->>API: GET /v1/traces?flow_id=...
API->>DB: query traces with spans
DB-->>API: trace records
API-->>Frontend: paginated traces + span tree
Frontend->>Frontend: render trace accordion, span details
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 3 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
Contributor
|
✅ Migration Validation Passed All migrations follow the Expand-Contract pattern correctly. |
Codecov Report❌ Patch coverage is ❌ Your project status has failed because the head coverage (42.40%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #11934 +/- ##
==========================================
+ Coverage 35.71% 35.74% +0.02%
==========================================
Files 1528 1532 +4
Lines 73960 74562 +602
Branches 11154 11146 -8
==========================================
+ Hits 26417 26651 +234
- Misses 46106 46475 +369
+ Partials 1437 1436 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![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: 7
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.secrets.baseline (1)
2158-2179:⚠️ Potential issue | 🟠 MajorFix stale filename mapping under the new
input_mixin.pyresults key.At Line [2158], the results key is
src/backend/base/langflow/inputs/input_mixin.py, but nested entries still usesrc/backend/base/langflow/initial_setup/starter_projects/Youtube Analysis.json(Lines [2161], [2168], [2175]). This makes the baseline internally inconsistent and can break/blur secret allowlist matching.🔧 Proposed fix
"src/backend/base/langflow/inputs/input_mixin.py": [ { "type": "Secret Keyword", - "filename": "src/backend/base/langflow/initial_setup/starter_projects/Youtube Analysis.json", + "filename": "src/backend/base/langflow/inputs/input_mixin.py", "hashed_secret": "665b1e3851eefefa3fb878654292f16597d25155", "is_verified": false, "line_number": 832 }, { "type": "Hex High Entropy String", - "filename": "src/backend/base/langflow/initial_setup/starter_projects/Youtube Analysis.json", + "filename": "src/backend/base/langflow/inputs/input_mixin.py", "hashed_secret": "35be14614e83fe56d9b2ca1c0e2c2a74890b6889", "is_verified": false, "line_number": 1443 }, { "type": "Hex High Entropy String", - "filename": "src/backend/base/langflow/initial_setup/starter_projects/Youtube Analysis.json", + "filename": "src/backend/base/langflow/inputs/input_mixin.py", "hashed_secret": "54ed260e3bc31bc77ee06754dff850981d39a66c", "is_verified": false, "line_number": 2195 } ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.secrets.baseline around lines 2158 - 2179, The baseline has inconsistent filename mappings: the outer results key uses "src/backend/base/langflow/inputs/input_mixin.py" but nested entries still reference "src/backend/base/langflow/initial_setup/starter_projects/Youtube Analysis.json"; update each nested entry under that results key to use the new filename "src/backend/base/langflow/inputs/input_mixin.py" (ensure the three affected hashed_secret entries are updated), and verify there are no other stale mappings under the same results key so secret allowlist matching remains consistent.src/frontend/src/components/ui/sidebar.tsx (1)
57-64:⚠️ Potential issue | 🟡 MinorCookie validation doesn't include the new sidebar sections.
getInitialSidebarSectionvalidates cookies against a hardcoded list ("search","components","bundles","mcp") but doesn't include"add_note","logs", or"traces". If a user navigates to these sections and they're persisted to a cookie, the state won't be restored correctly on page load.🔧 Suggested fix
function getInitialSidebarSection( defaultSection: SidebarSection, ): SidebarSection { const cookieValue = getCookie(SIDEBAR_SECTION_COOKIE_NAME); if (cookieValue === null) return defaultSection; if ( cookieValue === "search" || cookieValue === "components" || cookieValue === "bundles" || - cookieValue === "mcp" + cookieValue === "mcp" || + cookieValue === "add_note" || + cookieValue === "logs" || + cookieValue === "traces" ) { return cookieValue; } return defaultSection; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/components/ui/sidebar.tsx` around lines 57 - 64, Update getInitialSidebarSection to recognize the new sidebar keys so persisted cookies restore correctly: include "add_note", "logs", and "traces" alongside the existing values when validating cookieValue in the conditional (the block that currently checks for "search", "components", "bundles", "mcp"); ensure the function still returns cookieValue when it matches any of these allowed section names and falls back to the current default otherwise.src/lfx/src/lfx/services/tracing/service.py (1)
53-62:⚠️ Potential issue | 🟡 MinorDocstring argument order doesn't match method signature.
The docstring lists
flow_idbeforerun_name, but the method signature hasflow_idas the last parameter. This could confuse developers reading the documentation.📝 Suggested fix
"""Start tracers (minimal implementation - just logs). Args: run_id: Run identifier run_name: Run name user_id: User identifier session_id: Session identifier project_name: Project name + flow_id: Flow identifier - flow_id: Flow identifier """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lfx/src/lfx/services/tracing/service.py` around lines 53 - 62, The docstring for the Start tracers function does not match the method signature ordering; update the Args block in the start_tracers function's docstring so the parameter list matches the actual signature (put flow_id last, after project_name) and ensure each parameter name and short description exactly matches the function signature (run_id, run_name, user_id, session_id, project_name, flow_id) to avoid confusion.
🟡 Minor comments (14)
src/frontend/src/modals/flowLogsModal/components/FlowInsightsContent.tsx-226-232 (1)
226-232:⚠️ Potential issue | 🟡 MinorSearch input icon overlaps with text.
The search icon is positioned absolutely at
left-2, but theInputcomponent doesn't have padding-left to account for it. Users may see text overlapping with the search icon.Proposed fix
<Input value={searchText} onChange={(e) => setSearchText(e.target.value)} placeholder="Search runs..." - className="h-8 text-sm" + className="h-8 pl-8 text-sm" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/modals/flowLogsModal/components/FlowInsightsContent.tsx` around lines 226 - 232, The search icon is absolutely positioned at left-2 and the Input in FlowInsightsContent lacks left padding, causing overlap; update the Input usage (the JSX with value={searchText} onChange={...} placeholder="Search runs..." className="h-8 text-sm") to include left padding (e.g., add pl-8 or pl-7 to the className) so typed text does not overlap the icon and keep the existing icon positioning intact.src/frontend/src/controllers/API/queries/traces/__tests__/helpers.test.ts-54-54 (1)
54-54:⚠️ Potential issue | 🟡 MinorTest uses invalid status value.
The test data uses
status: "success"butSpanStatusonly allows"unset" | "ok" | "error". This should be"ok"to match the type definition. The same issue occurs on lines 73, 104, 127, and 147.Proposed fix
const apiSpan: SpanApiResponse = { id: "root", name: "Root", type: "chain", - status: "success", + status: "ok", // ...Apply similar fixes to all test data using
"success"or"running"- replace with valid values like"ok"or"unset".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/controllers/API/queries/traces/__tests__/helpers.test.ts` at line 54, Update the test fixtures in src/frontend/src/controllers/API/queries/traces/__tests__/helpers.test.ts to use valid SpanStatus values instead of "success" or "running": locate the test data objects (the entries that set status: "success"/"running") and replace those strings with "ok" (or "unset" where appropriate); ensure all occurrences mentioned (around the current lines and the other failing entries) are changed so every status conforms to the SpanStatus union ("unset" | "ok" | "error") used by the code under test.src/backend/base/langflow/services/tracing/native.py-71-72 (1)
71-72:⚠️ Potential issue | 🟡 MinorFallback
flow_idextraction may silently lose trace data.When
flow_idis not provided, the fallback extracts fromtrace_nameor uses the entiretrace_name. If this doesn't produce a valid UUID,_flush_to_database(lines 264-268) logs a warning and returns early, silently discarding all trace data.Consider validating or requiring
flow_idat construction time, or using a more robust fallback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend/base/langflow/services/tracing/native.py` around lines 71 - 72, The constructor's fallback assignment to self.flow_id can produce a non-UUID and later cause silent drops in _flush_to_database; update the __init__ logic that sets self.flow_id to validate the resulting value using uuid.UUID and if it is not a valid UUID either raise a clear ValueError (requiring callers to provide a valid flow_id) or, alternatively, generate a new UUID and log a warning—make the change in the code that currently sets self.flow_id (the fallback from trace_name) and ensure _flush_to_database will no longer silently return by enforcing a valid UUID at construction time.src/backend/base/langflow/alembic/versions/a1b2c3d4e5f6_add_trace_and_span_tables.py-107-108 (1)
107-108:⚠️ Potential issue | 🟡 MinorPostgreSQL-specific
DROP TYPEmay fail on SQLite.The
DROP TYPE IF EXISTSsyntax is PostgreSQL-specific. SQLite doesn't have a separate type system for enums (SQLAlchemy emulates enums as VARCHAR with constraints). This could cause the downgrade to fail on SQLite deployments.🔧 Suggested fix: wrap in dialect check
# Drop enums - op.execute("DROP TYPE IF EXISTS spantype") - op.execute("DROP TYPE IF EXISTS spanstatus") + # Only drop enum types on PostgreSQL (SQLite doesn't have them) + if conn.dialect.name == "postgresql": + op.execute("DROP TYPE IF EXISTS spantype") + op.execute("DROP TYPE IF EXISTS spanstatus")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend/base/langflow/alembic/versions/a1b2c3d4e5f6_add_trace_and_span_tables.py` around lines 107 - 108, The downgrade currently runs PostgreSQL-specific SQL via op.execute("DROP TYPE IF EXISTS spantype") and op.execute("DROP TYPE IF EXISTS spanstatus"), which will fail on SQLite; change the downgrade to first check the current dialect (use op.get_bind().dialect.name or similar) and only execute those DROP TYPE statements when the dialect is 'postgresql', skipping them for SQLite (or other dialects) to avoid executing DB-specific SQL.src/frontend/src/controllers/API/queries/traces/__tests__/use-get-trace.test.ts-40-65 (1)
40-65:⚠️ Potential issue | 🟡 MinorPlease add brief scenario comments per test.
The tests are readable, but they still need the explicit per-test purpose/scenario/expected-outcome comment required by project testing guidance.
As per coding guidelines, "Document each frontend test with a clear docstring/comment explaining its purpose, the scenario being tested, and expected outcomes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/controllers/API/queries/traces/__tests__/use-get-trace.test.ts` around lines 40 - 65, Add concise docstring-style comments above each test in the useGetTraceQuery suite describing the scenario and expected outcome: for the first test (it "returns null when traceId is missing") add a line like "Scenario: empty traceId; Expectation: useGetTraceQuery does not call API (mockApiGet) and yields no data", and for the second test (it "calls API and converts response") add a line like "Scenario: valid traceId provided; Expectation: calls API endpoint '/api/v1/traces/:id' via mockApiGet and passes response to mockConvertTrace for conversion". Place these comments immediately before each it block so reviewers can quickly see the intent of useGetTraceQuery, mockApiGet, and mockConvertTrace interactions.src/frontend/src/modals/flowLogsModal/components/TraceView/__tests__/SpanTree.test.tsx-46-129 (1)
46-129:⚠️ Potential issue | 🟡 MinorAdd a short purpose comment above each test case.
These tests are solid functionally, but they’re missing the per-test explanatory comment required by the repo testing guideline.
As per coding guidelines, "Document each frontend test with a clear docstring/comment explaining its purpose, the scenario being tested, and expected outcomes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/modals/flowLogsModal/components/TraceView/__tests__/SpanTree.test.tsx` around lines 46 - 129, Add a one-line purpose comment above each test case in the SpanTree suite: place a short docstring-style comment before each it(...) block in the describe("SpanTree", ...) file explaining what the test verifies (e.g., "renders tree and expands root spans by default" before the first it, "collapses and expands children when toggled" before the second, "calls onSelectSpan with the clicked span" before the third, and "marks the selected span via aria-selected" before the fourth). Keep each comment concise (one sentence), reference the scenario and expected outcome, and add them immediately above the corresponding it(...) blocks (e.g., above the it("renders a tree...", ...) etc.) so they follow the repo testing guideline.src/frontend/src/modals/flowLogsModal/components/TraceView/__tests__/SpanDetail.test.tsx-25-30 (1)
25-30:⚠️ Potential issue | 🟡 MinorDefault test fixture status should match runtime status vocabulary.
Using
"success"as the default status makes the baseline fixture diverge from the"ok" | "error" | "unset"paths exercised by the trace helpers/components.Suggested fixture alignment
- status: "success", + status: "ok",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/modals/flowLogsModal/components/TraceView/__tests__/SpanDetail.test.tsx` around lines 25 - 30, The test fixture buildSpan currently sets status: "success" which doesn't match the runtime status union ("ok" | "error" | "unset"); update the default in buildSpan (the helper that returns a Span) to use a runtime-valid value (e.g., status: "ok") so tests align with the Trace helpers/components and downstream assertions that expect "ok"|"error"|"unset".src/frontend/src/controllers/API/queries/traces/use-get-traces.ts-15-20 (1)
15-20:⚠️ Potential issue | 🟡 MinorGuard on sanitized
flowId, not raw truthiness.A whitespace/control-only
flowIdcan pass the current guard and still produce an emptyflow_idrequest parameter after sanitization.Suggested fix
const getTracesFn = async (): Promise<TracesResponse> => { - if (!flowId) return { traces: [], total: 0 }; + const sanitizedFlowId = sanitizeString(flowId ?? ""); + if (!sanitizedFlowId) return { traces: [], total: 0 }; const config: { params: Record<string, unknown> } = { - params: { flow_id: sanitizeString(flowId) }, + params: { flow_id: sanitizedFlowId }, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/controllers/API/queries/traces/use-get-traces.ts` around lines 15 - 20, The guard in getTracesFn uses the raw flowId truthiness so whitespace/control-only values slip through; compute a sanitizedFlowId = sanitizeString(flowId) and guard on that (e.g., if (!sanitizedFlowId) return { traces: [], total: 0 }); then use sanitizedFlowId in the params (params: { flow_id: sanitizedFlowId }) to ensure you never send an empty/whitespace flow_id; update references in getTracesFn accordingly.src/frontend/src/modals/flowLogsModal/components/TraceView/TraceAccordionItem.tsx-46-50 (1)
46-50:⚠️ Potential issue | 🟡 MinorReset selection when refreshed trace data no longer contains the previous span.
At Lines [46-50], selection initializes only when
!selectedSpan; after a refetch, the detail pane can retain a stale span not present in the current tree.Proposed fix
- useEffect(() => { - if (trace?.spans && trace.spans.length > 0 && !selectedSpan) { - setSelectedSpan(trace.spans[0]); - } - }, [trace?.spans, selectedSpan]); + useEffect(() => { + if (!trace?.spans?.length) { + setSelectedSpan(null); + return; + } + setSelectedSpan((prev) => + prev && trace.spans.some((span) => span.id === prev.id) + ? prev + : trace.spans[0], + ); + }, [trace?.spans]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/modals/flowLogsModal/components/TraceView/TraceAccordionItem.tsx` around lines 46 - 50, The effect that initializes selected span (useEffect watching trace?.spans and selectedSpan) only sets selectedSpan when none exists, so after a trace refetch the UI can keep a stale selectedSpan not present in the new trace; update the effect to detect when the current selectedSpan is not in the refreshed trace.spans and reset it (via setSelectedSpan) to either the first span or null, ensuring you reference the same identifiers (useEffect, trace?.spans, selectedSpan, setSelectedSpan) and update the dependency array accordingly so selection is cleared or replaced when trace data no longer contains the previously selected span.src/frontend/src/modals/flowLogsModal/components/TraceView/SpanTree.tsx-21-26 (1)
21-26:⚠️ Potential issue | 🟡 MinorReinitialize expanded root IDs when
spanschanges.Lines [21-26] only run once; when a new trace is loaded,
expandedIdscan keep stale IDs and collapse all new roots unexpectedly.Proposed fix
-import { useCallback, useState } from "react"; +import { useCallback, useEffect, useState } from "react"; @@ const [expandedIds, setExpandedIds] = useState<Set<string>>(() => { const initial = new Set<string>(); // Expand root level spans by default spans.forEach((span) => initial.add(span.id)); return initial; }); + + useEffect(() => { + setExpandedIds(new Set(spans.map((span) => span.id))); + }, [spans]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/modals/flowLogsModal/components/TraceView/SpanTree.tsx` around lines 21 - 26, The initial expandedIds Set is only created once via the useState initializer and isn't updated when the spans prop changes; add a useEffect that watches spans and reinitializes expandedIds (via setExpandedIds) to a new Set containing each span.id so root-level spans are expanded whenever a new trace is loaded, referencing the expandedIds/setExpandedIds state and the spans variable in SpanTree.tsx.src/backend/base/langflow/api/v1/traces.py-255-255 (1)
255-255:⚠️ Potential issue | 🟡 MinorKeep
get_traceserror responses schema-consistent.Error fallbacks omit
pages, while successful responses include it. Returning a consistent shape avoids client-side pagination edge cases.💡 Suggested fix
- return {"traces": [], "total": 0} + return {"traces": [], "total": 0, "pages": 0} ... - return {"traces": [], "total": 0} + return {"traces": [], "total": 0, "pages": 0}Also applies to: 259-259
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend/base/langflow/api/v1/traces.py` at line 255, The error fallback responses in get_traces return {"traces": [], "total": 0} which omits the pages field present in successful responses; update all fallback/exception return paths inside the get_traces function (including the other short-circuit return) to return {"traces": [], "pages": [], "total": 0} so the response schema is consistent for clients expecting pagination fields.src/frontend/src/modals/flowLogsModal/components/TraceView/index.tsx-182-182 (1)
182-182:⚠️ Potential issue | 🟡 MinorFix malformed spacing class in the filter panel container.
Line 182 contains
px-, which is incomplete and drops intended horizontal spacing.💡 Suggested fix
- <div className="border-t border-border bg-muted/30 px- py-3"> + <div className="border-t border-border bg-muted/30 px-3 py-3">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/modals/flowLogsModal/components/TraceView/index.tsx` at line 182, In the TraceView component fix the malformed Tailwind class in the container div (className containing "border-t border-border bg-muted/30 px- py-3") by removing the incomplete "px-" or replacing it with the intended horizontal padding class (e.g., "px-3" or "px-4") so the filter panel gets proper horizontal spacing; update the className in the JSX element inside TraceView accordingly.src/frontend/src/modals/flowLogsModal/components/TraceView/traceViewHelpers.ts-51-55 (1)
51-55:⚠️ Potential issue | 🟡 MinorPreserve explicit
0values informatTokens.
if (!tokens)treats0as missing and returnsnull, which hides valid zero-token results.💡 Suggested fix
export const formatTokens = (tokens: number | undefined): string | null => { - if (!tokens) return null; + if (tokens === undefined) return null; if (tokens < 1000) return `${tokens}`; return `${(tokens / 1000).toFixed(1)}k`; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/modals/flowLogsModal/components/TraceView/traceViewHelpers.ts` around lines 51 - 55, The formatTokens function incorrectly treats 0 as missing because it uses if (!tokens); change that guard to only return null for undefined or null (e.g., tokens == null or typeof tokens === 'undefined') so zero is preserved, then keep the existing formatting logic (tokens < 1000 and thousands formatting) in formatTokens.src/frontend/src/modals/flowLogsModal/components/TraceView/traceViewHelpers.ts-158-174 (1)
158-174:⚠️ Potential issue | 🟡 MinorAvoid rendering unknown/running statuses as an error icon.
Current fallback maps any non-
ok/error/unsetstatus toCircleX, which can mislabel non-error states.💡 Suggested fix
export const getStatusIconProps = ( status: string | null | undefined, ): StatusIconProps => { - const normalized = status ?? ""; - const isOk = normalized === "ok"; - const isError = normalized === "error"; - const isUnset = normalized === "unset"; + const normalized = (status ?? "").toLowerCase(); + const isOk = normalized === "ok" || normalized === "success"; + const isError = normalized === "error" || normalized === "failed"; + const isUnset = normalized === "unset" || normalized === "running"; return { colorClass: isError ? "text-status-red" : isOk ? "text-status-green" : "text-muted-foreground", - iconName: isUnset ? "Loader2" : isOk ? "CircleCheck" : "CircleX", + iconName: isError ? "CircleX" : isOk ? "CircleCheck" : "Loader2", shouldSpin: isUnset, }; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/modals/flowLogsModal/components/TraceView/traceViewHelpers.ts` around lines 158 - 174, getStatusIconProps currently maps any non-ok/error/unset status to the error icon (CircleX); update the logic in getStatusIconProps so only explicit "error" yields iconName "CircleX" and red color, "ok" yields "CircleCheck" and green, "unset" yields "Loader2" and shouldSpin true, and any other/unknown statuses use a neutral icon (e.g., "HelpCircle") and the muted color class; keep shouldSpin false for all except isUnset and adjust the return fields (colorClass, iconName, shouldSpin) accordingly.
🧹 Nitpick comments (22)
src/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/__tests__/sidebarSegmentedNav.test.tsx (1)
5-6: Consider removing unused mocks.
mockNavigateis defined andreact-router-dom/useCustomNavigateare mocked, but I don't see these being used in any test assertions. If these are not needed for the current tests, consider removing them to keep the test file focused.Also applies to: 70-76
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/__tests__/sidebarSegmentedNav.test.tsx` around lines 5 - 6, The test file contains an unused mock function mockNavigate and mocks for react-router-dom/useCustomNavigate that are not asserted against; remove the mockNavigate declaration and any unused jest.mock(...) for useCustomNavigate/react-router-dom (or adjust tests to assert navigation if intended) so the file only includes mocks actually used by tests — search for mockNavigate, useCustomNavigate, and related jest.mock calls in the test to locate and delete the unused mocks or add the appropriate assertions in the tests that rely on navigation.src/frontend/src/utils/dateTime.ts (1)
44-52: Consider using consistent locale-aware formatting.The function uses
Intl.DateTimeFormat(locale-aware) for the time and same-year cases, but hardcodesdd/mm/yyyyformat on line 51 for different-year dates. This creates an inconsistency where US users would see locale-appropriate times but a European date format.♻️ Suggested refactor for consistency
const sameYear = date.getFullYear() === now.getFullYear(); if (sameYear) { return new Intl.DateTimeFormat(undefined, { day: "2-digit", month: "short", }).format(date); } - const ddmmyyyy = `${pad2(date.getDate())}/${pad2(date.getMonth() + 1)}/${date.getFullYear()}`; - return `${ddmmyyyy} ${time}`; + const fullDate = new Intl.DateTimeFormat(undefined, { + day: "2-digit", + month: "2-digit", + year: "numeric", + }).format(date); + return `${fullDate} ${time}`; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/utils/dateTime.ts` around lines 44 - 52, Replace the hardcoded dd/mm/yyyy construction (the ddmmyyyy variable) with a locale-aware Intl.DateTimeFormat call so date formatting is consistent with the other branches; locate the block that checks sameYear and the fallback that creates ddmmyyyy (references: sameYear, ddmmyyyy, and the existing Intl.DateTimeFormat usage) and format the full date using Intl.DateTimeFormat(undefined, { day: "2-digit", month: "short" } or use { day: "2-digit", month: "2-digit", year: "numeric" } depending on desired month style) combined with the existing time string instead of concatenating a hardcoded string.src/frontend/src/modals/flowLogsModal/components/FlowInsightsContent.tsx (1)
64-72: Consider adding display name for debugging.For better React DevTools debugging, consider adding a
displayNameto this exported function component, similar to howFlowSidebarComponentdoes at the end of its file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/modals/flowLogsModal/components/FlowInsightsContent.tsx` around lines 64 - 72, Add a React displayName to the exported function component FlowInsightsContent to improve DevTools debugging: locate the FlowInsightsContent function and add a displayName assignment (e.g., FlowInsightsContent.displayName = 'FlowInsightsContent') after the function definition, mirroring the pattern used by FlowSidebarComponent.src/frontend/src/controllers/API/queries/traces/types.ts (1)
60-65: Consider extracting inline tokenUsage type.The
tokenUsagetype is defined inline here but matches theTokenUsageinterface inTraceView/types.ts. Consider importing and reusing that type for consistency and to avoid duplication.Proposed approach
+import type { TokenUsage } from "@/modals/flowLogsModal/components/TraceView/types"; + export interface SpanApiResponse { // ... other fields - tokenUsage?: { - promptTokens: number; - completionTokens: number; - totalTokens: number; - cost: number; - }; + tokenUsage?: TokenUsage; children: SpanApiResponse[]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/controllers/API/queries/traces/types.ts` around lines 60 - 65, Replace the inline tokenUsage object type with the shared TokenUsage interface from TraceView/types.ts: import the TokenUsage type and change the tokenUsage?: { ... } declaration to tokenUsage?: TokenUsage in the trace types file (remove the duplicated promptTokens/completionTokens/totalTokens/cost fields), ensuring any references to tokenUsage still match the imported interface.src/frontend/src/pages/FlowPage/components/flowSidebarComponent/index.tsx (2)
611-660: DOM selector coupling is fragile.The sidebar animation relies on
document.querySelector(".group\\/sidebar-wrapper")which couples the component to a specific CSS class name. If the class name changes in the Sidebar component, this will silently break.Consider using a React ref passed through context or a more robust approach to access the sidebar wrapper element.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/pages/FlowPage/components/flowSidebarComponent/index.tsx` around lines 611 - 660, The effect in useEffect that queries document.querySelector(".group\\/sidebar-wrapper") is fragile; replace the DOM selector with a ref passed from the Sidebar component via context or props so this component reads wrapper from that ref instead of querying the DOM. Add or consume a SidebarWrapperRef (e.g., SidebarContext.wrapperRef) and use wrapperRef.current in place of the query result, keeping the existing logic that uses expandedSidebarWidthRef, prevShowTracesRef, setIsFullSidebarPanelMounted, and setIsFullSidebarPanelShown; include a safe fallback to the old querySelector only if wrapperRef is null to preserve behavior during migration. Ensure the Sidebar component mounts the DOM node onto the provided ref (attach ref to the element that previously had the .group/sidebar-wrapper class).
601-601: Extract animation constant outside component.
SIDEBAR_EXPAND_ANIMATION_MSis defined inside the component, causing it to be recreated on every render. Move it outside the component scope for better performance and clarity.Proposed fix
+const SIDEBAR_EXPAND_ANIMATION_MS = 300; + export function FlowSidebarComponent({ isLoading }: FlowSidebarComponentProps) { const rawData = useTypesStore((state) => state.data); // ... later in the component - const SIDEBAR_EXPAND_ANIMATION_MS = 300;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/pages/FlowPage/components/flowSidebarComponent/index.tsx` at line 601, Move the SIDEBAR_EXPAND_ANIMATION_MS constant out of the React component so it isn't recreated on every render; locate the constant declaration inside the FlowSidebar component in flowSidebarComponent (symbol: SIDEBAR_EXPAND_ANIMATION_MS) and cut it to the module top-level (above the component/function) so the component imports the value from the same file scope rather than defining it each render, keeping the constant name and value unchanged.src/frontend/src/controllers/API/queries/traces/helpers.ts (2)
15-23:sanitizeParamsdoesn't handle nested objects.The function only sanitizes top-level string values. Nested objects with string properties (e.g.,
{ nested: { query: " hi\n" } }) won't have their strings sanitized. This may be intentional, but worth noting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/controllers/API/queries/traces/helpers.ts` around lines 15 - 23, sanitizeParams currently only sanitizes top-level string values; update it to recursively walk nested objects and arrays so all string properties are sanitized (use or implement a helper that checks for typeof "string" to call sanitizeString, Array.isArray to map over elements, and typeof "object" to recurse). Modify sanitizeParams (and/or extract a recursive helper) to handle Records and nested arrays, preserving non-string primitives and functions, and return a new sanitized structure. Ensure you still call sanitizeString for every string encountered and keep the existing return shape.
29-30: Type assertions bypass type safety.The casts
apiSpan.type as Span["type"]andapiSpan.status as Span["status"]assume the API always returns valid values. If the backend sends an unexpected value (e.g., a new span type), this could cause subtle bugs downstream.Consider adding validation or defaulting to a safe value.
Proposed approach with validation
+const VALID_SPAN_TYPES = new Set(["chain", "llm", "tool", "retriever", "embedding", "parser", "agent"]); +const VALID_STATUSES = new Set(["unset", "ok", "error"]); + function convertSpan(apiSpan: SpanApiResponse): Span { return { id: apiSpan.id, name: apiSpan.name, - type: apiSpan.type as Span["type"], - status: apiSpan.status as Span["status"], + type: VALID_SPAN_TYPES.has(apiSpan.type) ? (apiSpan.type as Span["type"]) : "chain", + status: VALID_STATUSES.has(apiSpan.status) ? (apiSpan.status as Span["status"]) : "unset", // ...rest }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/controllers/API/queries/traces/helpers.ts` around lines 29 - 30, The current mapping casts apiSpan.type and apiSpan.status with type assertions (apiSpan.type as Span["type"], apiSpan.status as Span["status"]) which can hide invalid backend values; update the assignment in the helper that builds the Span object to validate values instead of asserting: add small validator functions or lookup sets (e.g., isValidSpanType/isValidSpanStatus or a Set of allowed Span types/statuses) and use them to accept apiSpan.type/apiSpan.status or else fall back to a safe default (e.g., "unknown" or a defined Span default value). Replace the direct casts in the span construction with the validated value or default so invalid API values cannot leak into Span instances.src/frontend/src/controllers/API/queries/traces/__tests__/helpers.test.ts (1)
9-186: Consider adding edge case tests.The test coverage is good for happy paths. Consider adding tests for:
convertSpanwith deeply nested children (3+ levels)sanitizeStringwith empty string inputconvertTracewithnullinput/output values (already covered implicitly)As per coding guidelines: "Consider including edge cases and error conditions for comprehensive test coverage."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/controllers/API/queries/traces/__tests__/helpers.test.ts` around lines 9 - 186, Add edge-case unit tests: add a test for sanitizeString that passes an empty string and asserts it returns an empty string; add a test for convertSpan that builds a SpanApiResponse with 3+ nested children (use convertSpan to transform and assert the deepest child id/name/lengths are preserved and parent-child relationships hold across all levels); optionally add a convertTrace test that explicitly sets input and output to null and asserts convertTrace returns a Trace object with null input/output and proper spans when spans exist. Ensure tests exercise sanitizeString, convertSpan, and convertTrace respectively.src/backend/base/langflow/services/tracing/native.py (2)
243-252: Add explicit handling forasyncio.CancelledErrorinwait_for_flush.Per coding guidelines, async operations should implement proper cleanup with try/except for
asyncio.CancelledError. Currently,CancelledErrorwould be caught by the broadExceptionhandler and only logged at debug level, potentially masking cancellation.♻️ Suggested fix
async def wait_for_flush(self) -> None: """Wait for the flush task to complete. Called by TracingService after end() to ensure database write completes. """ if self._flush_task is not None: try: await self._flush_task + except asyncio.CancelledError: + logger.debug("Flush task was cancelled") + raise except Exception as e: # noqa: BLE001 logger.debug("Error waiting for flush: %s", e)As per coding guidelines: "Use
asyncio.create_task()for background tasks and implement proper cleanup with try/except forasyncio.CancelledError".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend/base/langflow/services/tracing/native.py` around lines 243 - 252, The wait_for_flush method currently catches all Exceptions and logs them, which hides asyncio.CancelledError; update wait_for_flush (method name) to explicitly except asyncio.CancelledError when awaiting self._flush_task (symbol _flush_task), log a cancellation-specific debug message and re-raise the CancelledError to allow proper task cancellation semantics, then keep the existing broad Exception handler for other errors and log them as before.
295-303: Consider using a custom namespace UUID instead ofNAMESPACE_DNS.
NAMESPACE_DNSis intended for DNS domain names per RFC 4122. For trace/span ID generation, a custom namespace UUID would be more semantically appropriate and avoid potential collisions with DNS-based UUIDs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend/base/langflow/services/tracing/native.py` around lines 295 - 303, Replace the use of NAMESPACE_DNS in the deterministic uuid5 conversion with a module-level custom namespace UUID: define a constant (e.g., TRACE_NAMESPACE_UUID) near the top of the module as a fixed UUID literal (UUID("...") or generate once and paste the value) and then call uuid5(TRACE_NAMESPACE_UUID, f"{self.trace_id}-{span_data['id']}") inside the loop; update the import usage accordingly and add a brief comment explaining this namespace is dedicated to trace/span IDs to avoid DNS-based collisions (references: NAMESPACE_DNS, uuid5, trace_id, completed_spans, span_data).src/backend/base/langflow/services/tracing/native_callback.py (1)
170-170: Complex lambda could benefit from a brief comment.The lambda
lambda k, d=None, u=usage: getattr(u, k, d)uses a default parameter to captureusagein the closure. While functional, this pattern can be confusing. A brief inline comment would aid readability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend/base/langflow/services/tracing/native_callback.py` at line 170, The inline lambda assigned to _get (lambda k, d=None, u=usage: getattr(u, k, d)) is hard to read because it captures usage via a default parameter; add a brief inline comment above or beside this lambda (in native_callback.py near the _get declaration) explaining that the default parameter u=usage is used to bind the current usage object into the closure so getattr can be used uniformly whether usage is a dict or an object, or optionally replace the lambda with a small named helper function (e.g., def _get(k, d=None, u=usage): return getattr(u, k, d)) and comment that helper for clarity.src/backend/base/langflow/alembic/versions/6b64062d0c6a_merge_heads.py (1)
22-27: Unusedconnvariable in merge migration.The
conn = op.get_bind()calls in bothupgrade()anddowngrade()are unused. For a merge-heads migration that performs no schema changes, these can be removed or the functions can be left empty.♻️ Optional cleanup
def upgrade() -> None: - conn = op.get_bind() + pass def downgrade() -> None: - conn = op.get_bind() + pass🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend/base/langflow/alembic/versions/6b64062d0c6a_merge_heads.py` around lines 22 - 27, Remove the unused "conn = op.get_bind()" assignments in the merge migration functions: open the functions upgrade() and downgrade() and either delete the "conn = op.get_bind()" lines or replace the function bodies with a simple no-op (e.g., pass) so there are no unused variables; ensure "op.get_bind()" is not referenced elsewhere in these functions before removing.src/backend/base/langflow/alembic/versions/a1b2c3d4e5f6_add_trace_and_span_tables.py (1)
84-85: Consider addingON DELETE CASCADEto foreign key constraints.The ORM model defines
cascade="all, delete-orphan"on theTraceTable.spansrelationship (seemodel.pylines 70-79), but the database-level FK constraints don't enforce this. If records are deleted via raw SQL or database tools, orphan spans could remain.♻️ Suggested enhancement
- sa.ForeignKeyConstraint(["trace_id"], ["trace.id"], name="fk_span_trace_id_trace"), - sa.ForeignKeyConstraint(["parent_span_id"], ["span.id"], name="fk_span_parent_span_id_span"), + sa.ForeignKeyConstraint(["trace_id"], ["trace.id"], name="fk_span_trace_id_trace", ondelete="CASCADE"), + sa.ForeignKeyConstraint(["parent_span_id"], ["span.id"], name="fk_span_parent_span_id_span", ondelete="SET NULL"),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend/base/langflow/alembic/versions/a1b2c3d4e5f6_add_trace_and_span_tables.py` around lines 84 - 85, The foreign key constraints in the migration (sa.ForeignKeyConstraint entries named fk_span_trace_id_trace and fk_span_parent_span_id_span in add_trace_and_span_tables.py) lack ON DELETE CASCADE, so deletes performed outside the ORM can leave orphaned spans; update those ForeignKeyConstraint definitions to include ondelete="CASCADE" (or recreate the constraints with ON DELETE CASCADE) so the DB enforces the same cascade behavior as the TraceTable.spans relationship.src/frontend/src/modals/flowLogsModal/components/TraceView/__tests__/traceViewHelpers.test.ts (1)
23-37: Add fallback-branch assertions for mapping helpers.Current tests validate known values well; adding unknown-value cases would lock down default behavior and prevent accidental regressions.
Suggested test additions
describe("getSpanIcon", () => { it("returns icon names for known types", () => { expect(getSpanIcon("agent")).toBe("Bot"); expect(getSpanIcon("chain")).toBe("Link"); expect(getSpanIcon("retriever")).toBe("Search"); }); + + it("falls back to Circle for unknown types", () => { + expect(getSpanIcon("unknown" as never)).toBe("Circle"); + }); }); @@ describe("getStatusVariant", () => { it("maps status to badge variants", () => { expect(getStatusVariant("ok")).toBe("successStatic"); expect(getStatusVariant("error")).toBe("errorStatic"); expect(getStatusVariant("unset")).toBe("secondaryStatic"); }); + + it("falls back to secondaryStatic for unknown status", () => { + expect(getStatusVariant("unknown" as never)).toBe("secondaryStatic"); + }); });Based on learnings: "Consider including edge cases and error conditions for comprehensive test coverage" and "Verify tests cover both positive and negative scenarios where appropriate."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/modals/flowLogsModal/components/TraceView/__tests__/traceViewHelpers.test.ts` around lines 23 - 37, Add negative-case assertions for both mapping helpers: in the getSpanIcon tests add an assertion that an unexpected input (e.g., "unknown" or empty string) returns the same fallback value as calling getSpanIcon with an empty/undefined input (e.g., expect(getSpanIcon("unknown")).toBe(getSpanIcon(""))), and in the getStatusVariant tests assert that an unknown status maps to the default variant by comparing expect(getStatusVariant("unknown")).toBe(getStatusVariant("unset")); reference the getSpanIcon and getStatusVariant helpers to locate where to add these fallback assertions.src/frontend/src/controllers/API/queries/traces/__tests__/use-get-traces.test.ts (1)
41-47: Test intent and assertion are only partially aligned.The case name says it “returns empty result,” but it only verifies that
api.getwas not called. Consider asserting the returned data shape too.Suggested test tightening
it("returns empty result when flowId is missing", async () => { - useGetTracesQuery({ flowId: null }); + const result = useGetTracesQuery({ flowId: null }); await Promise.resolve(); expect(mockApiGet).not.toHaveBeenCalled(); + expect(result.data).toEqual({ traces: [], total: 0 }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/controllers/API/queries/traces/__tests__/use-get-traces.test.ts` around lines 41 - 47, The test currently calls useGetTracesQuery({ flowId: null }) but only asserts mockApiGet wasn't called; change it to capture the hook's return (e.g., const result = useGetTracesQuery({ flowId: null }) or destructure { data } from useGetTracesQuery), await the promise tick as already done, then assert the returned data matches the expected empty shape (e.g., result.current.data or data equals the known empty response shape used by useGetTracesQuery such as empty array or { items: [], total: 0 }), and keep the existing expect(mockApiGet).not.toHaveBeenCalled() assertion; reference useGetTracesQuery and mockApiGet when locating the change.src/frontend/src/controllers/API/queries/traces/use-get-trace.ts (1)
1-2: Decouple API query types from modal/UI modules.Line [1] pulls
Tracefrommodals/..., which couples the API query layer to presentation types. MoveTraceintosrc/frontend/src/controllers/API/queries/traces/types.ts(or a shared trace domain types module) and import it from there.Proposed direction
-import type { Trace } from "../../../../modals/flowLogsModal/components/TraceView/types"; +import type { Trace } from "./types";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/controllers/API/queries/traces/use-get-trace.ts` around lines 1 - 2, The query layer is importing the UI/modal type Trace from modals which couples API code to presentation; extract or create a shared trace type module (e.g., src/frontend/src/controllers/API/queries/traces/types.ts) and move the Trace interface there, then update imports in use-get-trace.ts to import Trace from that new module (and ensure any other files using the modal Trace are updated to import from the shared types); keep useQueryFunctionType import unchanged and run type checks to confirm no breakage.src/frontend/tests/core/features/traces.spec.ts (1)
6-10: Add a brief scenario comment above each test block.A one-liner above each
test(...)would satisfy the frontend test documentation rule.As per coding guidelines "Document each frontend test with a clear docstring/comment explaining its purpose, the scenario being tested, and expected outcomes."
Also applies to: 47-51
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/tests/core/features/traces.spec.ts` around lines 6 - 10, Add a one-line doc comment immediately above each frontend test block (the test(...) calls) describing the scenario and expected outcome; for example, place a brief sentence above the test whose title is "should able to see and interact with Traces" and the other test(s) with similar titles (the ones around the second block mentioned) that states what user flow is exercised and what the test asserts. Ensure the comment is concise (one line) and precedes the test(...) invocation so it documents the purpose, scenario, and expected result for both the "should able to see and interact with Traces" test and the other test block referenced.src/backend/tests/unit/api/v1/test_traces.py (1)
217-249: Add a negative-path test for missing input and missing completed root spans.
test_extract_trace_io_from_spansvalidates only the happy path. Add a case where no"Chat Input"span exists and no root span hasend_time, then assert both output fields areNone.Proposed test addition
+def test_extract_trace_io_from_spans_without_input_or_completed_root(): + trace_id = uuid4() + spans = [ + SpanTable( + trace_id=trace_id, + name="Intermediate", + span_type=SpanType.CHAIN, + status=SpanStatus.OK, + inputs={"foo": "bar"}, + outputs={"output": "ignored"}, + parent_span_id=uuid4(), + ), + ] + + io_data = _extract_trace_io_from_spans(spans) + + assert io_data["input"] is None + assert io_data["output"] is NoneBased on learnings "Test both sync and async code paths, mock external dependencies appropriately, test error handling and edge cases, validate input/output behavior, and test component initialization and configuration."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend/tests/unit/api/v1/test_traces.py` around lines 217 - 249, Add a negative-path unit test to verify _extract_trace_io_from_spans returns None for both input and output when there is no "Chat Input" span and no completed root spans: create a test (e.g., in src/backend/tests/unit/api/v1/test_traces.py) that builds spans with SpanTable entries using SpanType.CHAIN and SpanStatus.OK but omit any span with name "Chat Input" and ensure root spans have no end_time (leave end_time as None), call _extract_trace_io_from_spans(spans) and assert io_data["input"] is None and io_data["output"] is None; reference SpanTable, SpanType, SpanStatus and the _extract_trace_io_from_spans function to locate where to add this test.src/frontend/src/modals/flowLogsModal/components/TraceView/__tests__/SpanNode.test.tsx (1)
55-144: Add short purpose comments for eachit(...)case.Coverage quality is strong; one-line scenario/expected-outcome comments above each test would align with repo test documentation requirements.
As per coding guidelines "Document each frontend test with a clear docstring/comment explaining its purpose, the scenario being tested, and expected outcomes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/modals/flowLogsModal/components/TraceView/__tests__/SpanNode.test.tsx` around lines 55 - 144, Add a one-line purpose comment above each test case in SpanNode.test.tsx describing the scenario and expected outcome (e.g., above the it("renders name, tokens, latency, and status") test explain it verifies rendered span text, metrics, and status icon; above the it("calls onSelect when the row is clicked") test note it ensures clicking the span node invokes onSelect; above the it("calls onToggle (and not onSelect) when expand button is clicked") test state it checks expand-button clicks call onToggle but not onSelect for nodes with children; above the it("does not toggle when span has no children") test mention it verifies the expand button is hidden/aria-hidden and clicking does not call onToggle). Keep each comment as a single clear sentence and place it immediately above the corresponding it(...) block.src/backend/base/langflow/api/v1/traces.py (1)
469-475: Consider a set-based delete for flow-wide cleanup.Deleting traces row-by-row can become expensive for large flows. A set-based delete path would reduce DB round trips and memory pressure.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend/base/langflow/api/v1/traces.py` around lines 469 - 475, Row-by-row deletion of TraceTable objects (select(TraceTable) then looping and await session.delete(trace)) is inefficient for large flows; replace it with a set-based delete using a SQL DELETE statement (e.g., use sqlalchemy.delete or TraceTable.__table__.delete()) with the same predicate TraceTable.flow_id == flow_id and execute it with session.exec so the DB removes all matching traces in one operation (ensure existing cascade/on-delete rules still remove spans as expected and commit if needed).src/frontend/src/modals/flowLogsModal/index.tsx (1)
150-194: Defer logs loading when the Traces tab is active.After introducing tabs, logs-related data work still runs even for trace-only usage paths. Deferring logs tab work until
activeTab === "logs"would reduce unnecessary API/load overhead.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (61)
.secrets.baselinesrc/backend/base/langflow/alembic/versions/6b64062d0c6a_merge_heads.pysrc/backend/base/langflow/alembic/versions/a1b2c3d4e5f6_add_trace_and_span_tables.pysrc/backend/base/langflow/api/router.pysrc/backend/base/langflow/api/v1/__init__.pysrc/backend/base/langflow/api/v1/traces.pysrc/backend/base/langflow/services/database/models/__init__.pysrc/backend/base/langflow/services/database/models/traces/__init__.pysrc/backend/base/langflow/services/database/models/traces/model.pysrc/backend/base/langflow/services/tracing/native.pysrc/backend/base/langflow/services/tracing/native_callback.pysrc/backend/base/langflow/services/tracing/service.pysrc/backend/tests/unit/api/v1/test_traces.pysrc/backend/tests/unit/services/tracing/test_tracing_service.pysrc/frontend/src/components/core/logCanvasControlsComponent/__tests__/LogCanvasControls.spec.tsxsrc/frontend/src/components/core/logCanvasControlsComponent/index.tsxsrc/frontend/src/components/ui/sidebar.tsxsrc/frontend/src/controllers/API/helpers/constants.tssrc/frontend/src/controllers/API/queries/traces/__tests__/helpers.test.tssrc/frontend/src/controllers/API/queries/traces/__tests__/use-get-trace.test.tssrc/frontend/src/controllers/API/queries/traces/__tests__/use-get-traces.test.tssrc/frontend/src/controllers/API/queries/traces/helpers.tssrc/frontend/src/controllers/API/queries/traces/index.tssrc/frontend/src/controllers/API/queries/traces/types.tssrc/frontend/src/controllers/API/queries/traces/use-get-trace.tssrc/frontend/src/controllers/API/queries/traces/use-get-traces.tssrc/frontend/src/modals/flowLogsModal/components/FlowInsightsContent.tsxsrc/frontend/src/modals/flowLogsModal/components/TraceView/SpanDetail.tsxsrc/frontend/src/modals/flowLogsModal/components/TraceView/SpanNode.tsxsrc/frontend/src/modals/flowLogsModal/components/TraceView/SpanTree.tsxsrc/frontend/src/modals/flowLogsModal/components/TraceView/TraceAccordionItem.tsxsrc/frontend/src/modals/flowLogsModal/components/TraceView/TraceDetailView.tsxsrc/frontend/src/modals/flowLogsModal/components/TraceView/__tests__/SpanDetail.test.tsxsrc/frontend/src/modals/flowLogsModal/components/TraceView/__tests__/SpanNode.test.tsxsrc/frontend/src/modals/flowLogsModal/components/TraceView/__tests__/SpanTree.test.tsxsrc/frontend/src/modals/flowLogsModal/components/TraceView/__tests__/traceViewHelpers.test.tssrc/frontend/src/modals/flowLogsModal/components/TraceView/index.tsxsrc/frontend/src/modals/flowLogsModal/components/TraceView/traceViewHelpers.tssrc/frontend/src/modals/flowLogsModal/components/TraceView/traceViewTypes.tssrc/frontend/src/modals/flowLogsModal/components/TraceView/types.tssrc/frontend/src/modals/flowLogsModal/config/__tests__/flowTraceColumnsHelpers.test.tssrc/frontend/src/modals/flowLogsModal/config/flowTraceColumns.tsxsrc/frontend/src/modals/flowLogsModal/config/flowTraceColumnsHelpers.tssrc/frontend/src/modals/flowLogsModal/index.tsxsrc/frontend/src/pages/FlowPage/components/PageComponent/MemoizedComponents.tsxsrc/frontend/src/pages/FlowPage/components/PageComponent/index.tsxsrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/__tests__/sidebarSegmentedNav.test.tsxsrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/searchInput.tsxsrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/components/sidebarSegmentedNav.tsxsrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/index.tsxsrc/frontend/src/pages/FlowPage/components/flowSidebarComponent/types/index.tssrc/frontend/src/pages/FlowPage/index.tsxsrc/frontend/src/routes.tsxsrc/frontend/src/utils/__tests__/dateTime.test.tssrc/frontend/src/utils/dateTime.tssrc/frontend/tests/core/features/logs.spec.tssrc/frontend/tests/core/features/traces.spec.tssrc/frontend/tests/extended/features/flow-logs-modal.spec.tssrc/lfx/src/lfx/graph/graph/base.pysrc/lfx/src/lfx/services/tracing/base.pysrc/lfx/src/lfx/services/tracing/service.py
💤 Files with no reviewable changes (5)
- src/frontend/tests/core/features/logs.spec.ts
- src/frontend/src/components/core/logCanvasControlsComponent/tests/LogCanvasControls.spec.tsx
- src/frontend/src/pages/FlowPage/components/PageComponent/index.tsx
- src/frontend/tests/extended/features/flow-logs-modal.spec.ts
- src/frontend/src/components/core/logCanvasControlsComponent/index.tsx
| "id": tid, | ||
| "name": trace.name, | ||
| "status": trace.status.value if trace.status else SpanStatus.UNSET, | ||
| "startTime": trace.start_time.isoformat() if trace.start_time else SpanStatus.UNSET, |
Contributor
There was a problem hiding this comment.
Use a datetime-compatible fallback for startTime.
startTime currently falls back to SpanStatus.UNSET, which mixes status semantics into a timestamp field and can break timestamp parsing clients.
💡 Suggested fix
- "startTime": trace.start_time.isoformat() if trace.start_time else SpanStatus.UNSET,
+ "startTime": trace.start_time.isoformat() if trace.start_time else None,- "startTime": trace.start_time.isoformat() if trace.start_time else SpanStatus.UNSET,
+ "startTime": trace.start_time.isoformat() if trace.start_time else None,Also applies to: 300-300
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/backend/base/langflow/api/v1/traces.py` at line 197, Replace the
non-datetime fallback for the JSON "startTime" field (currently using
SpanStatus.UNSET) with a datetime-compatible value: use
trace.start_time.isoformat() if trace.start_time else None (or omit the field)
so consumers get either an ISO timestamp or null; update both occurrences in
traces.py where "startTime" is set (referencing trace.start_time and the current
SpanStatus.UNSET fallback) to use the null/ISO fallback instead.
| total_latency_ms: int = Field(default=0, description="Total execution time in milliseconds") | ||
| total_tokens: int = Field(default=0, description="Total tokens used across all LLM calls") | ||
| total_cost: float = Field(default=0.0, description="Estimated total cost") | ||
| flow_id: UUID = Field(index=True, description="ID of the flow this trace belongs to") |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for foreign_key patterns related to flow
rg -n "foreign_key.*flow" --type py src/backend/base/langflow/services/database/models/Repository: langflow-ai/langflow
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Search for flow table/model definition
fd -e py . src/backend/base/langflow/services/database/models/ | xargs rg -l "class Flow.*Table|class Flow.*SQLModel" | head -20Repository: langflow-ai/langflow
Length of output: 128
🏁 Script executed:
#!/bin/bash
# Check the actual traces/model.py for any relationship definitions with flow
rg -A 5 -B 5 "flow_id" src/backend/base/langflow/services/database/models/traces/model.pyRepository: langflow-ai/langflow
Length of output: 1203
🏁 Script executed:
#!/bin/bash
# Check how other models in the database models define foreign keys
rg -n "Field.*foreign_key" --type py src/backend/base/langflow/services/database/models/ | head -20Repository: langflow-ai/langflow
Length of output: 1260
🏁 Script executed:
#!/bin/bash
# Check the FlowTable class definition
rg -A 10 "class FlowTable" src/backend/base/langflow/services/database/models/flow/model.py | head -20Repository: langflow-ai/langflow
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Also search for any relationship definition between Trace and Flow
rg -n "trace.*flow|flow.*trace" --type py src/backend/base/langflow/services/database/models/traces/model.pyRepository: langflow-ai/langflow
Length of output: 250
🏁 Script executed:
#!/bin/bash
# Check the flow model structure - look for class definitions
rg -n "^class " src/backend/base/langflow/services/database/models/flow/model.py | head -10Repository: langflow-ai/langflow
Length of output: 319
🏁 Script executed:
#!/bin/bash
# Get more context from the flow model file
head -50 src/backend/base/langflow/services/database/models/flow/model.py | cat -nRepository: langflow-ai/langflow
Length of output: 2504
Add foreign_key constraint to flow_id field for referential integrity.
flow_id is indexed but lacks a foreign_key constraint to the flow table. This deviates from the codebase pattern where all cross-entity references (e.g., user_id, folder_id, trace_id) use foreign_key="table.id". Without a foreign key, traces can become orphaned if flows are deleted. Add foreign_key="flow.id" to match the established pattern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/backend/base/langflow/services/database/models/traces/model.py` at line
50, The flow_id field in the Trace model is missing a foreign key constraint;
update the flow_id Field declaration (symbol: flow_id) to include
foreign_key="flow.id" so it follows the existing pattern for cross-entity
references and enforces referential integrity (ensure the change is made on the
model class where flow_id: UUID = Field(...) is defined).
Comment on lines
+58
to
+59
| if (statusFilter !== "all" && trace.status !== statusFilter) { | ||
| return false; |
Contributor
There was a problem hiding this comment.
Align status filter values with the actual trace status payload.
Line 58 compares exact status values, but Lines 217 and 219 use success and running. In this PR’s trace payload/status helpers, statuses are ok | error | unset, so those options won’t match and filtering will silently fail for those cases.
💡 Suggested fix
- <SelectItem value="success">Success</SelectItem>
+ <SelectItem value="ok">Success</SelectItem>
<SelectItem value="error">Error</SelectItem>
- <SelectItem value="running">Running</SelectItem>
+ <SelectItem value="unset">Running</SelectItem>Also applies to: 216-220
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/frontend/src/modals/flowLogsModal/components/TraceView/index.tsx` around
lines 58 - 59, The status filter is comparing UI option strings like "success"
and "running" against trace.status values which are "ok" | "error" | "unset",
causing filters to fail; in the TraceView component update the status filter
logic to normalize/translate between UI option values and payload statuses
(e.g., map "success" -> "ok", "running" -> "unset", "error" -> "error") or
change the option values emitted by the selector to use "ok" | "error" | "unset"
directly, then use that normalized value when checking trace.status (refer to
statusFilter and trace.status in TraceView and the option rendering/selection
code around the existing "success"/"running" usage).
Comment on lines
+31
to
+57
| <div | ||
| className={cn( | ||
| "flex cursor-pointer items-center gap-2 rounded-md px-2 py-1.5 transition-colors", | ||
| "hover:bg-muted/50", | ||
| isSelected && "bg-muted", | ||
| )} | ||
| style={{ paddingLeft: `${depth * 16 + 8}px` }} | ||
| onClick={onSelect} | ||
| data-testid={`span-node-${span.id}`} | ||
| role="treeitem" | ||
| aria-selected={isSelected} | ||
| aria-expanded={hasChildren ? isExpanded : undefined} | ||
| > | ||
| {/* Expand/collapse button */} | ||
| <button | ||
| className={cn( | ||
| "flex h-4 w-4 items-center justify-center rounded-sm text-muted-foreground transition-colors", | ||
| hasChildren && "hover:bg-muted-foreground/20", | ||
| !hasChildren && "invisible", | ||
| )} | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| if (hasChildren) onToggle(); | ||
| }} | ||
| tabIndex={-1} | ||
| aria-hidden={!hasChildren} | ||
| > |
Contributor
There was a problem hiding this comment.
Keyboard accessibility is missing for the treeitem interaction.
This node is selectable/toggleable via mouse, but it isn’t keyboard-operable as implemented. Also set the button type explicitly to avoid unintended submit behavior in form contexts.
Suggested accessibility fix
<div
className={cn(
"flex cursor-pointer items-center gap-2 rounded-md px-2 py-1.5 transition-colors",
"hover:bg-muted/50",
isSelected && "bg-muted",
)}
style={{ paddingLeft: `${depth * 16 + 8}px` }}
onClick={onSelect}
+ onKeyDown={(e) => {
+ if (e.key === "Enter" || e.key === " ") {
+ e.preventDefault();
+ onSelect();
+ }
+ if (hasChildren && ((e.key === "ArrowRight" && !isExpanded) || (e.key === "ArrowLeft" && isExpanded))) {
+ e.preventDefault();
+ onToggle();
+ }
+ }}
data-testid={`span-node-${span.id}`}
role="treeitem"
+ tabIndex={0}
aria-selected={isSelected}
aria-expanded={hasChildren ? isExpanded : undefined}
>
@@
- <button
+ <button
+ type="button"
className={cn(
"flex h-4 w-4 items-center justify-center rounded-sm text-muted-foreground transition-colors",
hasChildren && "hover:bg-muted-foreground/20",
!hasChildren && "invisible",
)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div | |
| className={cn( | |
| "flex cursor-pointer items-center gap-2 rounded-md px-2 py-1.5 transition-colors", | |
| "hover:bg-muted/50", | |
| isSelected && "bg-muted", | |
| )} | |
| style={{ paddingLeft: `${depth * 16 + 8}px` }} | |
| onClick={onSelect} | |
| data-testid={`span-node-${span.id}`} | |
| role="treeitem" | |
| aria-selected={isSelected} | |
| aria-expanded={hasChildren ? isExpanded : undefined} | |
| > | |
| {/* Expand/collapse button */} | |
| <button | |
| className={cn( | |
| "flex h-4 w-4 items-center justify-center rounded-sm text-muted-foreground transition-colors", | |
| hasChildren && "hover:bg-muted-foreground/20", | |
| !hasChildren && "invisible", | |
| )} | |
| onClick={(e) => { | |
| e.stopPropagation(); | |
| if (hasChildren) onToggle(); | |
| }} | |
| tabIndex={-1} | |
| aria-hidden={!hasChildren} | |
| > | |
| <div | |
| className={cn( | |
| "flex cursor-pointer items-center gap-2 rounded-md px-2 py-1.5 transition-colors", | |
| "hover:bg-muted/50", | |
| isSelected && "bg-muted", | |
| )} | |
| style={{ paddingLeft: `${depth * 16 + 8}px` }} | |
| onClick={onSelect} | |
| onKeyDown={(e) => { | |
| if (e.key === "Enter" || e.key === " ") { | |
| e.preventDefault(); | |
| onSelect(); | |
| } | |
| if (hasChildren && ((e.key === "ArrowRight" && !isExpanded) || (e.key === "ArrowLeft" && isExpanded))) { | |
| e.preventDefault(); | |
| onToggle(); | |
| } | |
| }} | |
| data-testid={`span-node-${span.id}`} | |
| role="treeitem" | |
| tabIndex={0} | |
| aria-selected={isSelected} | |
| aria-expanded={hasChildren ? isExpanded : undefined} | |
| > | |
| {/* Expand/collapse button */} | |
| <button | |
| type="button" | |
| className={cn( | |
| "flex h-4 w-4 items-center justify-center rounded-sm text-muted-foreground transition-colors", | |
| hasChildren && "hover:bg-muted-foreground/20", | |
| !hasChildren && "invisible", | |
| )} | |
| onClick={(e) => { | |
| e.stopPropagation(); | |
| if (hasChildren) onToggle(); | |
| }} | |
| tabIndex={-1} | |
| aria-hidden={!hasChildren} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/frontend/src/modals/flowLogsModal/components/TraceView/SpanNode.tsx`
around lines 31 - 57, Make the treeitem keyboard-accessible by making the root
div focusable and handling Enter/Space to select and toggle: add tabIndex={0} to
the container div that currently has role="treeitem", and implement an onKeyDown
handler on that div which calls onSelect() for Enter or Space (preventDefault
for Space). Ensure the expand/collapse control remains operable via keyboard by
setting the button's type="button" and keeping onClick/onKeyDown behavior that
calls onToggle() when hasChildren is true (or call onToggle from the container
handler when appropriate); keep aria-expanded={hasChildren ? isExpanded :
undefined} and aria-selected tied to isSelected.
Comment on lines
+93
to
+99
| variant={ | ||
| traceStatus === "success" | ||
| ? "successStatic" | ||
| : traceStatus === "error" | ||
| ? "errorStatic" | ||
| : "secondaryStatic" | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# First, check for backend model definitions and status values
echo "=== Checking backend trace/span status definitions ==="
fd 'model\.py' src/backend --exec grep -l "status" {} \; | head -20
# Look for status enums or type definitions in backend
echo -e "\n=== Searching for status enum/type definitions ==="
fd '\.(py)$' src/backend/base/langflow/services/database/models --exec grep -n "status.*=\|Status.*=\|class.*Status\|class.*Trace" {} + 2>/dev/null | head -30
# Check for actual status value assignments in models
echo -e "\n=== Checking for status value patterns in backend ==="
rg -n 'status.*=.*["\x27](ok|success|error|unset)' src/backend/base/langflow/services/database/models --type=py | head -20
# Inspect the trace conversion helpers in frontend
echo -e "\n=== Checking frontend trace conversion/helpers ==="
fd 'helpers\.(ts|tsx)' src/frontend/src/controllers/API/queries/traces --exec head -300 {}
# Check for status usage in the specific component
echo -e "\n=== Checking status usage in TraceAccordionItem ==="
cat src/frontend/src/modals/flowLogsModal/components/TraceView/TraceAccordionItem.tsx | head -150Repository: langflow-ai/langflow
Length of output: 472
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check backend trace model for status field definition
echo "=== Backend Trace Model Status Definition ==="
cat src/backend/base/langflow/services/database/models/traces/model.py
echo -e "\n\n=== Frontend TraceAccordionItem Component ==="
cat src/frontend/src/modals/flowLogsModal/components/TraceView/TraceAccordionItem.tsxRepository: langflow-ai/langflow
Length of output: 12599
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where TraceAccordionItem is imported/used
echo "=== TraceAccordionItem Usage ==="
rg -n "TraceAccordionItem" src/frontend/src/modals/flowLogsModal --type=tsx --type=ts
# Check the parent component
echo -e "\n=== flowLogsModal/index.tsx (parent) ==="
cat src/frontend/src/modals/flowLogsModal/index.tsx | head -200
# Look for trace conversion/normalization helpers
echo -e "\n=== Trace conversion helpers ==="
fd 'helpers\.(ts|tsx)' src/frontend/src/modals/flowLogsModal --exec cat {}
# Check API query for traces
echo -e "\n=== Trace API queries ==="
fd 'traces\.ts' src/frontend/src/controllers/API/queries --exec head -200 {}Repository: langflow-ai/langflow
Length of output: 141
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where TraceAccordionItem is imported/used (correct rg syntax)
echo "=== TraceAccordionItem Usage ==="
rg -n "TraceAccordionItem" src/frontend/src/modals/flowLogsModal -t ts
# Check the main flowLogsModal file
echo -e "\n=== Looking for flowLogsModal main file ==="
fd -e tsx -e ts . src/frontend/src/modals/flowLogsModal --max-depth 2
# Check the main index file
echo -e "\n=== flowLogsModal index file ==="
cat src/frontend/src/modals/flowLogsModal/index.tsx 2>/dev/null | head -250 || echo "File not found, checking alternatives..."
# Look for trace data handling
echo -e "\n=== Checking for trace data transformation ==="
rg -n "traceStatus\|trace.*status\|convertTrace\|normaliz" src/frontend/src/modals/flowLogsModal -t ts | head -40Repository: langflow-ai/langflow
Length of output: 7954
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the TraceView index file where TraceAccordionItem is instantiated
echo "=== TraceView/index.tsx (where TraceAccordionItem is used) ==="
cat src/frontend/src/modals/flowLogsModal/components/TraceView/index.tsxRepository: langflow-ai/langflow
Length of output: 15453
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the traces API query file
echo "=== Looking for traces API query ==="
fd 'traces' src/frontend/src/controllers/API/queries --type f
# Read the traces query file
echo -e "\n=== API Query for Traces ==="
cat src/frontend/src/controllers/API/queries/traces.ts 2>/dev/null || cat src/frontend/src/controllers/API/queries/traces.tsx 2>/dev/null
# Also check if there's any type definition
echo -e "\n=== Looking for trace type definitions ==="
fd 'trace' src/frontend/src/types --type f | head -10
# Check the types for trace response
echo -e "\n=== Checking API response types ==="
rg -n "traceStatus\|status.*:" src/frontend/src/types -t ts | head -20Repository: langflow-ai/langflow
Length of output: 277
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the use-get-traces.ts file
echo "=== use-get-traces.ts ==="
cat src/frontend/src/controllers/API/queries/traces/use-get-traces.ts
# Also check for type definitions
echo -e "\n=== Looking for Trace type in API types ==="
rg -n "interface.*Trace\|type.*Trace\|status" src/frontend/src/controllers/API --type ts | head -30Repository: langflow-ai/langflow
Length of output: 1497
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the types file in traces directory
echo "=== Types for Traces Query ==="
cat src/frontend/src/controllers/API/queries/traces/types.ts
# Also check for any trace transformation helpers
echo -e "\n=== Checking for trace conversion helpers ==="
cat src/frontend/src/controllers/API/queries/traces/helpers.ts 2>/dev/null || echo "No helpers.ts found"
# Search for status field mappings
echo -e "\n=== Searching for status mappings ==="
rg -n "status.*success\|ok.*success\|success.*ok" src/frontend/src --type ts | head -20Repository: langflow-ai/langflow
Length of output: 3414
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Double-check: search for any place where "success" is used as a status value in backend
echo "=== Searching backend for 'success' status ==="
rg -n '"success"' src/backend/base/langflow/services/database/models --type py | head -20
# Verify the Trace type definition in frontend
echo -e "\n=== Frontend Trace type definition ==="
cat src/frontend/src/modals/flowLogsModal/components/TraceView/types.ts
# Check filter options one more time
echo -e "\n=== Filter panel status options in TraceView ==="
grep -n -A 5 "Success\|success" src/frontend/src/modals/flowLogsModal/components/TraceView/index.tsxRepository: langflow-ai/langflow
Length of output: 110
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the frontend Trace type definition
cat src/frontend/src/modals/flowLogsModal/components/TraceView/types.ts
# Check the filter panel again for the exact status options
grep -n -B 2 -A 8 "SelectItem.*success\|SelectItem.*error\|SelectItem.*running" src/frontend/src/modals/flowLogsModal/components/TraceView/index.tsxRepository: langflow-ai/langflow
Length of output: 1643
Fix badge variant mapping to use backend status values "ok" instead of "success".
Line 94 checks traceStatus === "success", but backend SpanStatus enum defines only "unset" | "ok" | "error". The badge will never match and always render as "secondaryStatic". Also, the filter UI at line 217 offers "success" as an option which cannot occur in the data.
Change the condition to check for "ok" instead of "success", and update the filter UI options to match the actual backend status values: "unset" | "ok" | "error".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/frontend/src/modals/flowLogsModal/components/TraceView/TraceAccordionItem.tsx`
around lines 93 - 99, The badge variant mapping in TraceAccordionItem.tsx
incorrectly checks traceStatus === "success" (which never occurs); update the
conditional that sets variant to check for "ok" instead of "success" so "ok"
maps to "successStatic", and also update the filter UI options (the status
filter component/array used in this file) to use the backend SpanStatus values
"unset", "ok", and "error" (remove or replace the "success" option) so filtering
and badge rendering align with actual data.
Comment on lines
+11
to
+18
| test.skip( | ||
| !process?.env?.OPENAI_API_KEY, | ||
| "OPENAI_API_KEY required to run this test", | ||
| ); | ||
|
|
||
| if (!process.env.CI) { | ||
| dotenv.config({ path: path.resolve(__dirname, "../../.env") }); | ||
| } |
Contributor
There was a problem hiding this comment.
Load .env before evaluating the skip condition.
On Line [11], test.skip runs before Line [17] loads dotenv, so local runs can be skipped even when OPENAI_API_KEY exists in .env.
Proposed fix
- test.skip(
- !process?.env?.OPENAI_API_KEY,
- "OPENAI_API_KEY required to run this test",
- );
-
if (!process.env.CI) {
dotenv.config({ path: path.resolve(__dirname, "../../.env") });
}
+
+ test.skip(
+ !process?.env?.OPENAI_API_KEY,
+ "OPENAI_API_KEY required to run this test",
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| test.skip( | |
| !process?.env?.OPENAI_API_KEY, | |
| "OPENAI_API_KEY required to run this test", | |
| ); | |
| if (!process.env.CI) { | |
| dotenv.config({ path: path.resolve(__dirname, "../../.env") }); | |
| } | |
| if (!process.env.CI) { | |
| dotenv.config({ path: path.resolve(__dirname, "../../.env") }); | |
| } | |
| test.skip( | |
| !process?.env?.OPENAI_API_KEY, | |
| "OPENAI_API_KEY required to run this test", | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/frontend/tests/core/features/traces.spec.ts` around lines 11 - 18, Move
the dotenv.config call so the .env file is loaded before the test.skip condition
is evaluated: call dotenv.config({ path: path.resolve(__dirname, "../../.env")
}) (respecting the existing process.env.CI guard) prior to invoking
test.skip(!process?.env?.OPENAI_API_KEY, ...), ensuring the OPENAI_API_KEY from
.env is available when test.skip checks process.env.
Comment on lines
+29
to
+32
| while (outdatedComponents > 0) { | ||
| await page.getByTestId("update-button").first().click(); | ||
| outdatedComponents = await page.getByTestId("update-button").count(); | ||
| } |
Contributor
There was a problem hiding this comment.
Add retry caps to cleanup loops to avoid hanging CI runs.
Lines [29-38] and Lines [69-78] can loop indefinitely if those controls keep reappearing.
Proposed fix
- let outdatedComponents = await page.getByTestId("update-button").count();
-
- while (outdatedComponents > 0) {
+ let outdatedComponents = await page.getByTestId("update-button").count();
+ let attempts = 0;
+ const maxAttempts = 20;
+ while (outdatedComponents > 0 && attempts < maxAttempts) {
+ attempts++;
await page.getByTestId("update-button").first().click();
outdatedComponents = await page.getByTestId("update-button").count();
}
+ expect(outdatedComponents).toBe(0);Also applies to: 35-38, 69-72, 75-78
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/frontend/tests/core/features/traces.spec.ts` around lines 29 - 32, The
cleanup while loops that rely on outdatedComponents and repeated clicks (using
page.getByTestId("update-button").first().click() and similar loops) need a
retry cap to avoid infinite hangs in CI; add a maxRetries counter (e.g.,
maxRetries = 20) and increment a retry counter each iteration, breaking and
failing the test with a clear message if the cap is reached; apply the same
pattern to the other similar loops referenced (the blocks around lines using
outdatedComponents and the blocks around page.getByTestId("remove-button"/other
testIds) at lines ~69-78) so all cleanup loops exit deterministically.
This was referenced Feb 27, 2026
![codeflash-ai[bot]](https://avatars.githubusercontent.com/in/413177?s=60&v=4){kind=small}
| return TraceloopTracer | ||
|
|
||
|
|
||
| def _get_native_tracer(): |
Contributor
There was a problem hiding this comment.
⚡️Codeflash found 112% (1.12x) speedup for _get_native_tracer in src/backend/base/langflow/services/tracing/service.py
⏱️ Runtime : 7.45 microseconds → 3.52 microseconds (best of 554 runs)
📝 Explanation and details
Brief: The optimized version memoizes the resolved NativeTracer class on the _get_native_tracer function (function attribute) so the expensive import/lookup runs only once. Subsequent calls do a cheap getattr + return. This removes repeated import overhead in hot paths while preserving behavior and identity.
What changed
- Added a one-time cache stored as _get_native_tracer._cached.
- On each call we first check getattr(_get_native_tracer, "_cached", None) and return that if present.
- Only when cache is missing do we perform from langflow.services.tracing.native import NativeTracer and store it on the function.
Why this speeds things up
- In Python an import statement still performs work every time it executes (module lookup in sys.modules, attribute binding, bytecode execution for the import statement). Even though the module is cached, the import statement itself is not free. The original code executed that import on every call.
- The optimized code replaces that repeated work with a single import (one hit in the profiler) and then a cheap getattr on the function object for subsequent calls. getattr on a function attribute and a fast return are far cheaper than executing the import statement again.
- Line-profiler evidence: the import line went from being hit 3019 times (dominating the original timing) to a single hit in the optimized run; most time in the optimized run is now the cheap getattr/branch/return, which is much faster per call.
- Measured effect: average runtime dropped from ~7.45 µs to ~3.52 µs (≈111% relative speedup).
Behavioral/compatibility notes
- Identity and semantics are preserved: the same class object is returned (cached reference), so tests asserting identity, metadata and instantiability still pass.
- Storing the cache on the function keeps the state local and avoids adding a module-level global variable.
- Threading: there is a benign race window where two threads racing to set _cached could both import and assign — both assign the same class object, so no incorrect state results. If strict single-import ordering is needed, a lock could be added, but for this use case that’s unnecessary.
When this helps most
- Hot paths that call _get_native_tracer repeatedly (the tests with 1000 calls, and any looped/hot code paths) see the largest gains. Annotated tests building large collections (1000 calls) show the optimization is effective and stable.
- If the function is rarely called, the benefit is negligible; the first call still pays the import cost.
Cost
- Very small memory and complexity cost: one extra attribute on the function. No extra dependencies or API changes.
In short: replacing a repeated import statement with a function-level memoized reference removes repeated interpreter/import overhead and converts the hot path into a fast attribute lookup + return, which is why the optimized version is measurably faster and safe to merge.
✅ Correctness verification report:
| Test | Status |
|---|---|
| ⚙️ Existing Unit Tests | 🔘 None Found |
| 🌀 Generated Regression Tests | ✅ 10 Passed |
| ⏪ Replay Tests | 🔘 None Found |
| 🔎 Concolic Coverage Tests | 🔘 None Found |
| 📊 Tests Coverage | 100.0% |
🌀 Click to see Generated Regression Tests
import inspect # used to introspect call signatures safely
import pytest # used for our unit tests
# Import the real class that the function should return.
from langflow.services.tracing.native import NativeTracer
from langflow.services.tracing.service import _get_native_tracer
def _class_constructible_without_arguments(cls) -> bool:
"""
Helper: determine if cls() can be called without arguments by examining
its __init__ signature. Returns True if safe to call with no args.
"""
try:
sig = inspect.signature(cls)
except (ValueError, TypeError):
# If signature cannot be obtained (e.g., builtins or C types), attempt a safe trial.
# We'll conservatively return False to avoid surprising constructor side-effects.
return False
params = list(sig.parameters.values())
# If there are no parameters at all, it's safe.
if not params:
return True
# Many classes' first parameter is 'self'; ignore it for our check.
# Count required parameters excluding 'self' and excluding VAR_POSITIONAL/VAR_KEYWORD.
required_count = 0
for i, p in enumerate(params):
# Skip the first parameter if named 'self' (bound instance method style).
if i == 0 and p.name == "self":
continue
# VAR_POSITIONAL (*args) or VAR_KEYWORD (**kwargs) do not force positional args.
if p.kind in (inspect.Parameter.VAR_POSITIONAL, inspect.Parameter.VAR_KEYWORD):
continue
# If a parameter has no default, it's required.
if p.default is inspect._empty:
required_count += 1
return required_count == 0
def test_basic_returns_exact_native_tracer_class():
# Call the function under test to get the returned value.
codeflash_output = _get_native_tracer(); returned = codeflash_output
def test_multiple_calls_are_idempotent_and_same_identity():
# Call the function multiple times and collect results.
results = [_get_native_tracer() for _ in range(10)] # small repeated sampling
# All returned references should be identical (same object).
first = results[0]
for idx, r in enumerate(results):
pass
def test_returned_value_is_a_class_and_not_an_instance():
# Ensure returned value is a class, not an instance of NativeTracer.
codeflash_output = _get_native_tracer(); returned = codeflash_output
def test_large_scale_idempotence_many_calls():
# Call the function a large number of times to check stability under scale.
large_count = 1000 # scale target per requirements
results = [_get_native_tracer() for _ in range(large_count)]
# All results should be identical to the imported NativeTracer class.
for idx, r in enumerate(results):
pass
#------------------------------------------------
import inspect # to examine constructor signatures
from importlib import import_module # to import modules dynamically
# imports
import pytest # used for our unit tests
from langflow.services.tracing.service import _get_native_tracer
def test_basic_returns_native_tracer_class():
# Import the NativeTracer class directly from its module
from langflow.services.tracing.native import NativeTracer # real import
# Call the function under test and ensure it returns the same class object
codeflash_output = _get_native_tracer(); result = codeflash_output
def test_class_metadata_matches_native_module():
# Verify metadata like __name__ and __module__ are as expected
NativeTracer = import_module("langflow.services.tracing.native").NativeTracer # real import via importlib
codeflash_output = _get_native_tracer(); cls = codeflash_output
def test_repeated_calls_are_idempotent_and_consistent():
# Call the function many times to ensure it consistently returns the same object
codeflash_output = _get_native_tracer(); first = codeflash_output
# Use a relatively large but deterministic loop to check for consistency (1000 iterations)
results = [_get_native_tracer() for _ in range(1000)]
# All results should be identical (the same class object)
for idx, r in enumerate(results):
pass
def test_instantiability_or_signature_indicates_required_args():
# Ensure we use the real class to decide instantiability
NativeTracer = import_module("langflow.services.tracing.native").NativeTracer
codeflash_output = _get_native_tracer(); cls = codeflash_output
# Attempt to instantiate without arguments if possible.
# If instantiation fails with TypeError, verify the signature indeed requires parameters.
try:
instance = cls() # try constructing with no args using the real constructor
except TypeError as e:
# Construction failed: inspect signature to ensure that the class requires parameters.
sig = inspect.signature(cls)
# Count how many parameters (excluding 'self' for __init__) are required for __init__
init = getattr(cls, "__init__", None)
if init is None:
# If there's no __init__, re-raise the unexpected TypeError
raise
init_sig = inspect.signature(init)
# Exclude 'self' parameter if present; count params without defaults
params = [
p for name, p in init_sig.parameters.items()
if name != "self"
]
required_params = [p for p in params if p.default is inspect._empty and p.kind in (p.POSITIONAL_ONLY, p.POSITIONAL_OR_KEYWORD)]
# The test passes because the TypeError is explained by required __init__ args
else:
pass
def test_native_tracer_attribute_consistency():
# Ensure attributes accessible on the imported class are the same as on the returned class
native_module = import_module("langflow.services.tracing.native")
NativeTracer = native_module.NativeTracer
codeflash_output = _get_native_tracer(); cls = codeflash_output
# Compare a handful of common class attributes for consistency
attributes_to_check = ["__name__", "__module__", "__qualname__", "__doc__"]
for attr in attributes_to_check:
pass
def test_large_scale_collection_identity():
# Build a large collection (1000 elements) of the returned class to ensure scalability and identity preservation
collected = [_get_native_tracer() for _ in range(1000)]
# Use set to confirm all elements are the exact same object (set of length 1)
unique = set(collected)
# And that single element should be the actual NativeTracer class from its module
NativeTracer = import_module("langflow.services.tracing.native").NativeTracer
# codeflash_output is used to check that the output of the original code is the same as that of the optimized code.To test or edit this optimization locally git merge codeflash/optimize-pr11934-2026-02-27T11.06.39
Suggested change
| def _get_native_tracer(): | |
| def _get_native_tracer(): | |
| # Cache the resolved tracer on the function to avoid repeated imports | |
| cached = getattr(_get_native_tracer, "_cached", None) | |
| if cached is not None: | |
| return cached | |
| _get_native_tracer._cached = NativeTracer |
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
2 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 by CodeRabbit
Release Notes
New Features
Tests