Repository navigation
fix(deep-research): require writer-committed final reports - #375
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (2)docs/source/**/*📄 CodeRabbit inference engine (AGENTS.md)
Files:
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (1)
WalkthroughDeep research now restricts final-report mutations to the writer, records backend-committed output with a run-local SHA-256 digest, validates that digest before completion, removes inline report salvage, and rejects missing or stale output with ChangesFinal report commitment
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant DeepResearcherAgent
participant OrchestratorGraph
participant WriterSubagent
participant Backend
participant FinalReportCommitTracker
DeepResearcherAgent->>FinalReportCommitTracker: create tracker for current run
DeepResearcherAgent->>OrchestratorGraph: build graph with tracker
OrchestratorGraph->>WriterSubagent: request final report
WriterSubagent->>Backend: write or edit final report
Backend-->>WriterSubagent: mutation result and report bytes
WriterSubagent->>FinalReportCommitTracker: record committed digest
OrchestratorGraph->>FinalReportCommitTracker: validate backend report digest
FinalReportCommitTracker-->>DeepResearcherAgent: committed Markdown or failure
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
690de2f to
221bd07
Compare
|
/ok to test 221bd07 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@docs/source/architecture/agents/deep-researcher.md`:
- Around line 43-46: Update the flow, handoff, and middleware descriptions in
this document to mention both canonical /shared/output.md and the route-local
/output.md alias wherever writer output paths, digest validation, or ownership
guards are described. Explicitly identify /output.md as route-local while
preserving the existing public-versus-internal path distinction.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 07580737-c0ff-484d-a599-470f2befcad0
📒 Files selected for processing (8)
docs/source/architecture/agents/deep-researcher.mdsrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/factory.pytests/aiq_agent/agents/deep_researcher/test_agent.pytests/aiq_agent/agents/deep_researcher/test_custom_middleware.pytests/aiq_agent/agents/deep_researcher/test_factory.pytests/aiq_agent/jobs/test_runner.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Lint and Hooks
- GitHub Check: Pytest and Coverage
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
tests/aiq_agent/jobs/test_runner.pytests/aiq_agent/agents/deep_researcher/test_factory.pysrc/aiq_agent/agents/deep_researcher/factory.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.pytests/aiq_agent/agents/deep_researcher/test_custom_middleware.pysrc/aiq_agent/agents/deep_researcher/agent.pytests/aiq_agent/agents/deep_researcher/test_agent.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/jobs/test_runner.pytests/aiq_agent/agents/deep_researcher/test_factory.pytests/aiq_agent/agents/deep_researcher/test_custom_middleware.pytests/aiq_agent/agents/deep_researcher/test_agent.py
docs/source/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update the docs under docs/source/ when behavior, configuration, or workflows change
Files:
docs/source/architecture/agents/deep-researcher.md
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}: Review documentation for command accuracy, branch-name consistency, current CI and copy-pr-bot behavior, public
vs internal boundary clarity, stale examples, and links that no longer match the repository layout.
Files:
docs/source/architecture/agents/deep-researcher.md
src/aiq_agent/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/aiq_agent/**/*.py: Respect authenticated data sources by honoring requires_auth, per-user token pass-through, and backend token validators; apply owner guardrails before loading protected report or artifact context into an agent
Do not weaken or bypass AuthMiddleware, validators, or auth gating without a prior design discussion
Files:
src/aiq_agent/agents/deep_researcher/factory.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/agent.py
src/aiq_agent/agents/**/*
⚙️ CodeRabbit configuration file
src/aiq_agent/agents/**/*: Review agent changes for research workflow correctness, graph state transitions, prompt/tool contracts,
HITL behavior, and failure handling. Flag changes that weaken source attribution, report generation,
async cancellation, checkpointing, or data-source selection without focused tests and docs.
Files:
src/aiq_agent/agents/deep_researcher/factory.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/agent.py
🧠 Learnings (1)
📚 Learning: 2026-07-06T23:55:42.908Z
Learnt from: cdgamarose-nv
Repo: NVIDIA-AI-Blueprints/aiq PR: 311
File: src/aiq_agent/agents/deep_researcher/prompts/orchestrator.j2:74-81
Timestamp: 2026-07-06T23:55:42.908Z
Learning: In agent test files (e.g., tests/aiq_agent/agents/*/test_agent.py), avoid brittle assertions that match exact substrings from prompt template files (such as *.j2 prompt wording). Prompt wording can change frequently, so instead assert structural/behavioral properties (e.g., that the prompt builder is called, that required sections/fields are present via stable markers, that the model output/agent behavior conforms to an expected schema, or that key actions are taken) rather than matching literal prompt text.
Applied to files:
tests/aiq_agent/agents/deep_researcher/test_agent.py
🔇 Additional comments (8)
src/aiq_agent/agents/deep_researcher/custom_middleware.py (2)
98-138: LGTM!Also applies to: 286-403, 404-479
60-65: 🔒 Security & PrivacyNormalize repeated leading slashes before path matching.
//shared/output.mdstays double-slash here, so it missesFINAL_REPORT_STATE_PATHSand the ownership/commit hooks don't see that form.tests/aiq_agent/agents/deep_researcher/test_custom_middleware.py (1)
284-462: LGTM!Also applies to: 465-639
src/aiq_agent/agents/deep_researcher/factory.py (1)
48-50: LGTM!Also applies to: 139-139, 435-492, 508-508, 540-540, 552-596
tests/aiq_agent/agents/deep_researcher/test_factory.py (1)
28-30: LGTM!Also applies to: 115-133, 255-276, 318-373
tests/aiq_agent/jobs/test_runner.py (1)
73-73: LGTM!Also applies to: 2344-2344, 2403-2403
src/aiq_agent/agents/deep_researcher/agent.py (1)
38-38: LGTM!Also applies to: 177-215, 256-277
tests/aiq_agent/agents/deep_researcher/test_agent.py (1)
35-73: LGTM!Also applies to: 424-424, 494-494, 563-563, 1121-1324, 1363-1471, 1499-1530, 1565-1729
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/source/architecture/agents/deep-researcher.md (1)
298-303: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winStale docs:
_salvage_inline_report()was removed in this PR.This Phase 4 paragraph still presents inline salvage as a live "defensive compatibility path" (≥400 chars + heading), but this PR deletes
_salvage_inline_report()and makes missing/stale/modified writer output fail closed withwriter_output_not_committed. This directly contradicts the new contract documented at lines 100-103. Remove the salvage description here.📝 Proposed doc fix
-This is the normative synthesis contract. As a defensive compatibility path, -`_salvage_inline_report()` accepts the orchestrator's final message when the -output file is missing, but only if the message is substantive Markdown: at -least 400 characters with a Markdown heading and not merely the writer -completion marker. Otherwise, missing writer output remains an error. +This is the synthesis contract. There is no inline-message salvage path: the +runtime accepts only writer output whose exact UTF-8 bytes match the run-local +writer digest. Missing, stale, or modified output fails closed with +`writer_output_not_committed`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/architecture/agents/deep-researcher.md` around lines 298 - 303, Remove the paragraph describing `_salvage_inline_report()` and inline Markdown fallback from the Phase 4 synthesis contract. Keep the documentation aligned with the current fail-closed behavior, where missing, stale, or modified writer output produces `writer_output_not_committed`, and preserve the surrounding normative synthesis content.Source: Path instructions
🤖 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.
Outside diff comments:
In `@docs/source/architecture/agents/deep-researcher.md`:
- Around line 298-303: Remove the paragraph describing
`_salvage_inline_report()` and inline Markdown fallback from the Phase 4
synthesis contract. Keep the documentation aligned with the current fail-closed
behavior, where missing, stale, or modified writer output produces
`writer_output_not_committed`, and preserve the surrounding normative synthesis
content.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: bcd28d19-10bb-44d6-a804-f849725a8f19
📒 Files selected for processing (8)
docs/source/architecture/agents/deep-researcher.mdsrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/factory.pytests/aiq_agent/agents/deep_researcher/test_agent.pytests/aiq_agent/agents/deep_researcher/test_custom_middleware.pytests/aiq_agent/agents/deep_researcher/test_factory.pytests/aiq_agent/jobs/test_runner.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Pytest and Coverage
- GitHub Check: Script Validation
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
tests/aiq_agent/jobs/test_runner.pysrc/aiq_agent/agents/deep_researcher/factory.pytests/aiq_agent/agents/deep_researcher/test_factory.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.pytests/aiq_agent/agents/deep_researcher/test_custom_middleware.pytests/aiq_agent/agents/deep_researcher/test_agent.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/jobs/test_runner.pytests/aiq_agent/agents/deep_researcher/test_factory.pytests/aiq_agent/agents/deep_researcher/test_custom_middleware.pytests/aiq_agent/agents/deep_researcher/test_agent.py
src/aiq_agent/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/aiq_agent/**/*.py: Respect authenticated data sources by honoring requires_auth, per-user token pass-through, and backend token validators; apply owner guardrails before loading protected report or artifact context into an agent
Do not weaken or bypass AuthMiddleware, validators, or auth gating without a prior design discussion
Files:
src/aiq_agent/agents/deep_researcher/factory.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.py
src/aiq_agent/agents/**/*
⚙️ CodeRabbit configuration file
src/aiq_agent/agents/**/*: Review agent changes for research workflow correctness, graph state transitions, prompt/tool contracts,
HITL behavior, and failure handling. Flag changes that weaken source attribution, report generation,
async cancellation, checkpointing, or data-source selection without focused tests and docs.
Files:
src/aiq_agent/agents/deep_researcher/factory.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.py
docs/source/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update the docs under docs/source/ when behavior, configuration, or workflows change
Files:
docs/source/architecture/agents/deep-researcher.md
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,SECURITY.md,CODE-OF-CONDUCT.md}: Review documentation for command accuracy, branch-name consistency, current CI and copy-pr-bot behavior, public
vs internal boundary clarity, stale examples, and links that no longer match the repository layout.
Files:
docs/source/architecture/agents/deep-researcher.md
🧠 Learnings (1)
📚 Learning: 2026-07-06T23:55:42.908Z
Learnt from: cdgamarose-nv
Repo: NVIDIA-AI-Blueprints/aiq PR: 311
File: src/aiq_agent/agents/deep_researcher/prompts/orchestrator.j2:74-81
Timestamp: 2026-07-06T23:55:42.908Z
Learning: In agent test files (e.g., tests/aiq_agent/agents/*/test_agent.py), avoid brittle assertions that match exact substrings from prompt template files (such as *.j2 prompt wording). Prompt wording can change frequently, so instead assert structural/behavioral properties (e.g., that the prompt builder is called, that required sections/fields are present via stable markers, that the model output/agent behavior conforms to an expected schema, or that key actions are taken) rather than matching literal prompt text.
Applied to files:
tests/aiq_agent/agents/deep_researcher/test_agent.py
🔇 Additional comments (19)
docs/source/architecture/agents/deep-researcher.md (1)
43-46: Document the route-local/output.mdalias. The digest gate, ownership guard, and handoff descriptions at lines 43-46, 91-109, and 157-161 still name only/shared/output.md, though the implementation also accepts the route-local/output.mdalias.Also applies to: 91-109, 157-161
Source: Path instructions
src/aiq_agent/agents/deep_researcher/custom_middleware.py (3)
60-136: LGTM!
286-401: LGTM!
415-465: LGTM!tests/aiq_agent/agents/deep_researcher/test_custom_middleware.py (2)
284-462: LGTM!
477-639: LGTM!src/aiq_agent/agents/deep_researcher/factory.py (3)
139-139: LGTM!
435-492: LGTM!
508-596: LGTM!tests/aiq_agent/agents/deep_researcher/test_factory.py (3)
115-133: LGTM!
255-277: LGTM!
353-373: LGTM!tests/aiq_agent/jobs/test_runner.py (1)
2344-2344: LGTM!Also applies to: 2403-2403
src/aiq_agent/agents/deep_researcher/agent.py (2)
177-215: LGTM!
256-277: LGTM!tests/aiq_agent/agents/deep_researcher/test_agent.py (4)
55-73: LGTM!
424-563: LGTM!
1121-1306: LGTM!
1363-1530: LGTM!
|
One documentation correction before merge:
For the route-local alias, keep |
Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
|
/ok to test 385d34b |
|
/merge |
09cb55d
into
NVIDIA-AI-Blueprints:release/2.2
Overview
Deep-research currently accepts any non-empty
/shared/output.mdas the final report, including stale planner/orchestrator content from the same state. This prerequisite establishes a small writer-owned commit boundary before the broader convergence/artifact work in #373.FinalReportCommitTrackerperDeepResearcherAgent.run().write_file/edit_filemutations of/shared/output.mdand its route-local/output.mdstate alias withfinal_report_writer_only.write_filecalls through the existing backend, refresh the exact UTF-8 SHA-256 after successful edits, and sanitize backend failures aswriter_output_commit_failed.writer_output_not_committed.This changes no API fields, configuration schema, database schema, deployment behavior, or sandbox-provider implementation.
After this prerequisite merges, #373 can rebase onto
release/2.2and use its validated ResearchNotes fallback when no committed writer report exists.DCO sign-off for the squash commit
Signed-off-by: Kyle Zheng kyzheng@nvidia.com
Validation
uv run ruff check .— passeduv run ruff format --check .— passed (372 files)uv run pytest— 1,836 passed, 13 skippedpre-commit run --all-files— all 13 hooks passed, including secrets, lockfiles, links, and notebook outputcd docs && make html— passeddeep_researcherreached success with a non-empty 12,351-byte reportgit commit -s.Where should reviewers start?
FinalReportCommitTracker,FinalReportOwnershipGuardMiddleware, andFinalReportCommitMiddlewareincustom_middleware.py.agent.pyand role-specific wiring infactory.py.StateBackend/CompositeBackendstale-planner overwrite regression intest_custom_middleware.py.Related Issues
Summary by CodeRabbit
/shared/output.md(or the supported backend alias) during the current run.writer_output_not_committed.