Repository navigation
fix(*): show what a dag node returned when its row waits on a decision - #1063
Conversation
A DAG node that stops on a decision is announced with the runner's brief to the main agent as the fenced report: the verdict, the resolve_dag_node calls it may make and that tool's whole schema. The web UI folded that brief open under "What came back", run together into one paragraph. The runner now passes what the node returned on that attempt (its error, when it raised) through every announcer to the manager, which carries it as `output` on the delivery mark, cut at 20,000 characters. The model's text is unchanged. The web fold shows that output under its own caption; a stored entry without it keeps the fold it had. The same mark now says `error` for a node that has already failed, where it said `exception`, and the web row words `notice` and `cancelled` as their own states instead of "Finished". Only a graph's own result is read for the run tally, since a node's output may quote one.
|
The fix in real Chrome, on the web UI's offline mode ( |
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the full diff and traced the node output through the runner, foreground outbox/release path, manager injection, stored transcript mark, live RPC boundary, generated schemas, and web live/replay rendering. I also checked relevant callers and history, the repository rules and canonical context terms, backward compatibility for older entries without output, status handling, test changes for weakening, and the architecture boundary between runtime, RPC, and UI. I tried to refute the edge cases around empty output, crashed nodes, repeated attempts, stall notices, released foreground runs, truncation, and graph-tally text; the implementation and tests account for them.
Verification: uv run --extra dev pytest tests/test_subagent_dag_adjudication.py tests/test_subagent_dag_runner.py tests/test_subagent_manager.py tests/test_playbook_stint.py -q (787 passed); focused web Vitest run (231 passed); web and TUI TypeScript type checks; web and TUI RPC generation checks; TUI i18n generation check; source-language check; and git diff --check all passed. An earlier Python invocation without the dev extra failed during shared fixture setup because raven_everos was absent, before any test body ran; rerunning with the repository's dev environment passed.
|
Not a blocker (review of Severity of this one finding, not a verdict on the pull request. Any blocking findings from this pass are review threads on the changed files; GitHub renders those collapsed, as a file name with no text.
|
|
No new Windows compatibility issue found in the inspected production changes. Reviewed head Native Windows verification (Python 3.13.12, Node 25.9.0):
The existing general UI observations are not re-filed. Limits: no full repository suite, live model/remote dispatch, browser visual check or interactive TUI. Tests used detached exact-revision checkouts and isolated profile/Raven/OAuth state. Exact verification commands and resultsuv sync --frozen --python 3.13 --extra dev
# native; exit 1; cwd C:\Users\livxu\AppData\Local\Temp\raven-windows-review\93f210e71847404cb53894de7ac36f62\pr1063\head
uv run --frozen --no-sync pytest tests/test_subagent_dag_adjudication.py tests/test_subagent_dag_runner.py::test_a_node_report_carries_what_the_node_itself_returned tests/test_subagent_dag_runner.py::test_a_crashed_node_report_carries_its_error_as_what_came_back tests/test_subagent_dag_runner.py::test_a_continued_nodes_second_report_carries_the_second_attempts_output tests/test_subagent_dag_runner.py::test_a_re_sent_report_still_carries_what_the_node_returned tests/test_subagent_dag_runner.py::test_a_stint_driver_passes_a_notification_on_with_its_output tests/test_subagent_dag_runner.py::test_the_announcer_is_told_whether_a_decision_is_pending tests/test_subagent_manager.py::test_an_informational_node_notice_is_headed_as_a_notice_not_a_failure tests/test_subagent_manager.py::test_a_node_report_marks_what_the_node_returned_for_the_reader tests/test_subagent_manager.py::test_a_node_report_marks_an_empty_return_as_empty_not_absent tests/test_subagent_manager.py::test_a_node_report_bounds_the_output_it_marks tests/test_subagent_manager.py::test_a_stall_notice_marks_no_output tests/test_playbook_stint.py::TestWhoAnswersARoundsSuspendedNode -n 0 -x --tb=short --basetemp C:\Users\livxu\AppData\Local\Temp\raven-windows-review\93f210e71847404cb53894de7ac36f62\checks\1063-native\pytest --junitxml C:\Users\livxu\AppData\Local\Temp\raven-windows-review\93f210e71847404cb53894de7ac36f62\checks\1063-native\junit.xml
# native-rest; exit 0; cwd C:\Users\livxu\AppData\Local\Temp\raven-windows-review\93f210e71847404cb53894de7ac36f62\pr1063\head
uv run --frozen --no-sync pytest tests/test_subagent_dag_runner.py::test_a_stint_driver_passes_a_notification_on_with_its_output tests/test_subagent_dag_runner.py::test_the_announcer_is_told_whether_a_decision_is_pending tests/test_subagent_manager.py::test_an_informational_node_notice_is_headed_as_a_notice_not_a_failure tests/test_subagent_manager.py::test_a_node_report_marks_what_the_node_returned_for_the_reader tests/test_subagent_manager.py::test_a_node_report_marks_an_empty_return_as_empty_not_absent tests/test_subagent_manager.py::test_a_node_report_bounds_the_output_it_marks tests/test_subagent_manager.py::test_a_stall_notice_marks_no_output tests/test_playbook_stint.py::TestWhoAnswersARoundsSuspendedNode -n 0 -x --tb=short --basetemp C:\Users\livxu\AppData\Local\Temp\raven-windows-review\93f210e71847404cb53894de7ac36f62\checks\1063-native-rest\pytest --junitxml C:\Users\livxu\AppData\Local\Temp\raven-windows-review\93f210e71847404cb53894de7ac36f62\checks\1063-native-rest\junit.xml
# base-cat; exit 0; cwd C:\Users\livxu\AppData\Local\Temp\raven-windows-review\93f210e71847404cb53894de7ac36f62\base431599
uv run --frozen --no-sync pytest tests/test_subagent_dag_runner.py::test_a_foreground_call_returns_the_first_report_while_the_graph_keeps_running -n 0 -x --tb=short --basetemp C:\Users\livxu\AppData\Local\Temp\raven-windows-review\93f210e71847404cb53894de7ac36f62\checks\1063-base-cat\pytest --junitxml C:\Users\livxu\AppData\Local\Temp\raven-windows-review\93f210e71847404cb53894de7ac36f62\checks\1063-base-cat\junit.xml
# native-python; exit 0; cwd C:\Users\livxu\AppData\Local\Temp\raven-windows-review\93f210e71847404cb53894de7ac36f62\pr1063\head
uv run --frozen --no-sync pytest tests/test_subagent_dag_runner.py::test_a_re_sent_report_still_carries_what_the_node_returned -p native_echo_fixture -n 0 -x --tb=short --basetemp C:\Users\livxu\AppData\Local\Temp\raven-windows-review\93f210e71847404cb53894de7ac36f62\checks\1063-native-python\pytest --junitxml C:\Users\livxu\AppData\Local\Temp\raven-windows-review\93f210e71847404cb53894de7ac36f62\checks\1063-native-python\junit.xml
# cwd: isolated head checkout/ui-web; exit 0; 231 passed
npm.cmd ci --ignore-scripts --no-audit --no-fund
node node_modules/vitest/vitest.mjs run src/features/transcript/TranscriptPage.test.tsx src/state/session/pipeline.events.test.ts --reporter=default --reporter=json --outputFile.json="C:/Users/livxu/AppData/Local/Temp/raven-windows-review/93f210e71847404cb53894de7ac36f62/checks/1063-web/results.json"For |
## Summary A still-running DAG node is labelled Finished in the TUI. Truncated node output also includes the system's cut marker inside the node's own Markdown, and expanding a long delivery body has no height limit. This follow-up addresses the three review findings on #1063. - Keep the first 20,000 characters of node text in `output` and carry a separate optional `output_truncated` boolean through the manager, spine, three RPC delivery shapes, generated clients and web live/replay paths. Render the translated system notice outside the node's Markdown. A whitespace-only truncated prefix still exposes the fold; older records and node-authored suffixes retain their contents. - Bound expanded delivery bodies with `max-height: min(50vh, 32rem)` and scrolling. The labelled region accepts keyboard focus, including navigation to its final line. This applies to every delivery fold. - Use the existing Still running translation for TUI notice rows in both live and replay views. - Replace the foreground test helper's external cat command with a real Python echo process and platform-aware argument quoting. The release/re-send regression explicitly refuses cat and uses a script path containing a space. - Apply the pinned formatter to two existing TUI component expressions so the whole-tree formatting hook passes. These changes only reflow whitespace. The model-facing report and untrusted-content handling remain unchanged. ## Type - [x] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification After applying the patch to current main, using Python 3.13.13 on Linux: - `uv run --frozen --no-sync pytest -q -n 0 --tb=short` with branch coverage enabled: **1,575 passed** across `test_subagent_dag_adjudication.py`, `test_subagent_dag_runner.py`, `test_subagent_manager.py`, `test_playbook_stint.py`, `test_rpc_spine.py`, `test_rpc_schema_match.py`, `test_spine_scheduler_lane.py`, `test_spine_events.py`, `test_rpc_turn_cancel.py`, `test_spine_scheduler.py` and `test_security_untrusted_context.py` under `tests/`. This includes the newly merged cancellation path. - `node node_modules/vitest/vitest.mjs run --maxWorkers=1 --no-file-parallelism`: web **253 files, 4,234 passed**; TUI **144 files, 2,251 passed, 13 skipped**. Both exited 0. - The repository diff-coverage gate passed: **100% (10/10 executable changed Python lines)**. Coverage was collected for the five changed runtime/RPC modules; this is diff coverage, not repository-wide coverage. - `uv run --frozen --no-sync ruff check`, `ruff format --check`, `ty check`, `lint-imports` and `deptry raven` passed. Ruff covered the repository lint targets; ty covered the source targets. All 10 import contracts were kept. - In `ui-web`: `npm run gen:check`, `npm run type-check`, `npm run lint` and `npm run build` passed. In `ui-tui`: `npm run lint:rpc`, `npm run lint:i18n`, `npm run type-check`, `npm run lint` and `npm run build` passed; lint retains 23 existing warnings and no errors. - `uv run --frozen --no-sync pre-commit run --files` over every changed file passed, including the whole-tree TUI formatting hook. - `UV_NO_SYNC=1 make check-commits check-large-files check-source-language PYTHON_VERSION=3.13.13 COMMIT_RANGE=origin/main...HEAD` passed, including commitlint and the source-language carrier rules. - Real Chromium on the offline page, driving the production history renderer: long output is bounded at 500px in a 1440x1000 viewport and 422px in 780x844 and 390x844 viewports. End reaches the last line, the cut notice is outside Markdown, and collapse works. No page errors. The page's existing 780px minimum width still limits its narrow layout. - Patch preparation included failing regressions before each fix and four rejected single-construct truncation mutants. - Native Windows validation and a fresh Python suite on the follow-up base have since been run, on the branch rebased onto `e8332e5d`: `pytest tests/test_subagent_manager.py tests/test_subagent_dag_runner.py tests/test_rpc_spine.py tests/test_rpc_schema_match.py tests/test_spine_events.py tests/test_spine_turn.py tests/test_rpc_turn_cancel.py tests/test_playbook_stint.py` -> the same 11 failures as `origin/main` on the same files, 0 introduced, all of them this Windows account's (a `~/.raven/oauth` permission refusal, symlink privilege, `os.killpg`, and the POSIX path rules #1066 fixes). The full `ui-web` suite passed (254 files, 4,278 tests) and the full `ui-tui` suite passed in CI. `ruff check`, `ruff format --check`, `ty check`, `lint-imports`, `npm run gen:check`, `npm run type-check`, `npm run lint` (ui-web and ui-tui), `npm run lint:rpc`, `npm run lint:i18n` and the repo's commit, large-file and source-language gates all pass on the rebased branch. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed ## Risk The truncation flag is additive. Older clients still display the output prefix but do not gain the separate notice. Previously stored records with an embedded suffix are preserved because the client cannot distinguish that suffix from node-authored text. The height cap changes scrolling for spawn and whole-graph results as well as node reports. Original run files retain the complete output. Delivered text follows the existing content policy; no dependency, migration or permission change is introduced. Rollback: revert this change. Previously stored marks may retain the optional field, which the older code ignores. - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes ## Related Issues #1063

Summary
A DAG node that stops on a decision is announced to the main agent with the runner's brief as the fenced report (
_exception_report): the verdict, theresolve_dag_nodecalls it may make, and that tool's whole schema. The web UI took the fence's text as the delivery row's body, so What came back on a row waiting on a decision opened onto that brief, run together into one paragraph, and never showed what the node had said. A spawn never reaches this state; only a DAG node's own report does.announce_dag_exception, which carries it asoutputon the delivery mark. The injected text the model reads is unchanged.outputis cut at 20,000 characters, the headdag.nodereads by default; the run dir keeps the whole text. It is present even when empty, so a node that said nothing is told apart from a runtime that never sent it.outputis declared onsubagent.delivered, onturn.started'sdelegatedand on a stored entry'sdelegated(OpenRPC, the Pydantic models, both generated clients), and the descriptions that named the fence as the only readable body now nameoutputas well.outputunder its own caption, What the node returned. A stored entry without it keeps the fold it had.exceptionand read Waiting on a decision; it is now markederrorand reads Failed. On the web,notice(a stall notice about a node still running) andcancelledfell through tookand read Finished; they now read Still running and Stopped.N completed, N failed, ...) is read only off a graph's own result. A node's report carries its own status, and its output may quote a sub-graph's tally; such a row would otherwise read Partly done.output, so the web UI's offline mode shows what ships.Not changed: the TUI does not read
output(its rows already worderror), and the model-facing report and ask are byte-identical.Type
Verification
At the head, on origin/main 4315993:
uv run --frozen --all-extras pytest -q -n 2overtests/test_subagent_*.py,tests/test_rpc_*.py,tests/test_spine*.py,tests/test_agent_loop_session*.py,tests/test_playbook*.py,tests/test_agent_loop_subagent*.pyandtests/test_security_untrusted_context.py: 7322 passed, 2 failed.tests/test_subagent_node_runtime.py::test_what_cannot_be_read_names_nothingfails whenever the suite runs as root (it builds a file it expects to be unreadable) and imports onlyraven.agent.subagent.node_runtime, which this change does not touch.tests/test_subagent_acp_remote.py::test_a_daemon_killed_outright_mid_turn_ends_the_turn_with_the_reason_and_leaves_no_agenthit its time budget under load and passed three times alone; it reaches none of the announcer code.npm testinui-web, both test roots: 253 files, 4220 of 4222 tests passed. The two others timed out at 5 s under load (src/state/session/pipeline.events.test.tsandsrc/state/session/registry.test.ts), and both files then passed twice alone, 68 of 68; the same suite passed 4222 of 4222 on the content before the rebase.ui-web:npm run gen:check,npm run type-check, andnpm run lint(0 errors; its 4 warnings are in files this change does not touch).ui-tui:npm run lint:rpc,npm run lint:i18n,npm run type-check, andsrc/__tests__/chatStream.test.ts(54 passed).ruff check,ruff format --checkandty checkover the touched Python files;lint-imports: 10 kept, 0 broken.scripts/check_commit_messages.py origin/main..HEAD, commitlint on the commit and on the prospective squash header,scripts/check_source_language.py origin/main...HEADandscripts/check_large_files.py origin/main...HEAD: all exit 0.Every new test was watched failing before its fix. A pass of 22 single-construct mutants -- each forwarding hop, the read of a crash's error before the judge's reason overwrites it, the
errorstatus, empty versus absent, the cap and its boundary, the live and replay pass-throughs, the caption, the tally predicate and the two new labels -- left none surviving.Real Chrome on the web UI's offline mode (
vite,src/page.html?stub=1): the scheduled conversation's suspended row reads Waiting on a decision and folds open onto What the node returned and the node's own two sentences.Relevant tests pass locally
Relevant lint / type checks pass locally
User-facing docs or screenshots are updated when needed
The local failures are the ones named above: a root-only test, and three timing-bound tests that pass alone. The changelog carries the entry.
Risk
A field is added to the mark, nothing is removed: a client that ignores
output(the TUI, an older web build) keeps showing the fence's text, and an entry stored before this keeps its old fold.A failed node's mark changes from
exceptiontoerror. The mark's status is read only by the two clients' row labels; nothing in the runtime routes on it.A stored conversation grows by up to 20,000 characters per node report. The text is the sub-agent's own and is kept unredacted, by the same rule the spawn announce applies to a result.
ExceptionAnnouncer, the outbox'sAnnounceReportandput_report, and the DAG tool's three adapters gain a defaultedoutputkeyword, and every in-tree implementation takes it. An announcer written against the old fixed signature would raise on it; none exists in the tree.Rollback: revert the squash commit. Nothing validates a stored mark at runtime, so entries that already carry
outputare passed through and ignored by the old code.Security impact considered
Backward compatibility considered
Rollback path is clear for risky changes
Related Issues
N/A