refactor: update citation verification handling in DeepResearcherAgent - #237
refactor: update citation verification handling in DeepResearcherAgent#237rkarmaka wants to merge 19 commits into
Conversation
|
@cdgamarose-nv Solving issue #235 |
Greptile SummaryThis PR refactors how
Confidence Score: 4/5The deep-researcher logic change is sound, but the chat_researcher node does not yet read citation_verification_status, so users can receive an unverified or potentially hallucinated deep-research report with no indication that source verification was skipped. The state-model and deep-researcher changes are clean and the new tests cover the two key branches. The gap is in chat_researcher: citation_verification_status is populated by deep_researcher but never inspected when building the response returned to the caller, meaning the unverified-report UX improvement the PR is building toward is currently absent. src/aiq_agent/agents/chat_researcher/agent.py — the citation_verification_status field on the returned DeepResearchAgentState is not yet consumed here. Important Files Changed
Sequence DiagramsequenceDiagram
participant CR as ChatResearcherAgent
participant DR as DeepResearcherAgent
participant SRM as SourceRegistryMiddleware
participant CV as citation_verification
CR->>DR: deep_research_fn(deep_state)
DR->>DR: agent.ainvoke() [retry loop]
DR->>SRM: _get_registry().all_sources()
alt Sources captured
SRM-->>DR: [source list]
DR->>CV: verify_citations(report, registry)
CV-->>DR: verified_report
DR-->>CR: "DeepResearchAgentState(citation_verification_status=None)"
else Empty source registry
SRM-->>DR: []
DR->>DR: validate_tool_availability()
alt "available_count == 0"
DR->>DR: logger.error(all tools unavailable)
else "available_count > 0"
DR->>DR: logger.warning(model answered without search)
end
DR->>DR: "result[citation_verification_status] = {status: unverified, ...}"
DR-->>CR: "DeepResearchAgentState(citation_verification_status={status: unverified, ...})"
Note over CR: citation_verification_status NOT checked - unverified report returned silently
end
Reviews (2): Last reviewed commit: "refactor: update error handling in deep ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Thank you for your contribution @rkarmaka. We will review soon |
|
@rkarmaka can you resolve conflicts? |
|
/ok to test 54fc182 |
|
@rkarmaka if you can fix the conflicts, we can make sure this goes into 2.2 release |
|
/ok to test 54fc182 |
|
@AjayThorve Sorry I missed your earlier comment. I will go ahead and resolve the conflict. Thanks! |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughDeep research now records citation-verification outcomes, propagates sanitized status through report contexts and job APIs, preserves it across report edits, and displays warnings in chat and UI reports. ChangesCitation Verification Status Propagation
Estimated code review effort: 4 (Complex) | ~55 minutes Sequence Diagram(s)sequenceDiagram
participant DeepResearcherAgent
participant JobRunner
participant JobsAPI
participant UIReportLoader
DeepResearcherAgent->>JobRunner: attach citation_verification_status
JobRunner->>JobRunner: strip warning and persist report status
JobRunner->>JobsAPI: return report and citation status
JobsAPI->>UIReportLoader: provide JobReportResponse
UIReportLoader->>UIReportLoader: prepend warning when present
UIReportLoader->>UIReportLoader: set report content
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/aiq_agent/agents/chat_researcher/agent.py`:
- Around line 333-356: `deep_research_node` is dropping
`citation_verification_status`, so chat can’t distinguish verified from
unverified deep research results. Update `chat_researcher/agent.py` in the
`self.deep_research_fn(deep_state)` flow to propagate the status from the deep
research result into the returned payload (or use it to emit a user-facing
notice when unverified), and remove the stale commented-out
`EmptySourceRegistryError` block now that that path is no longer used.
In `@src/aiq_agent/agents/deep_researcher/agent.py`:
- Around line 314-334: The `citation_verification_status.reason` in
`deep_researcher/agent.py` is too generic and does not distinguish between the
“all tools unavailable” path and the “tools available but unused” path. Update
the logic around the `logger.error`/`logger.warning` branches so the
`result["citation_verification_status"]` payload gets a distinct
machine-readable `reason` based on `available_count` and `unavailable` (for
example, one value for unavailable tools and another for unused tools), while
keeping the rest of the status fields intact.
🪄 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: 05745aa1-39d0-413c-95ff-4a36f055237b
📒 Files selected for processing (4)
src/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/models/state.pytests/aiq_agent/agents/deep_researcher/test_agent.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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:
src/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/models/state.pysrc/aiq_agent/agents/deep_researcher/agent.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/chat_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/models/state.pysrc/aiq_agent/agents/deep_researcher/agent.py
**
⚙️ CodeRabbit configuration file
**:AI-Q Agent Guidance
Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in.agents/skills/— load the
relevant skill before starting a workflow it covers.Project overview
AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.Primary boundaries:
- Backend Python package:
src/aiq_agent/.- Data-source and tool packages:
sources/(each is its own package).- Frontends and tooling:
frontends/(web UI infrontends/ui/, eval harnesses
infrontends/benchmarks/).- Configs, deployment, docs:
configs/,deploy/,docs/.Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treatsources/*as independent packages: prefer the smallest change
scoped to the package you are touching.Repository structure
Path Purpose src/aiq_agent/Backend agent, FastAPI extensions, auth, observability, knowledge sources/Data-source / tool packages (e.g. tavily_web_search,google_scholar_paper_search)configs/Workflow YAML configs (e.g. config_cli_default.yml)frontends/ui/Next.js / React / TypeScript / Tailwind / KUI web UI frontends/benchmarks/Eval harnesses: freshqa,deepsearch_qa,deepresearch_benchdeploy/Docker Compose and Helm/Kubernetes assets; deploy/.envfor secretsdocs/source/...
Files:
src/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/models/state.pysrc/aiq_agent/agents/deep_researcher/agent.pytests/aiq_agent/agents/deep_researcher/test_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/chat_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/models/state.pysrc/aiq_agent/agents/deep_researcher/agent.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/agents/deep_researcher/test_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 (3)
src/aiq_agent/agents/deep_researcher/models/state.py (1)
58-76: LGTM!tests/aiq_agent/agents/deep_researcher/test_agent.py (1)
1038-1085: LGTM!src/aiq_agent/agents/deep_researcher/agent.py (1)
328-334: 🩺 Stability & AvailabilityNo issue here.
agent.ainvoke()is used in the default mapping-returning path, andcitation_verification_statusis already a field onDeepResearchAgentState.> Likely an incorrect or invalid review comment.
1c1c697 to
a23a4a0
Compare
- - Removed the raising of EmptySourceRegistryError when no sources are available during deep research. - Added logging for cases where reports are generated without captured sources, indicating whether tools were unavailable or if the model answered without using search results. - Introduced a new citation_verification_status field in DeepResearchAgentState to track unverified reports, including the reason and available tool count. - Updated tests to ensure correct behavior when the source registry is empty and when sources are captured. Signed-off-by: Ranit Karmakar <karmakarranit6@gmail.com>
- Removed the raising of EmptySourceRegistryError when no verifiable sources are found during deep research. - Retained commented-out code for reference, indicating the previous error handling approach. - Adjusted the flow to ensure reports are returned with citation verification status, preventing loss of information when sources are unavailable. Signed-off-by: Ranit Karmakar <karmakarranit6@gmail.com>
Remove leftover conflict markers from the develop rebase, drop unused imports in DeepResearcherAgent, and keep both the new unverified-status tests and develop's writer-markdown test with the factory patch path. Signed-off-by: Ranit Karmakar <karmakarranit6@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Ranit Karmakar <karmakarranit6@gmail.com>
a23a4a0 to
84d08eb
Compare
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)
src/aiq_agent/agents/deep_researcher/agent.py (1)
182-195: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
state.filesfallback is lost for dict results due to operator precedence.Line 185 parses as
result.get("files", None)for the dict branch andgetattr(result, "files", None) or files or {}for the non-dict branch. Theor files or {}fallback only applies to the non-dict case. Whenresultis a dict without a"files"key (or withNone),filesbecomesNoneand thestate.filesfallback passed at line 274 is silently ignored, falling through to_salvage_inline_reportinstead.🐛 Proposed fix — wrap the ternary in parentheses so the fallback applies to both branches
- files = result.get("files", None) if isinstance(result, dict) else getattr(result, "files", None) or files or {} + files = (result.get("files", None) if isinstance(result, dict) else getattr(result, "files", None)) or files or {}🤖 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 `@src/aiq_agent/agents/deep_researcher/agent.py` around lines 182 - 195, The fallback to state.files is only applied in the non-dict branch of _extract_final_markdown, so dict results can ignore the passed-in files map and miss /shared/output.md or /output.md. Update _extract_final_markdown in deep_researcher.agent so the result.get("files") / getattr(result, "files") selection is grouped correctly and the files parameter is used as a fallback for both dict and non-dict result shapes, then keep the existing output-path extraction logic intact.
♻️ Duplicate comments (1)
src/aiq_agent/agents/deep_researcher/agent.py (1)
312-332: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
reasondoesn't distinguish tool-unavailable vs. tool-unused cases.The log message differentiates "all tools unavailable" (lines 313-318) from "tools available but unused" (lines 320-325), but the machine-readable
reasonis hardcoded to"empty_source_registry"in both cases. Downstream consumers can only infer the real cause by cross-referencingavailable_tool_count.♻️ Proposed fix
+ reason = "no_tools_available" if available_count == 0 else "sources_not_captured" if result is not None: result["citation_verification_status"] = { "status": "unverified", - "reason": "empty_source_registry", + "reason": reason, "available_tool_count": available_count, "unavailable_tools": unavailable, }🤖 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 `@src/aiq_agent/agents/deep_researcher/agent.py` around lines 312 - 332, The verification status in deep_researcher/agent.py uses the same hardcoded reason for two different outcomes, so update the logic around the citation_verification_status assignment to set a distinct machine-readable reason based on available_count and unavailable. Use one reason for the “all tools unavailable” path in the logger.error branch and a different one for the “tools available but unused” path in the logger.warning branch, keeping the rest of the result structure unchanged.
🤖 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 `@src/aiq_agent/agents/deep_researcher/agent.py`:
- Around line 182-195: The fallback to state.files is only applied in the
non-dict branch of _extract_final_markdown, so dict results can ignore the
passed-in files map and miss /shared/output.md or /output.md. Update
_extract_final_markdown in deep_researcher.agent so the result.get("files") /
getattr(result, "files") selection is grouped correctly and the files parameter
is used as a fallback for both dict and non-dict result shapes, then keep the
existing output-path extraction logic intact.
---
Duplicate comments:
In `@src/aiq_agent/agents/deep_researcher/agent.py`:
- Around line 312-332: The verification status in deep_researcher/agent.py uses
the same hardcoded reason for two different outcomes, so update the logic around
the citation_verification_status assignment to set a distinct machine-readable
reason based on available_count and unavailable. Use one reason for the “all
tools unavailable” path in the logger.error branch and a different one for the
“tools available but unused” path in the logger.warning branch, keeping the rest
of the result structure unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 9fa6ab94-e24d-4ee0-848d-d6f9aa55eefd
📒 Files selected for processing (2)
src/aiq_agent/agents/deep_researcher/agent.pytests/aiq_agent/agents/deep_researcher/test_agent.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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:
src/aiq_agent/agents/deep_researcher/agent.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/agent.py
**
⚙️ CodeRabbit configuration file
**:AI-Q Agent Guidance
Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in.agents/skills/— load the
relevant skill before starting a workflow it covers.Project overview
AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.Primary boundaries:
- Backend Python package:
src/aiq_agent/.- Data-source and tool packages:
sources/(each is its own package).- Frontends and tooling:
frontends/(web UI infrontends/ui/, eval harnesses
infrontends/benchmarks/).- Configs, deployment, docs:
configs/,deploy/,docs/.Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treatsources/*as independent packages: prefer the smallest change
scoped to the package you are touching.Repository structure
Path Purpose src/aiq_agent/Backend agent, FastAPI extensions, auth, observability, knowledge sources/Data-source / tool packages (e.g. tavily_web_search,google_scholar_paper_search)configs/Workflow YAML configs (e.g. config_cli_default.yml)frontends/ui/Next.js / React / TypeScript / Tailwind / KUI web UI frontends/benchmarks/Eval harnesses: freshqa,deepsearch_qa,deepresearch_benchdeploy/Docker Compose and Helm/Kubernetes assets; deploy/.envfor secretsdocs/source/...
Files:
src/aiq_agent/agents/deep_researcher/agent.pytests/aiq_agent/agents/deep_researcher/test_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/agent.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/agents/deep_researcher/test_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 (3)
src/aiq_agent/agents/deep_researcher/agent.py (1)
274-274: LGTM!tests/aiq_agent/agents/deep_researcher/test_agent.py (2)
1119-1149: LGTM!
1151-1173: LGTM!
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)
src/aiq_agent/agents/deep_researcher/agent.py (1)
332-338: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAvoid item assignment on
resulthere.DeepResearchAgentStateis aBaseModel, and this flow already handles dict-or-model results elsewhere.result["citation_verification_status"] = ...will raise ifainvokereturns the state object, so update the state viamodel_copy(update=...)orsetattrbefore returning it.🤖 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 `@src/aiq_agent/agents/deep_researcher/agent.py` around lines 332 - 338, Avoid direct item assignment on the `result` object in `DeepResearchAgentState` handling, since `ainvoke` may return a BaseModel instance rather than a dict. In the `DeepResearchAgent` flow where `citation_verification_status` is set for the `empty_source_registry` case, update the result via `model_copy(update=...)` or `setattr` depending on whether `result` is a model or dict, matching the existing dict-or-model handling used elsewhere before returning it.
🤖 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 `@src/aiq_agent/agents/deep_researcher/agent.py`:
- Around line 332-338: Avoid direct item assignment on the `result` object in
`DeepResearchAgentState` handling, since `ainvoke` may return a BaseModel
instance rather than a dict. In the `DeepResearchAgent` flow where
`citation_verification_status` is set for the `empty_source_registry` case,
update the result via `model_copy(update=...)` or `setattr` depending on whether
`result` is a model or dict, matching the existing dict-or-model handling used
elsewhere before returning it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: f9f4a68b-fcc3-482f-99a7-e3edd2911110
📒 Files selected for processing (1)
src/aiq_agent/agents/deep_researcher/agent.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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:
src/aiq_agent/agents/deep_researcher/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/agent.py
**
⚙️ CodeRabbit configuration file
**:AI-Q Agent Guidance
Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in.agents/skills/— load the
relevant skill before starting a workflow it covers.Project overview
AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.Primary boundaries:
- Backend Python package:
src/aiq_agent/.- Data-source and tool packages:
sources/(each is its own package).- Frontends and tooling:
frontends/(web UI infrontends/ui/, eval harnesses
infrontends/benchmarks/).- Configs, deployment, docs:
configs/,deploy/,docs/.Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treatsources/*as independent packages: prefer the smallest change
scoped to the package you are touching.Repository structure
Path Purpose src/aiq_agent/Backend agent, FastAPI extensions, auth, observability, knowledge sources/Data-source / tool packages (e.g. tavily_web_search,google_scholar_paper_search)configs/Workflow YAML configs (e.g. config_cli_default.yml)frontends/ui/Next.js / React / TypeScript / Tailwind / KUI web UI frontends/benchmarks/Eval harnesses: freshqa,deepsearch_qa,deepresearch_benchdeploy/Docker Compose and Helm/Kubernetes assets; deploy/.envfor secretsdocs/source/...
Files:
src/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/agent.py
🔇 Additional comments (3)
src/aiq_agent/agents/deep_researcher/agent.py (3)
333-337:reasonis still hardcoded to"empty_source_registry"for both the tools-unavailable and tools-unused paths.This was raised previously. Note the regression tests (
test_run_empty_source_registry_returns_unverified_status) assertreason == "empty_source_registry", so any split ofreasonmust update those assertions in lockstep.
141-141: LGTM!
186-191: LGTM!
|
/ok to test dd3a572 |
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/aiq_agent/agents/deep_researcher/agent.py (2)
350-355: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not silently publish the unverified report.
As flagged in a previous review, this
unverifiedstatus is retained only in the internalDeepResearchAgentState. External response boundaries discard it, making a report produced with zero captured sources indistinguishable from a verified report to users and downstream consumers. Please propagate a typed verification disposition through both job and chat responses and surface an explicit warning forunverified.🤖 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 `@src/aiq_agent/agents/deep_researcher/agent.py` around lines 350 - 355, Propagate the typed verification disposition from DeepResearchAgentState through both job and chat response boundaries, preserving “unverified” when citation_verification_status indicates an empty source registry. Update the external response models and serializers used by DeepResearchAgent so unverified reports are distinguishable from verified reports, and add an explicit user-facing warning for the unverified disposition.
349-356: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPrevent
TypeErroron object results and distinguish thereasonfield.The
reasonfield is hardcoded to"empty_source_registry", failing to distinguish between unavailable and unused tools (as flagged in a previous review). Additionally, sinceresultcould be an object rather than a dictionary, using item assignment (result["citation_verification_status"] = ...) risks aTypeError.
src/aiq_agent/agents/deep_researcher/agent.py#L349-L356: Safely assign the verification status and distinguish thereasonfield based on tool availability.tests/aiq_agent/agents/deep_researcher/test_agent.py#L1259-L1262: Update the assertion to match the distinct reason ("sources_not_captured"), sinceavailable_countwill be 1 in this test.🛠️ Proposed fixes
src/aiq_agent/agents/deep_researcher/agent.py- if result is not None: - result["citation_verification_status"] = { - "status": "unverified", - "reason": "empty_source_registry", - "available_tool_count": available_count, - "unavailable_tools": unavailable, - } + if result is not None: + reason = "no_tools_available" if available_count == 0 else "sources_not_captured" + status_payload = { + "status": "unverified", + "reason": reason, + "available_tool_count": available_count, + "unavailable_tools": unavailable, + } + if isinstance(result, dict): + result["citation_verification_status"] = status_payload + else: + setattr(result, "citation_verification_status", status_payload)
tests/aiq_agent/agents/deep_researcher/test_agent.py- assert result.citation_verification_status["reason"] == "empty_source_registry" + assert result.citation_verification_status["reason"] == "sources_not_captured"🤖 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 `@src/aiq_agent/agents/deep_researcher/agent.py` around lines 349 - 356, In the verification-status assignment in src/aiq_agent/agents/deep_researcher/agent.py lines 349-356, safely update both dictionary and object results without assuming item assignment, and set reason based on tool availability so unavailable tools remain distinct from unused tools. Update the corresponding assertion in tests/aiq_agent/agents/deep_researcher/test_agent.py lines 1259-1262 to expect "sources_not_captured"; no other test behavior should change.
🤖 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.
Duplicate comments:
In `@src/aiq_agent/agents/deep_researcher/agent.py`:
- Around line 350-355: Propagate the typed verification disposition from
DeepResearchAgentState through both job and chat response boundaries, preserving
“unverified” when citation_verification_status indicates an empty source
registry. Update the external response models and serializers used by
DeepResearchAgent so unverified reports are distinguishable from verified
reports, and add an explicit user-facing warning for the unverified disposition.
- Around line 349-356: In the verification-status assignment in
src/aiq_agent/agents/deep_researcher/agent.py lines 349-356, safely update both
dictionary and object results without assuming item assignment, and set reason
based on tool availability so unavailable tools remain distinct from unused
tools. Update the corresponding assertion in
tests/aiq_agent/agents/deep_researcher/test_agent.py lines 1259-1262 to expect
"sources_not_captured"; no other test behavior should change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 030809cd-617a-401f-8008-65ddd344cd21
📒 Files selected for processing (2)
src/aiq_agent/agents/deep_researcher/agent.pytests/aiq_agent/agents/deep_researcher/test_agent.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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/agents/deep_researcher/test_agent.pysrc/aiq_agent/agents/deep_researcher/agent.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/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/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/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
Signed-off-by: Ranit Karmakar <karmakarranit6@gmail.com>
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 `@src/aiq_agent/agents/chat_researcher/agent.py`:
- Around line 356-370: The report state currently risks storing citation-warning
text in last_report_markdown instead of clean markdown. Update the
DeepResearcherAgent result handling around report_text and
prepend_citation_verification_warning to capture or retrieve the raw report
markdown before applying the warning, use that clean value for
last_report_markdown, and retain the warning only in the visible report_message
content.
🪄 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: 7bf67f53-0898-419c-9704-57f916967901
📒 Files selected for processing (15)
frontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/adapters/api/deep-research-client.tsfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.spec.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.tssrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/models/__init__.pysrc/aiq_agent/agents/deep_researcher/models/state.pytests/aiq_agent/agents/chat_researcher/test_agent.pytests/aiq_agent/agents/deep_researcher/test_agent.pytests/aiq_agent/fastapi_extensions/test_deep_research.pytests/aiq_agent/jobs/test_runner.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (8)
**/*.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/fastapi_extensions/test_deep_research.pysrc/aiq_agent/agents/deep_researcher/models/__init__.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pytests/aiq_agent/agents/chat_researcher/test_agent.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/models/state.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/fastapi_extensions/test_deep_research.pytests/aiq_agent/agents/chat_researcher/test_agent.pytests/aiq_agent/agents/deep_researcher/test_agent.py
frontends/ui/**/*.{js,ts,jsx,tsx,vue}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run npm lint, type-check, and build validation for UI changes in frontends/ui
Files:
frontends/ui/src/adapters/api/index.tsfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/adapters/api/deep-research-client.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.spec.ts
frontends/ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
frontends/ui/**/*.{ts,tsx,jsx,js}: The UI is built with Next.js / React / TypeScript / Tailwind with KUI components; reuse existing KUI components and visual patterns rather than introducing new ones
Validate UI-affecting changes with npm run lint, npm run type-check, and npm run test:ci, and include a screenshot for visible changes
Files:
frontends/ui/src/adapters/api/index.tsfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/adapters/api/deep-research-client.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.spec.ts
frontends/ui/**/*
⚙️ CodeRabbit configuration file
frontends/ui/**/*: Review UI changes for strict TypeScript behavior, API contract alignment, auth/session handling, accessible controls,
resilient loading and error states, and report/chat state consistency. Prefer existing UI patterns and require tests
for changed user-visible workflows.
Files:
frontends/ui/src/adapters/api/index.tsfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/adapters/api/deep-research-client.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.spec.ts
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/models/__init__.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/models/state.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/models/__init__.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/models/state.pysrc/aiq_agent/agents/deep_researcher/agent.py
{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}
⚙️ CodeRabbit configuration file
{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}: Treat API, auth, and job-runner changes as externally visible contracts. Check authorization boundaries,
request tracing, async job lifecycle, websocket reconnect behavior, error responses, and cross-user data isolation.
Require tests for route behavior, access decisions, and job state transitions when those surfaces change.
Files:
frontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/src/aiq_api/jobs/runner.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/chat_researcher/test_agent.pytests/aiq_agent/agents/deep_researcher/test_agent.py
🔇 Additional comments (17)
src/aiq_agent/agents/deep_researcher/models/state.py (1)
28-87: LGTM!Also applies to: 125-125
src/aiq_agent/agents/deep_researcher/models/__init__.py (1)
16-22: LGTM!Also applies to: 43-57
src/aiq_agent/agents/deep_researcher/agent.py (1)
33-33: LGTM!Also applies to: 44-48, 279-286, 347-371, 398-404
tests/aiq_agent/agents/deep_researcher/test_agent.py (1)
1233-1267: LGTM!Also applies to: 1268-1296, 1299-1321
tests/aiq_agent/agents/chat_researcher/test_agent.py (1)
848-888: LGTM!frontends/aiq_api/src/aiq_api/jobs/runner.py (2)
827-829: LGTM!
1250-1258: LGTM!frontends/ui/src/adapters/api/index.ts (1)
33-37: LGTM!Also applies to: 113-123
frontends/ui/src/features/chat/hooks/use-load-job-data.ts (3)
29-29: LGTM!
262-268: LGTM!
336-342: LGTM!frontends/ui/src/features/chat/hooks/use-load-job-data.spec.ts (1)
98-105: LGTM!Also applies to: 189-213, 436-439, 461-461, 470-472, 482-485, 519-521
frontends/aiq_api/src/aiq_api/routes/jobs.py (1)
51-51: LGTM!Also applies to: 401-418, 1166-1168
tests/aiq_agent/fastapi_extensions/test_deep_research.py (1)
189-207: LGTM!tests/aiq_agent/jobs/test_runner.py (1)
606-630: LGTM!frontends/ui/src/adapters/api/deep-research-client.ts (1)
42-54: LGTM!Also applies to: 779-779, 798-808
frontends/ui/src/adapters/api/deep-research-client.spec.ts (1)
5-9: LGTM!Also applies to: 38-57
Signed-off-by: Ranit Karmakar <karmakarranit6@gmail.com>
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 `@tests/aiq_agent/agents/chat_researcher/test_agent.py`:
- Around line 981-984: Update the assertions in the relevant chat researcher
test to derive the expected warning through the imported
citation_verification_warning function instead of hardcoding its user-facing
string. Preserve the existing checks that the final message begins with the
warning and contains it exactly once, along with the last_report_markdown
assertion.
🪄 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: 21109bf7-fde6-4cc0-a1f9-cf72e7368f77
📒 Files selected for processing (4)
src/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/models/__init__.pysrc/aiq_agent/agents/deep_researcher/models/state.pytests/aiq_agent/agents/chat_researcher/test_agent.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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:
src/aiq_agent/agents/deep_researcher/models/__init__.pytests/aiq_agent/agents/chat_researcher/test_agent.pysrc/aiq_agent/agents/deep_researcher/models/state.pysrc/aiq_agent/agents/chat_researcher/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/models/__init__.pysrc/aiq_agent/agents/deep_researcher/models/state.pysrc/aiq_agent/agents/chat_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/models/__init__.pysrc/aiq_agent/agents/deep_researcher/models/state.pysrc/aiq_agent/agents/chat_researcher/agent.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/agents/chat_researcher/test_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/chat_researcher/test_agent.py
🔇 Additional comments (3)
src/aiq_agent/agents/deep_researcher/models/state.py (1)
88-97: LGTM!src/aiq_agent/agents/deep_researcher/models/__init__.py (1)
23-59: LGTM!src/aiq_agent/agents/chat_researcher/agent.py (1)
388-405: LGTM!
Signed-off-by: Ranit Karmakar <karmakarranit6@gmail.com>
|
/ok to test 3670801 |
AjayThorve
left a comment
AjayThorve
left a comment
There was a problem hiding this comment.
Remaining citation-verification contract findings on the current head.
| clarifier_result: Log from clarifier agent dialog. | ||
| available_documents: User-uploaded documents with summaries for context. | ||
| citation_verification_status: Set when citation verification was skipped | ||
| because the source registry was empty. ``None`` means the report was |
There was a problem hiding this comment.
[P1] Do not equate missing status with verified
The new contract says None means the report was verified, but the control flow also leaves it None when enable_citation_verification=False and when verification runs with a nonempty registry but finds zero valid citations. In the latter case, a report with no citation markers is returned unchanged and the API/UI omit the warning even though no claims were verified.
Make the verification engine return an explicit closed outcome for every run—at minimum verified, unverified, or disabled—with machine-readable reasons such as no_sources and no_valid_citations. Propagate that outcome instead of inferring success from absence.
| # Apply caller metadata first, then set the canonical report last so a | ||
| # stray "report" key in output_metadata can never overwrite the real report. | ||
| output = {**(output_metadata or {}), "report": report} | ||
| citation_verification_status = _extract_citation_verification_status(result) |
There was a problem hiding this comment.
[P1] Preserve unverified disposition across derived reports
This extracts the disposition only from the current agent result. ReportRewriterAgentState has no citation status, and report_output_metadata(parent_job_id, "edit") does not carry the parent disposition, so an edit of an unverified parent is persisted and returned with citation_verification_status=null. The warning is also flattened into report text before persistence, so the rewrite model can alter or drop the only remaining trust signal.
Treat unverified as monotonic through report edits and parent-seeded research unless the complete derived report is reverified. Carry the disposition through ReportContext and output metadata, and keep canonical report text separate from presentation warnings.
| class CitationVerificationStatusResponse(BaseModel): | ||
| """Public citation-verification disposition for a generated report.""" | ||
|
|
||
| status: str = Field(..., description="Citation verification status, for example 'unverified'") |
There was a problem hiding this comment.
[P2] Make the public disposition a closed domain type
This new API exposes unrestricted str fields while the agent state uses dict[str, Any] and the TypeScript client uses plain string. Invalid status/reason combinations are representable, and public_citation_verification_status() silently discards any future status other than unverified.
Define the authoritative CitationVerificationOutcome alongside CitationVerificationResult in common.citation_verification, use Literal or enum values for status and reason, and map that type into the API DTO. User-facing warning copy should be rendered at the response/UI boundary rather than defining the domain contract.
Signed-off-by: Ranit Karmakar <karmakarranit6@gmail.com>
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 `@src/aiq_agent/agents/chat_researcher/agent.py`:
- Around line 472-481: Update the inline report edit return in report_edit_node
to compute the combined citation verification status, retain clean revised in
last_report_markdown, and pass the visible message content through
prepend_citation_verification_warning using that status, matching
deep_research_node behavior.
🪄 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: 301eac61-5aa6-4c08-807e-256189fe95fe
📒 Files selected for processing (23)
frontends/aiq_api/src/aiq_api/jobs/report_context.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/tests/test_report_context.pyfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/adapters/api/deep-research-client.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.spec.tssrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/models/state.pysrc/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/models/__init__.pysrc/aiq_agent/agents/deep_researcher/models/state.pysrc/aiq_agent/agents/report_rewriter/agent.pysrc/aiq_agent/agents/report_rewriter/models.pysrc/aiq_agent/common/__init__.pysrc/aiq_agent/common/citation_verification.pytests/aiq_agent/agents/chat_researcher/test_agent.pytests/aiq_agent/agents/deep_researcher/test_agent.pytests/aiq_agent/agents/report_rewriter/test_agent.pytests/aiq_agent/common/test_citation_verification.pytests/aiq_agent/fastapi_extensions/test_deep_research.pytests/aiq_agent/jobs/test_runner.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (8)
**/*.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/report_rewriter/models.pytests/aiq_agent/fastapi_extensions/test_deep_research.pysrc/aiq_agent/common/__init__.pyfrontends/aiq_api/tests/test_report_context.pysrc/aiq_agent/agents/chat_researcher/models/state.pytests/aiq_agent/agents/chat_researcher/test_agent.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pysrc/aiq_agent/agents/deep_researcher/models/__init__.pytests/aiq_agent/common/test_citation_verification.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pytests/aiq_agent/agents/report_rewriter/test_agent.pysrc/aiq_agent/agents/deep_researcher/models/state.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/agents/report_rewriter/agent.pyfrontends/aiq_api/src/aiq_api/jobs/report_context.pytests/aiq_agent/agents/deep_researcher/test_agent.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/common/citation_verification.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/fastapi_extensions/test_deep_research.pyfrontends/aiq_api/tests/test_report_context.pytests/aiq_agent/agents/chat_researcher/test_agent.pytests/aiq_agent/common/test_citation_verification.pytests/aiq_agent/agents/report_rewriter/test_agent.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/report_rewriter/models.pysrc/aiq_agent/common/__init__.pysrc/aiq_agent/agents/chat_researcher/models/state.pysrc/aiq_agent/agents/deep_researcher/models/__init__.pysrc/aiq_agent/agents/deep_researcher/models/state.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/agents/report_rewriter/agent.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/common/citation_verification.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/report_rewriter/models.pysrc/aiq_agent/agents/chat_researcher/models/state.pysrc/aiq_agent/agents/deep_researcher/models/__init__.pysrc/aiq_agent/agents/deep_researcher/models/state.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/agents/report_rewriter/agent.pysrc/aiq_agent/agents/chat_researcher/agent.py
frontends/ui/**/*.{js,ts,jsx,tsx,vue}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run npm lint, type-check, and build validation for UI changes in frontends/ui
Files:
frontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/adapters/api/deep-research-client.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.spec.ts
frontends/ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
frontends/ui/**/*.{ts,tsx,jsx,js}: The UI is built with Next.js / React / TypeScript / Tailwind with KUI components; reuse existing KUI components and visual patterns rather than introducing new ones
Validate UI-affecting changes with npm run lint, npm run type-check, and npm run test:ci, and include a screenshot for visible changes
Files:
frontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/adapters/api/deep-research-client.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.spec.ts
frontends/ui/**/*
⚙️ CodeRabbit configuration file
frontends/ui/**/*: Review UI changes for strict TypeScript behavior, API contract alignment, auth/session handling, accessible controls,
resilient loading and error states, and report/chat state consistency. Prefer existing UI patterns and require tests
for changed user-visible workflows.
Files:
frontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/adapters/api/deep-research-client.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.spec.ts
{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}
⚙️ CodeRabbit configuration file
{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}: Treat API, auth, and job-runner changes as externally visible contracts. Check authorization boundaries,
request tracing, async job lifecycle, websocket reconnect behavior, error responses, and cross-user data isolation.
Require tests for route behavior, access decisions, and job state transitions when those surfaces change.
Files:
frontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/src/aiq_api/jobs/report_context.py
🧠 Learnings (2)
📚 Learning: 2026-06-14T17:49:00.640Z
Learnt from: torkian
Repo: NVIDIA-AI-Blueprints/aiq PR: 273
File: frontends/aiq_api/tests/test_sse_reconnect_cursor.py:384-401
Timestamp: 2026-06-14T17:49:00.640Z
Learning: When using `unittest.mock.patch` for code that imports dependencies inside functions/generators (e.g., inside `aiq_api.routes.jobs`), don’t patch via an attribute that doesn’t exist on the consuming module. If the generator does `from ..jobs.event_store import EventStore` inside the generator body, then `aiq_api.routes.jobs` will not have an `EventStore` attribute; patch the source class/method in its defining module instead (e.g., `aiq_api.jobs.event_store.EventStore.get_events_async`). Patching `aiq_api.routes.jobs.EventStore...` would raise `AttributeError` because that symbol is not present at module scope.
Applied to files:
frontends/aiq_api/tests/test_report_context.py
📚 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/chat_researcher/test_agent.pytests/aiq_agent/agents/report_rewriter/test_agent.pytests/aiq_agent/agents/deep_researcher/test_agent.py
🪛 ast-grep (0.44.1)
tests/aiq_agent/agents/report_rewriter/test_agent.py
[info] 135-140: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{
"parent_job_id": "parent-job",
"citation_verification_status": {"status": "unverified", "reason": "no_sources"},
}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🔇 Additional comments (27)
frontends/aiq_api/src/aiq_api/jobs/report_context.py (1)
18-18: LGTM!Also applies to: 49-49, 70-74, 195-200, 233-234, 237-260, 314-328
frontends/aiq_api/src/aiq_api/jobs/runner.py (1)
819-837: LGTM!Also applies to: 1262-1272
frontends/aiq_api/src/aiq_api/routes/jobs.py (1)
51-53: LGTM!Also applies to: 403-420, 894-898, 1172-1174
frontends/aiq_api/tests/test_report_context.py (1)
185-213: LGTM!Also applies to: 295-306
tests/aiq_agent/fastapi_extensions/test_deep_research.py (1)
189-207: LGTM!tests/aiq_agent/jobs/test_runner.py (1)
606-626: LGTM!frontends/ui/src/adapters/api/deep-research-client.ts (1)
42-59: LGTM!Also applies to: 784-784, 803-813
frontends/ui/src/adapters/api/deep-research-client.spec.ts (1)
5-9: LGTM!Also applies to: 38-57
frontends/ui/src/features/chat/hooks/use-load-job-data.spec.ts (1)
98-105: LGTM!Also applies to: 189-213, 436-439, 461-461, 470-472, 482-485, 519-521
src/aiq_agent/common/citation_verification.py (2)
133-198: LGTM!
1083-1121: LGTM!src/aiq_agent/agents/deep_researcher/models/state.py (2)
63-110: LGTM!
130-147: LGTM!src/aiq_agent/agents/deep_researcher/models/__init__.py (1)
49-72: LGTM!src/aiq_agent/common/__init__.py (1)
81-90: LGTM!src/aiq_agent/agents/deep_researcher/agent.py (1)
353-382: LGTM!tests/aiq_agent/agents/deep_researcher/test_agent.py (3)
1233-1267: LGTM!
1299-1348: LGTM!
1696-1703: LGTM!src/aiq_agent/agents/chat_researcher/agent.py (1)
389-408: LGTM!src/aiq_agent/agents/chat_researcher/models/state.py (1)
43-47: LGTM!Also applies to: 86-86
src/aiq_agent/agents/report_rewriter/agent.py (1)
8-11: LGTM!Also applies to: 25-26, 59-66, 80-101, 111-118, 135-155, 182-182, 235-243, 255-255
src/aiq_agent/agents/report_rewriter/models.py (1)
32-32: LGTM!src/aiq_agent/agents/chat_researcher/register.py (1)
419-422: LGTM!Also applies to: 456-476, 535-539
tests/aiq_agent/common/test_citation_verification.py (1)
550-567: LGTM!Also applies to: 579-580, 1107-1125
tests/aiq_agent/agents/report_rewriter/test_agent.py (1)
122-122: LGTM!Also applies to: 123-145
tests/aiq_agent/agents/chat_researcher/test_agent.py (1)
31-31: LGTM!Also applies to: 943-986
Signed-off-by: Ranit Karmakar <karmakarranit6@gmail.com>
|
@rkarmaka Following up - would you be able to address the PR comments and resolve merge conflicts? |
…pty-source-registry Brings in upstream 2.2.1 hardening (NVIDIA-AI-Blueprints#454, NVIDIA-AI-Blueprints#455) and configurable shallow citation enforcement (NVIDIA-AI-Blueprints#456). No textual conflicts: upstream's change to _extract_title_for_url (HTML-escaped URL matching) is disjoint from this branch's citation-verification disposition helpers in the same file. Signed-off-by: Ranit Karmakar <karmakarranit6@gmail.com>
Signed-off-by: Ranit Karmakar <karmakarranit6@gmail.com>
|
Thanks again for working through this and for keeping the branch updated. Since this PR was opened, #397/#405 changed the empty-source behavior: AI-Q now preserves the sanitized report, returns an actionable typed failure, and makes the report available through the report endpoint. That addresses the original report-loss problem. After the latest merges, this PR also no longer removes the empty-source exception—the agent still raises before the new verification status is set. The remaining changes are now fairly broad, and there are a couple of unresolved concerns, including empty-source reports not receiving the new status and verification-disabled reports potentially being promoted to “verified” during follow-up. Given how the underlying behavior has evolved, I’m going to close this PR as no longer needed. If you can still reproduce a user-facing gap on the current develop branch, please reopen the issue (or open a focused new one) with the current reproduction steps. We’d be happy to look at a smaller change targeted specifically at that remaining behavior. |
3 participants
Summary by CodeRabbit
citation_verification_statusand consistently surface the related warning in returned report text.unverifieddisposition.