Repository navigation
fix(flows): dedupe double-rendered run output, add plain-language summary - #4822
Conversation
…mary
B19: node output was persisted as a double-nested `{ json: { json, raw,
text } }` envelope where `json`/`raw` hold identical copies of the
payload. The run-inspector's normalizer only unwrapped the outer n8n
item envelope, so the table/JSON views rendered the same data twice
under sibling `json`/`raw` keys. `unwrapPayloadEnvelope` in
lib/flows/runItems.ts now collapses that known envelope shape to a
single canonical payload.
B20: the run inspector's step output showed raw Composio/tool JSON
(costUsd, labelIds, markdownFormatted) and full flow:/run: ids as the
only view — unreadable for a non-technical user. Each step now shows
an always-visible, plain-language summary line (lib/flows/
runStepSummary.ts) derived generically from step status + known
output shapes (`summary`/`message` fields, `successful: false` +
`error`, array/`data` counts), with the raw JSON moved behind the
existing "Show raw output" disclosure. The drawer header now
truncates flow_id/thread_id to short form (full value on hover),
matching FlowRunsDrawer's existing convention.
Also fixes a frontend/backend type drift: `FlowRunStep.status`/
`duration_ms` exist on the wire (src/openhuman/flows/types.rs) but
were missing from the TS type, so the inspector had no durable
per-step success/failure signal to fall back on once the live socket
status goes quiet.
New i18n keys added to all locales with real translations.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe flow-run inspector now shows localized per-step summaries, keeps raw output behind an explicit disclosure, normalizes double-wrapped payloads, supports live step outcome metadata, and truncates flow and thread identifiers while retaining full values in tooltips. ChangesFlow inspector observability
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant FlowRunInspectorDrawer
participant normalizeItems
participant summarizeStep
participant RunItemDataBrowser
FlowRunInspectorDrawer->>normalizeItems: normalize step output
normalizeItems->>summarizeStep: provide canonical items
summarizeStep->>FlowRunInspectorDrawer: return localized summary
FlowRunInspectorDrawer->>RunItemDataBrowser: reveal raw output on request
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49cbcc6cb7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (value.json !== undefined && value.json !== null) return value.json; | ||
| if (value.raw !== undefined && value.raw !== null) return value.raw; | ||
| if (value.text !== undefined && value.text !== null) return value.text; |
There was a problem hiding this comment.
Require duplicate check before unwrapping payload envelopes
For any workflow node that intentionally returns a payload shaped exactly like { json, raw, text } (for example parsed content plus the original raw body), this unwraps to json before checking whether raw or text are actually duplicate envelope metadata. That silently removes meaningful fields from both the table and JSON views; please only collapse this shape when the sibling values are absent/null or proven duplicates of json.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 96fe774: unwrapPayloadEnvelope() no longer collapses the {json,raw,text} shape unconditionally. It now only collapses to json when raw/text are absent/null or proven deep-equal duplicates of json; if either sibling carries genuinely distinct, non-null data, the payload is left intact. Added regression tests in runItems.test.ts covering the distinct-siblings case (preserved) and the proven-duplicate case (still collapses).
| const failed = | ||
| step.status === 'error' || unsuccessful || (step.status === undefined && !!errorMessage); |
There was a problem hiding this comment.
Do not infer failure from status-less payload fields
When a step has no persisted status, this treats any top-level error/error_message string as a failed step. Reconstructed steps such as triggers can have status omitted while still carrying arbitrary user payloads, so a successful webhook/trigger payload that includes an error field would now render a red failure summary even though the run did not fail; please reserve failure classification for explicit step status or known failure envelopes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 96fe774: summarizeStep() no longer infers failure from a bare top-level error/error_message field when status is absent. Failure is now derived only from an explicit status === 'error' or a known failure envelope (Composio-style successful: false). Added a regression test in runStepSummary.test.ts with a status-less trigger payload {json:{event:'x', error:'user typo field'}} that no longer renders as failed, alongside one confirming the real successful:false envelope still does.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/lib/flows/runItems.ts`:
- Around line 105-128: Add namespaced, grep-friendly diagnostics to
unwrapPayloadEnvelope and toItem, logging entry, branch selection, and exit with
only redacted metadata such as value type, envelope-key names, and selected
branch; never include payload contents. Cover non-object, non-envelope, json,
raw, text, all-null, item-shaped, and payload-shaped paths while preserving the
existing normalization behavior.
In `@app/src/lib/flows/runStepSummary.test.ts`:
- Around line 24-73: Update summarizeStep’s success-summary selection,
specifically the itemsProduced fallback after primaryPayload and arrayLength
handling, because multi-item inputs are normalized into arrays and never reach
that branch. Remove the unreachable fallback or reorder the checks only if a
distinct multi-item summary is required, preserving the existing array-count
behavior.
In `@app/src/lib/i18n/es.ts`:
- Around line 3888-3889: Update the Spanish translations for
flowRuns.inspector.summary.itemsFetched and
flowRuns.inspector.summary.itemsProduced to avoid “elemento(s)”, using
count-neutral wording such as “Elementos obtenidos: {count}” and “Elementos
generados: {count}”, while preserving the existing interpolation key.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 17b8d4ac-10fd-43f5-82aa-63b220607964
📒 Files selected for processing (22)
app/src/components/flows/FlowRunInspectorDrawer.tsxapp/src/components/flows/__tests__/FlowRunInspectorDrawer.test.tsxapp/src/components/flows/__tests__/RunItemDataBrowser.test.tsxapp/src/lib/flows/runItems.test.tsapp/src/lib/flows/runItems.tsapp/src/lib/flows/runStepSummary.test.tsapp/src/lib/flows/runStepSummary.tsapp/src/lib/i18n/ar.tsapp/src/lib/i18n/bn.tsapp/src/lib/i18n/de.tsapp/src/lib/i18n/en.tsapp/src/lib/i18n/es.tsapp/src/lib/i18n/fr.tsapp/src/lib/i18n/hi.tsapp/src/lib/i18n/id.tsapp/src/lib/i18n/it.tsapp/src/lib/i18n/ko.tsapp/src/lib/i18n/pl.tsapp/src/lib/i18n/pt.tsapp/src/lib/i18n/ru.tsapp/src/lib/i18n/zh-CN.tsapp/src/services/api/flowsApi.ts
…sses @coderabbitai on app/src/lib/flows/runItems.ts:128) unwrapPayloadEnvelope/toItem silently picked among several normalization branches (non-object, non-envelope, json/raw/text, all-null, item-shaped vs payload-shaped) with no way to tell which path a given step's output took. Add namespaced app:flows:items debug logs at each branch, capturing only safe metadata (value type, recognized envelope-key names, selected branch) — never payload values.
…@coderabbitai on app/src/lib/flows/runStepSummary.test.ts:73) primaryPayload() always turns >1 items into an array, so arrayLength() (and the itemsFetched branch) matches before the items.length > 1 fallback can ever run — itemsProduced was dead code. Remove the branch, add a test proving the multi-item case is still summarized correctly via itemsFetched, and drop the now-unused flowRuns.inspector.summary.itemsProduced key from every locale file. Also fix the Spanish itemsFetched string per review: "elemento(s)" read awkwardly in Spanish; use the count-neutral "Elementos obtenidos: {count}" (addresses @coderabbitai on app/src/lib/i18n/es.ts:3889).
…elds Addresses two Codex comments on PR tinyhumansai#4822 that weren't covered by the earlier CodeRabbit pass: - unwrapPayloadEnvelope() collapsed any `{json,raw,text}`-shaped payload to `json` unconditionally, even when `raw`/`text` carried genuinely distinct, non-null data (e.g. parsed content plus a real raw body). Now only collapses when the sibling fields are absent/null or proven deep-equal duplicates of the kept value; otherwise the payload is left intact (@chatgpt-codex-connector comment 3571409474, app/src/lib/flows/runItems.ts:112). - summarizeStep() inferred failure from a bare top-level `error`/ `error_message` field whenever a step's `status` was absent. Reconstructed steps (e.g. triggers) can omit `status` while carrying arbitrary user payloads, so a successful webhook/trigger payload with an incidental `error` field rendered as a failure. Failure is now only ever derived from an explicit `status === 'error'` or a known failure envelope (Composio-style `successful: false`) (@chatgpt-codex-connector comment 3571409478, app/src/lib/flows/runStepSummary.ts:105).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/lib/flows/runStepSummary.ts`:
- Around line 102-111: Update the outcome-classification flow around
extractErrorMessage, isMarkedUnsuccessful, and the failed calculation to emit
namespaced, grep-friendly diagnostics containing status presence/value, item
count, whether successful:false was recognized, and the selected outcome branch.
Log only metadata and classification details—never payload contents or extracted
error text—and preserve the existing failure semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bb1fc6ae-16fe-4ba1-ab30-03cd547dcbbb
📒 Files selected for processing (4)
app/src/lib/flows/runItems.test.tsapp/src/lib/flows/runItems.tsapp/src/lib/flows/runStepSummary.test.tsapp/src/lib/flows/runStepSummary.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/lib/flows/runItems.ts
…cation Per CodeRabbit: emit grep-friendly `app:flows:step-summary` logs for the classification inputs (status presence/value, item count, recognized `successful:false` marker, error-reason presence, failed) and the selected outcome branch. Metadata only — never payload contents or error text.
Summary
{ json: { json, raw, text } }envelope wherejson/rawhold identical copies of the payload; the inspector rendered both, so users saw the same data twice (once per column/key).lib/flows/runItems.tsnow collapses that known envelope shape to a single canonical payload before it reaches the table/JSON views.costUsd,labelIds,markdownFormatted) plus fullflow_id/thread_id— unreadable for a non-technical user. Each step now shows an always-visible, plain-language one-line summary (lib/flows/runStepSummary.ts), with the raw JSON moved behind the existing "Show raw output" disclosure.FlowRunStep.status/duration_msexist on the wire (src/openhuman/flows/types.rs) but were missing from the TSFlowRunSteptype, so the inspector had no durable per-step success/failure signal once the live socket status goes quiet.flow_id/thread_idto short form (full value on hover viatitle), matchingFlowRunsDrawer's existingrun.id.slice(0, 8)convention, instead of printing the full ids in the primary view.Problem
Two MED-severity issues in the Workflows (flows) run-output UI:
Solution
unwrapPayloadEnvelopeinlib/flows/runItems.tsdetects the internal{ json, raw, text }envelope (keys must be a subset of exactly those three, andjsonmust be present) and collapses it to one canonical value (json, falling back toraw/text). Conservative by design — anything that doesn't match this exact known shape passes through untouched, so real payloads that happen to have fields namedjson/rawaren't silently mangled.summarizeStep(lib/flows/runStepSummary.ts) derives a{ outcome: 'success'|'error'|'neutral', text }summary generically from a step's normalized output — no tool-name-specific matching (no "gmail", no "slack"):step.status === 'error', or a Composio-stylesuccessful: falseenvelope (even whenstep.statusis'success'— the engine step succeeded but the underlying tool call failed), triggers the failure branch; the reason is read fromerror(string or{ message }).summary/messagestring field is surfaced verbatim (this is how the double-nested Gmail-summary example from the bug report becomes the primary line). Failing that, array/{ data: [...] }counts give a generic "Fetched N item(s)" line. Falls back to a generic "Step completed"/"No output produced".FlowRunInspectorDrawer.tsx'sStepRowrenders this summary line above the existing (relabeled) "Show raw output"<details>disclosure, and usesstep.statusas a dot-color fallback once the live socket status (useFlowRunProgress) goes quiet.flowRuns.inspector.summary.*, andflowRuns.inspector.outputrelabeled to "Show raw output") added with real translations to all 13 locales.I deliberately did not join against the flow's
WorkflowNode[]graph to look up a node'skind(e.g.agent/tool_call/http_request) —FlowRunStepcarries no node-kind field, and fetching+caching the full flow graph in this drawer for one more signal felt like scope creep for a MED bug fix. The summary is generic enough (status + output-shape heuristics) to read well for any node kind without it; this can be a natural follow-up if we want kind-flavored phrasing.Submission Checklist
runItems.test.ts(B19 envelope collapse: identical raw/json, differing raw/json, false-positive avoidance),RunItemDataBrowser.test.tsx(renders the double-wrapped payload exactly once, in both table and JSON views),runStepSummary.test.ts(success/failure derivation, Composiosuccessful: false, nestederror.message, no id leakage),FlowRunInspectorDrawer.test.tsx(primary summary visible + raw technical fields hidden until expanded, failure summary, header id truncation).pnpm testlocally on the affected suites (53/53, then 309/309 across allflowstests).## Related— N/A, see above.Closes #NNN— N/A, no tracked GitHub issue number was provided for B19/B20 (root-caused directly by the requester).Impact
app/src/), desktop app. No Rust/core changes.RunItemDataBrowser/table/JSON views are unchanged in behavior for non-enveloped payloads.Related
WorkflowNode[]graph to flavor the summary text by nodekind(not required for the fix — see note in Solution above).Verification
pnpm typecheck— passpnpm lint— pass (0 errors; pre-existing unrelated warnings in other files only)pnpm teston affected suites —RunItemDataBrowser.test.tsx,FlowRunInspectorDrawer.test.tsx,runItems.test.ts,runStepSummary.test.ts: 53/53 passed. Fullapp/src/components/flows+app/src/lib/flows+flowsApisuite: 309/309 passed.pnpm i18n:check— pass (0 missing/extra keys across all 13 locales)pnpm i18n:english:check— pre-existing failures unrelated to this branch (confirmed none of the newflowRuns.inspector.*keys appear in the flagged list)pnpm format— pass, Prettier applied minor formatting to the two new test-adjacent filesSummary by CodeRabbit