fix(research): improve empty-source outcomes and artifact docs - #405
Conversation
|
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 (2)
📜 Recent review details⏰ Context from checks skipped due to timeout. (2)
🧰 Additional context used📓 Path-based instructions (4)**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/aiq_agent/**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/aiq_agent/agents/**/*⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (1)📚 Learning: 2026-07-06T23:55:42.908ZApplied to files:
🔇 Additional comments (2)
WalkthroughThis change adds typed empty-source failure classification, validates research source configuration earlier, preserves sanitized generated reports, persists terminal job outcomes, and documents synchronous versus persistent artifact workflows. It also updates Markdown link-check exclusions. ChangesEmpty-Source Failure Handling
Markdown Link Checker
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
4cc3d8f to
ecf0402
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
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/register.py (1)
245-291: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSee consolidated comment on
agent.pylines 281-301: this_run's catch-all re-logs the sameEmptySourceRegistryErrorat ERROR level a second time.🤖 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/register.py` around lines 245 - 291, The catch-all exception handler in the deep research execution flow re-logs EmptySourceRegistryError after it has already been logged elsewhere. Update the exception handling around active_agent.run in _run to avoid duplicate ERROR-level logging for that exception while preserving propagation and existing logging for other failures.
🤖 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/deep_researcher/agent.py`:
- Around line 281-301: Prevent EmptySourceRegistryError from being logged twice
at ERROR level: update the exception handling in the surrounding method and
deep_research_agent._run to re-raise this typed expected error before the
generic Exception handlers, avoiding logger.error/logger.exception for it while
preserving existing handling for other exceptions.
- Around line 253-256: Update the validation call in the deep researcher run
flow to avoid default noisy logging when checking self.tools on every run. Use
the validator’s quiet or non-logging option while preserving source selection
and availability validation in validate_research_source_configuration.
In `@src/aiq_agent/agents/shallow_researcher/register.py`:
- Line 178: Update the shallow-research registration flow around
validate_research_source_configuration so it does not trigger the full
tool-availability log sweep on every request. Reuse the consolidated validation
behavior defined in tool_validation.py, preserving source-configuration
validation while avoiding repeated selected_tools availability logging.
In `@src/aiq_agent/common/tool_validation.py`:
- Around line 103-131: The validate_research_source_configuration path should
avoid noisy per-request logging from validate_tool_availability. Update its call
to explicitly disable the helper’s default logging while preserving the existing
validation results and EmptySourceRegistryError behavior.
---
Outside diff comments:
In `@src/aiq_agent/agents/deep_researcher/register.py`:
- Around line 245-291: The catch-all exception handler in the deep research
execution flow re-logs EmptySourceRegistryError after it has already been logged
elsewhere. Update the exception handling around active_agent.run in _run to
avoid duplicate ERROR-level logging for that exception while preserving
propagation and existing logging for other failures.
🪄 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: 40177cbe-1353-43fe-a1ae-4358148f81e8
📒 Files selected for processing (20)
ci/markdown-link-check-config.jsondocs/source/examples/skills-sandbox/index.mddocs/source/integration/rest-api.mdfrontends/aiq_api/src/aiq_api/jobs/callbacks.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/tests/test_content_encryption_routes.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/register.pysrc/aiq_agent/agents/shallow_researcher/agent.pysrc/aiq_agent/agents/shallow_researcher/register.pysrc/aiq_agent/common/__init__.pysrc/aiq_agent/common/citation_verification.pysrc/aiq_agent/common/tool_validation.pytests/aiq_agent/agents/chat_researcher/test_agent.pytests/aiq_agent/agents/deep_researcher/test_agent.pytests/aiq_agent/agents/shallow_researcher/test_agent.pytests/aiq_agent/agents/shallow_researcher/test_register_per_user_mcp.pytests/aiq_agent/common/test_citation_verification.pytests/aiq_agent/jobs/test_runner.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (8)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Format and lint Python with Ruff using a 120-character line length, Python 3.11 target, rules E/F/W/I/PL/UP, and single-line imports; avoid reformatting unrelated code.
Never print or log secret values, including through tool output or error messages.
**/*.py: Runuv run ruff check .anduv run ruff format --check .for root Python changes.
Runuv run pytestfor root project Python changes.
Files:
src/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/common/__init__.pysrc/aiq_agent/common/tool_validation.pytests/aiq_agent/common/test_citation_verification.pysrc/aiq_agent/common/citation_verification.pysrc/aiq_agent/agents/shallow_researcher/register.pyfrontends/aiq_api/src/aiq_api/jobs/callbacks.pytests/aiq_agent/jobs/test_runner.pytests/aiq_agent/agents/shallow_researcher/test_register_per_user_mcp.pyfrontends/aiq_api/tests/test_content_encryption_routes.pysrc/aiq_agent/agents/shallow_researcher/agent.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pytests/aiq_agent/agents/chat_researcher/test_agent.pysrc/aiq_agent/agents/deep_researcher/register.pytests/aiq_agent/agents/shallow_researcher/test_agent.pytests/aiq_agent/agents/deep_researcher/test_agent.pysrc/aiq_agent/agents/deep_researcher/agent.py
src/aiq_agent/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/aiq_agent/**/*.py: Register new data sources indata_source_registryso the UI can toggle them.
Missing-secret paths must degrade gracefully by stubbing or skipping rather than crashing or leaking information.
Respect authenticated data sources by honoringrequires_auth, passing through per-user tokens, using backend token validators, and applying owner guardrails before loading protected report or artifact context into an agent.
Do not weaken or bypassAuthMiddleware, token validators, or authentication gating without prior design discussion.
Files:
src/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/common/__init__.pysrc/aiq_agent/common/tool_validation.pysrc/aiq_agent/common/citation_verification.pysrc/aiq_agent/agents/shallow_researcher/register.pysrc/aiq_agent/agents/shallow_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/register.pysrc/aiq_agent/agents/deep_researcher/agent.py
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Keep changes inside this repository, avoid editing adjacent repositories, and scope changes to the smallest relevant independent package, especially undersources/.
Run the narrowest relevant validation command first and broaden to the full suite only when a change crosses shared boundaries.
Keep pull requests scoped, exclude unrelated files and generated artifacts, never include secrets, and provide validation commands and results.
**/*: Do not include secrets, credentials, private hostnames, internal-only logs, customer data, or generated local artifacts.
Add or update tests for behavior changes.
Files:
src/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/common/__init__.pyci/markdown-link-check-config.jsondocs/source/integration/rest-api.mdsrc/aiq_agent/common/tool_validation.pytests/aiq_agent/common/test_citation_verification.pysrc/aiq_agent/common/citation_verification.pysrc/aiq_agent/agents/shallow_researcher/register.pyfrontends/aiq_api/src/aiq_api/jobs/callbacks.pytests/aiq_agent/jobs/test_runner.pytests/aiq_agent/agents/shallow_researcher/test_register_per_user_mcp.pyfrontends/aiq_api/tests/test_content_encryption_routes.pysrc/aiq_agent/agents/shallow_researcher/agent.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pytests/aiq_agent/agents/chat_researcher/test_agent.pysrc/aiq_agent/agents/deep_researcher/register.pytests/aiq_agent/agents/shallow_researcher/test_agent.pytests/aiq_agent/agents/deep_researcher/test_agent.pydocs/source/examples/skills-sandbox/index.mdsrc/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/chat_researcher/agent.pysrc/aiq_agent/agents/shallow_researcher/register.pysrc/aiq_agent/agents/shallow_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/register.pysrc/aiq_agent/agents/deep_researcher/agent.py
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock,mcp/pyproject.toml,mcp/uv.lock}
⚙️ CodeRabbit configuration file
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock,mcp/pyproject.toml,mcp/uv.lock}: Review automation and packaging changes for least-privilege permissions, pinned versions where appropriate,
copy-pr-bot pull-request/ branch behavior, reproducible uv/npm setup, secret handling, and consistency with
the documented validation matrix.
Files:
ci/markdown-link-check-config.json
docs/source/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update canonical documentation under
docs/source/when behavior, configuration, or workflows change; do not duplicate full documentation pages into skills.
Files:
docs/source/integration/rest-api.mddocs/source/examples/skills-sandbox/index.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/integration/rest-api.mddocs/source/examples/skills-sandbox/index.md
{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/callbacks.pyfrontends/aiq_api/src/aiq_api/jobs/runner.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_content_encryption_routes.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/shallow_researcher/test_agent.pytests/aiq_agent/agents/deep_researcher/test_agent.py
🪛 ast-grep (0.45.0)
frontends/aiq_api/tests/test_content_encryption_routes.py
[info] 613-618: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{
"report": "# Preserved generated answer",
"outcome_reason": outcome_reason,
}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🔇 Additional comments (25)
ci/markdown-link-check-config.json (1)
38-40: LGTM!src/aiq_agent/common/citation_verification.py (1)
43-43: LGTM!Also applies to: 109-144, 153-174
src/aiq_agent/agents/deep_researcher/agent.py (1)
33-33: LGTM!Also applies to: 331-331
src/aiq_agent/agents/shallow_researcher/agent.py (1)
360-396: LGTM!tests/aiq_agent/agents/deep_researcher/test_agent.py (1)
45-46: LGTM!Also applies to: 356-432, 1331-1435
tests/aiq_agent/agents/shallow_researcher/test_agent.py (1)
33-33: LGTM!Also applies to: 46-51, 821-905
src/aiq_agent/agents/deep_researcher/register.py (1)
34-35: LGTM!Also applies to: 321-328
src/aiq_agent/agents/shallow_researcher/register.py (1)
29-30: LGTM!Also applies to: 109-110, 209-215
src/aiq_agent/agents/chat_researcher/agent.py (1)
225-228: LGTM!Also applies to: 345-348
tests/aiq_agent/agents/shallow_researcher/test_register_per_user_mcp.py (1)
24-25: LGTM!Also applies to: 103-157
src/aiq_agent/common/__init__.py (1)
62-62: LGTM!Also applies to: 98-98
tests/aiq_agent/common/test_citation_verification.py (1)
22-27: LGTM!Also applies to: 43-113
tests/aiq_agent/agents/chat_researcher/test_agent.py (1)
31-32: LGTM!Also applies to: 245-316
frontends/aiq_api/src/aiq_api/jobs/runner.py (4)
244-296: LGTM!
298-344: LGTM!
1067-1084: LGTM!
38-41: LGTM!frontends/aiq_api/src/aiq_api/jobs/callbacks.py (1)
129-137: LGTM!Also applies to: 393-393
frontends/aiq_api/tests/test_content_encryption_routes.py (1)
83-91: LGTM!Also applies to: 121-127, 594-639
tests/aiq_agent/jobs/test_runner.py (3)
93-102: LGTM!
924-1235: LGTM!
2495-2544: LGTM!docs/source/examples/skills-sandbox/index.md (2)
109-113: LGTM!Also applies to: 122-147, 156-182, 279-282
183-186: 📐 Maintainability & Code Quality
../../integration/rest-api.md#durable-sandbox-artifactsis a valid link.> Likely an incorrect or invalid review comment.docs/source/integration/rest-api.md (1)
207-213: LGTM!
## Summary - classify empty-source research outcomes and return actionable, sanitized responses - persist typed async job failures safely and add regression coverage across research workflows - clarify that direct `nat run` artifacts are non-persistent and document the `nat serve` job/artifact retrieval workflow ## Validation - Sphinx HTML build passed - Sphinx linkcheck passed ## Related issue - https://linear.app/nvidia/issue/AIQ-27/docs-clarify-generated-artifact-persistence-requires-nat-servejob Signed-off-by: Tanner Leach <tanleach@users.noreply.github.com> ## Summary by CodeRabbit * **New Features** * Research runs now classify empty-source situations (no sources selected, tools unavailable, or no results) and return consistent, user-friendly remediation messages. * Failed jobs with actionable source-selection issues preserve sanitized answers and final report artifacts for later retrieval. * **Bug Fixes** * Job failures continue to avoid exposing internal exception details or plaintext output. * **Documentation** * Clarified REST “Get Job Status” semantics for typed source-condition failures and refined the skills-sandbox async/persistence example. * **Tests** * Expanded coverage for empty-source handling, public-facing messaging, and encrypted report/error preservation in API/job flows. Authors: - Tanner Leach (https://github.com/tanleach) Approvers: - Ajay Thorve (https://github.com/AjayThorve) URL: #397 (cherry picked from commit aa06ef8)
ecf0402 to
ec590bd
Compare
Signed-off-by: Tanner Leach <tleach@nvidia.com>
|
/ok to test 690d8a8 |
|
/merge |
2 participants
Summary
Backports #397 to release/2.2, including actionable empty-source outcomes, safe async failure persistence, regression coverage, and artifact workflow documentation.
Verification
Summary by CodeRabbit