Add report follow-up jobs and chat routing - #271
Conversation
c6bdbd6 to
733511b
Compare
|
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. |
|
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:
WalkthroughAdds a full-stack "report follow-up / report edit" capability. A new internal ChangesReport Follow-up / Edit Feature
Sequence Diagram(s)sequenceDiagram
participant UI as Frontend
participant WS as WebSocket
participant Chat as ChatResearcherAgent
participant Classifier as IntentClassifier
participant EditAPI as POST /report/edit
participant Submit as submit_agent_job
participant Rewriter as ReportRewriterAgent
UI->>WS: sendMessage(text, sources, activeReportJobId)
WS->>Chat: payload {query, active_report_job_id}
Chat->>Classifier: run(state with active_report_job_id)
Classifier-->>Chat: IntentResult {target: "report", report_action: "edit"}
Chat->>Chat: route_after_orchestration → report_edit_node
Chat->>EditAPI: _submit_report_edit_job(state)
EditAPI->>EditAPI: authorize_job_access + verify SUCCESS
EditAPI->>EditAPI: resolve_report_context → ReportContext
EditAPI->>EditAPI: to_initial_files + report_output_metadata
EditAPI->>Submit: submit_agent_job(report_rewriter, allow_internal=True, initial_files, output_metadata)
Submit->>Rewriter: run(state with seeded files)
Rewriter->>Rewriter: read files, render edit.j2, invoke LLM
Rewriter-->>Submit: updated state with revised report
Submit-->>EditAPI: child_job_id
EditAPI-->>Chat: "Report edit job submitted: <child_job_id>"
Chat-->>UI: SSE escalation → deep-research banner + streaming
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 13
🤖 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 `@ci/markdown-link-check-config.json`:
- Around line 11-16: The markdown-link-check configuration currently uses overly
broad regex patterns that ignore all URLs from the FastAPI and Dask domains (the
patterns starting with `^https?://fastapi\\.tiangolo\\.com` and
`^https?://(www\\.)?dask\\.org`). Replace these domain-wide ignore patterns with
more specific rules that target only the particular flaky URLs or exact paths
that are known to cause issues, rather than disabling validation for entire
domains. This will preserve the ability to catch real documentation regressions
while still allowing legitimate URLs from these sources to be validated.
In `@docs/source/integration/rest-api.md`:
- Around line 140-145: The HTTP status code table for the report-edit endpoint
is missing documentation for the 500 error case that can be returned by the API.
Add a new row to the status table with status code 500 and its corresponding
reason explaining that the report edit job submission failed, ensuring the
documentation accurately reflects all possible API responses including the
"Failed to submit report edit job" error case.
In `@frontends/aiq_api/src/aiq_api/jobs/report_context.py`:
- Around line 206-208: The EventStore.get_events_async call at line 207 lacks
error handling for transient failures. Wrap this call in a try-except block to
catch exceptions from the async call. When an exception occurs, check if the
report is already available from job output; if it is, gracefully skip the event
source extraction and continue with report-only sources by having
_sources_from_events operate on an empty or unavailable events list. Only
propagate the error if report is not available, since in that case there's no
fallback for source extraction.
In `@frontends/aiq_api/src/aiq_api/jobs/runner.py`:
- Around line 525-528: The update() call on line 527 blindly merges
output_metadata into the output dictionary, which allows the "report" key to be
overwritten if output_metadata contains a "report" entry. Before calling
output.update(output_metadata), filter or validate the output_metadata to
exclude the "report" key to prevent collisions, ensuring that the canonical
report field persists unchanged in the output that gets passed to
job_store.update_status().
In `@frontends/aiq_api/src/aiq_api/routes/jobs.py`:
- Around line 285-297: The `input` field definition currently only validates
`min_length=1`, which allows whitespace-only strings to pass validation. Since
downstream report rewriting requires non-empty stripped instructions, add a
custom validator to the `input` field that strips whitespace and rejects the
request if the resulting string is empty. This ensures whitespace-only edit
instructions are caught at the API boundary before creating failing jobs
downstream. Apply the same validation logic to the corresponding field mentioned
at lines 552-576.
- Around line 581-583: The exception handler catching generic RuntimeError and
mapping it to HTTP 403 misclassifies non-authorization failures as permission
errors, violating the API error-contract clarity. In the report-edit submission
route (around the except RuntimeError as e block), replace the overly broad
RuntimeError handler with either a more specific exception type that actually
represents authorization failures or use a more appropriate HTTP status code
(such as 500 for internal/runtime errors or 400 for bad requests depending on
the actual error source). Ensure the HTTP status code accurately reflects the
type of failure being handled.
In `@frontends/aiq_api/tests/test_report_edit.py`:
- Around line 186-206: The test function
test_job_report_response_includes_report_interaction_metadata validates the
response JSON but does not verify that the _authorize_job_access mock was
invoked with the correct job ID. Add an assertion after the existing response
validations to confirm that _authorize_job_access was called with "child-job-1"
as the argument, ensuring that access control checks are properly enforced on
the GET report endpoint and preventing regressions where authorization might be
bypassed.
In `@src/aiq_agent/agents/chat_researcher/agent.py`:
- Around line 337-355: The logger.warning calls in the report_ask_node and
report_edit_node functions are logging raw exception objects using the %s format
specifier, which can expose sensitive information from upstream errors. Remove
the exception parameter e from both logger.warning calls (at line 340 in
report_ask_node and at line 354 in report_edit_node) to prevent logging raw
exception text. Keep only the descriptive message portion without the exception
details in each log statement.
In `@src/aiq_agent/agents/chat_researcher/nodes/intent_classifier.py`:
- Line 172: The issue is that bool(parsed.get("use_parent_report_context")) at
line 172 will incorrectly evaluate the string "false" as True because any
non-empty string is truthy in Python. The LLM output can return string booleans
which need proper normalization. Replace the bool() conversion with logic that
checks if the value is a string and compares it to "true" case-insensitively,
otherwise convert it to a boolean normally. This same fix also needs to be
applied to the additional locations mentioned in the comment at lines 258-265
where similar boolean normalization is needed.
In `@src/aiq_agent/agents/chat_researcher/register.py`:
- Around line 91-96: The llm.ainvoke call at line 91 lacks timeout protection,
which could cause the entire chat turn to block if the LLM provider stalls. Wrap
the await llm.ainvoke([HumanMessage(content=prompt)]) call with an appropriate
timeout mechanism (such as asyncio.wait_for) to ensure the operation completes
within a reasonable timeframe and fails gracefully if the provider does not
respond in time, allowing the chat interaction to degrade gracefully instead of
blocking indefinitely.
- Around line 386-393: The parent report context resolution at
_resolve_report_context_for_state(state) is currently an unguarded operation
that will propagate exceptions and fail the entire deep-job submission if parent
context lookup encounters errors like 409 or 503 responses. Wrap the entire
parent context resolution block (starting from the report_context import through
output_metadata assignment) in a try-except handler that catches exceptions,
logs the error for observability, and allows the function to continue with async
deep-job submission without parent context enrichment rather than aborting the
workflow. This way, parent context becomes an optional enrichment path rather
than a required workflow dependency.
In `@src/aiq_agent/agents/chat_researcher/utils.py`:
- Around line 153-155: The issue is that the `or` operator treats empty lists as
falsy, causing explicitly provided empty data_sources lists to be replaced with
values from fallback sources. This affects the parse_data_sources calls at lines
153-155, 176-177, and 199-200. Instead of using `or`, you need to explicitly
check if the first parse_data_sources result is None (not provided) before
falling back to the next source. Replace each occurrence where
parse_data_sources is chained with `or` by checking if the first result is not
None; only use the second source when the first result is actually None,
allowing empty lists to be preserved as the caller's explicit choice.
In `@src/aiq_agent/agents/report_rewriter/agent.py`:
- Around line 111-114: Remove the `break` statement from the callback dispatch
loop in the code block where callbacks with the `emit_final_report` method are
being called. The `break` statement currently exits the loop after the first
matching callback, preventing other registered callbacks from receiving the
final report event. Delete the `break` line to allow all callbacks that have the
`emit_final_report` method to process the revised_report.
🪄 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: cc9a634e-c4d9-4256-97d7-82b37de745f8
📒 Files selected for processing (37)
ci/markdown-link-check-config.jsondocs/source/integration/rest-api.mdfrontends/aiq_api/src/aiq_api/jobs/report_context.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/jobs/submit.pyfrontends/aiq_api/src/aiq_api/registry.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/tests/test_agent_registry_visibility.pyfrontends/aiq_api/tests/test_job_access.pyfrontends/aiq_api/tests/test_job_submit_data_sources.pyfrontends/aiq_api/tests/test_report_context.pyfrontends/aiq_api/tests/test_report_edit.pyfrontends/aiq_api/tests/test_submit_collision.pyfrontends/aiq_api/tests/test_submit_internal_agent.pyfrontends/ui/src/adapters/api/websocket-client.spec.tsfrontends/ui/src/adapters/api/websocket-client.tsfrontends/ui/src/features/chat/hooks/use-websocket-chat.spec.tsfrontends/ui/src/features/chat/hooks/use-websocket-chat.tssrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/models/intent.pysrc/aiq_agent/agents/chat_researcher/models/state.pysrc/aiq_agent/agents/chat_researcher/nodes/intent_classifier.pysrc/aiq_agent/agents/chat_researcher/prompts/intent_classification.j2src/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/agents/chat_researcher/utils.pysrc/aiq_agent/agents/report_rewriter/__init__.pysrc/aiq_agent/agents/report_rewriter/agent.pysrc/aiq_agent/agents/report_rewriter/models.pysrc/aiq_agent/agents/report_rewriter/prompts/edit.j2tests/aiq_agent/agents/chat_researcher/models/test_intent.pytests/aiq_agent/agents/chat_researcher/models/test_state.pytests/aiq_agent/agents/chat_researcher/nodes/test_intent_classifier.pytests/aiq_agent/agents/chat_researcher/test_agent.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.pytests/aiq_agent/agents/chat_researcher/test_utils.pytests/aiq_agent/agents/report_rewriter/test_agent.pytests/aiq_agent/jobs/test_runner.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: UI Unit Tests
🧰 Additional context used
📓 Path-based instructions (12)
**/*.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/report_rewriter/__init__.pytests/aiq_agent/agents/chat_researcher/models/test_state.pysrc/aiq_agent/agents/chat_researcher/models/state.pyfrontends/aiq_api/tests/test_agent_registry_visibility.pytests/aiq_agent/agents/report_rewriter/test_agent.pyfrontends/aiq_api/tests/test_submit_internal_agent.pyfrontends/aiq_api/tests/test_job_submit_data_sources.pyfrontends/aiq_api/tests/test_report_context.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.pyfrontends/aiq_api/tests/test_submit_collision.pysrc/aiq_agent/agents/chat_researcher/models/intent.pyfrontends/aiq_api/tests/test_report_edit.pysrc/aiq_agent/agents/report_rewriter/agent.pytests/aiq_agent/agents/chat_researcher/models/test_intent.pysrc/aiq_agent/agents/report_rewriter/models.pytests/aiq_agent/agents/chat_researcher/test_utils.pyfrontends/aiq_api/src/aiq_api/registry.pyfrontends/aiq_api/tests/test_job_access.pytests/aiq_agent/agents/chat_researcher/test_agent.pytests/aiq_agent/jobs/test_runner.pysrc/aiq_agent/agents/chat_researcher/nodes/intent_classifier.pyfrontends/aiq_api/src/aiq_api/jobs/submit.pyfrontends/aiq_api/src/aiq_api/jobs/report_context.pytests/aiq_agent/agents/chat_researcher/nodes/test_intent_classifier.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/utils.pysrc/aiq_agent/agents/chat_researcher/register.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/routes/jobs.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/__init__.pysrc/aiq_agent/agents/chat_researcher/models/state.pysrc/aiq_agent/agents/chat_researcher/models/intent.pysrc/aiq_agent/agents/report_rewriter/agent.pysrc/aiq_agent/agents/report_rewriter/models.pysrc/aiq_agent/agents/chat_researcher/nodes/intent_classifier.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/utils.pysrc/aiq_agent/agents/chat_researcher/register.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/report_rewriter/__init__.pysrc/aiq_agent/agents/report_rewriter/prompts/edit.j2tests/aiq_agent/agents/chat_researcher/models/test_state.pysrc/aiq_agent/agents/chat_researcher/models/state.pyfrontends/aiq_api/tests/test_agent_registry_visibility.pyfrontends/ui/src/adapters/api/websocket-client.tstests/aiq_agent/agents/report_rewriter/test_agent.pyfrontends/aiq_api/tests/test_submit_internal_agent.pyci/markdown-link-check-config.jsonfrontends/aiq_api/tests/test_job_submit_data_sources.pyfrontends/aiq_api/tests/test_report_context.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.pyfrontends/aiq_api/tests/test_submit_collision.pyfrontends/ui/src/adapters/api/websocket-client.spec.tssrc/aiq_agent/agents/chat_researcher/models/intent.pysrc/aiq_agent/agents/chat_researcher/prompts/intent_classification.j2frontends/aiq_api/tests/test_report_edit.pysrc/aiq_agent/agents/report_rewriter/agent.pytests/aiq_agent/agents/chat_researcher/models/test_intent.pysrc/aiq_agent/agents/report_rewriter/models.pytests/aiq_agent/agents/chat_researcher/test_utils.pydocs/source/integration/rest-api.mdfrontends/aiq_api/src/aiq_api/registry.pyfrontends/ui/src/features/chat/hooks/use-websocket-chat.spec.tsfrontends/aiq_api/tests/test_job_access.pytests/aiq_agent/agents/chat_researcher/test_agent.pytests/aiq_agent/jobs/test_runner.pysrc/aiq_agent/agents/chat_researcher/nodes/intent_classifier.pyfrontends/ui/src/features/chat/hooks/use-websocket-chat.tsfrontends/aiq_api/src/aiq_api/jobs/submit.pyfrontends/aiq_api/src/aiq_api/jobs/report_context.pytests/aiq_agent/agents/chat_researcher/nodes/test_intent_classifier.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/utils.pysrc/aiq_agent/agents/chat_researcher/register.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/routes/jobs.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/__init__.pysrc/aiq_agent/agents/report_rewriter/prompts/edit.j2src/aiq_agent/agents/chat_researcher/models/state.pysrc/aiq_agent/agents/chat_researcher/models/intent.pysrc/aiq_agent/agents/chat_researcher/prompts/intent_classification.j2src/aiq_agent/agents/report_rewriter/agent.pysrc/aiq_agent/agents/report_rewriter/models.pysrc/aiq_agent/agents/chat_researcher/nodes/intent_classifier.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/utils.pysrc/aiq_agent/agents/chat_researcher/register.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/agents/chat_researcher/models/test_state.pyfrontends/aiq_api/tests/test_agent_registry_visibility.pytests/aiq_agent/agents/report_rewriter/test_agent.pyfrontends/aiq_api/tests/test_submit_internal_agent.pyfrontends/aiq_api/tests/test_job_submit_data_sources.pyfrontends/aiq_api/tests/test_report_context.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.pyfrontends/aiq_api/tests/test_submit_collision.pyfrontends/aiq_api/tests/test_report_edit.pytests/aiq_agent/agents/chat_researcher/models/test_intent.pytests/aiq_agent/agents/chat_researcher/test_utils.pyfrontends/aiq_api/tests/test_job_access.pytests/aiq_agent/agents/chat_researcher/test_agent.pytests/aiq_agent/jobs/test_runner.pytests/aiq_agent/agents/chat_researcher/nodes/test_intent_classifier.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/websocket-client.tsfrontends/ui/src/adapters/api/websocket-client.spec.tsfrontends/ui/src/features/chat/hooks/use-websocket-chat.spec.tsfrontends/ui/src/features/chat/hooks/use-websocket-chat.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/websocket-client.tsfrontends/ui/src/adapters/api/websocket-client.spec.tsfrontends/ui/src/features/chat/hooks/use-websocket-chat.spec.tsfrontends/ui/src/features/chat/hooks/use-websocket-chat.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/websocket-client.tsfrontends/ui/src/adapters/api/websocket-client.spec.tsfrontends/ui/src/features/chat/hooks/use-websocket-chat.spec.tsfrontends/ui/src/features/chat/hooks/use-websocket-chat.ts
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock}
⚙️ CodeRabbit configuration file
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,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 the docs under docs/source/ when behavior, configuration, or workflows change
Files:
docs/source/integration/rest-api.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.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/registry.pyfrontends/aiq_api/src/aiq_api/jobs/submit.pyfrontends/aiq_api/src/aiq_api/jobs/report_context.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/routes/jobs.py
🧠 Learnings (1)
📚 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_agent_registry_visibility.pyfrontends/aiq_api/tests/test_submit_internal_agent.pyfrontends/aiq_api/tests/test_job_submit_data_sources.pyfrontends/aiq_api/tests/test_report_context.pyfrontends/aiq_api/tests/test_submit_collision.pyfrontends/aiq_api/tests/test_report_edit.pyfrontends/aiq_api/tests/test_job_access.py
🪛 ast-grep (0.44.0)
frontends/aiq_api/src/aiq_api/jobs/report_context.py
[warning] 95-95: Do not make http calls without encryption
Context: "http://"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
🔇 Additional comments (35)
src/aiq_agent/agents/report_rewriter/__init__.py (1)
1-10: LGTM!src/aiq_agent/agents/report_rewriter/models.py (1)
1-32: LGTM!src/aiq_agent/agents/report_rewriter/agent.py (1)
1-110: LGTM!Also applies to: 116-123
src/aiq_agent/agents/report_rewriter/prompts/edit.j2 (1)
1-24: LGTM!tests/aiq_agent/agents/report_rewriter/test_agent.py (1)
1-65: LGTM!frontends/aiq_api/src/aiq_api/jobs/report_context.py (1)
1-205: LGTM!Also applies to: 209-260
frontends/aiq_api/tests/test_report_context.py (1)
1-173: LGTM!frontends/ui/src/adapters/api/websocket-client.ts (1)
226-237: LGTM!frontends/ui/src/adapters/api/websocket-client.spec.ts (1)
203-224: LGTM!frontends/ui/src/features/chat/hooks/use-websocket-chat.ts (5)
71-77: LGTM!
126-137: LGTM!
558-569: LGTM!
619-640: LGTM!
1190-1190: LGTM!frontends/ui/src/features/chat/hooks/use-websocket-chat.spec.ts (2)
346-376: LGTM!
1243-1314: LGTM!frontends/aiq_api/src/aiq_api/registry.py (1)
44-46: LGTM!Also applies to: 57-57, 67-67, 81-81, 119-126
frontends/aiq_api/src/aiq_api/jobs/submit.py (1)
27-29: LGTM!Also applies to: 44-61, 144-146, 165-166, 188-192, 249-264, 284-308
frontends/aiq_api/tests/test_agent_registry_visibility.py (1)
13-22: LGTM!Also applies to: 24-37, 39-47, 49-90
frontends/aiq_api/tests/test_submit_internal_agent.py (1)
18-33: LGTM!Also applies to: 35-66
frontends/aiq_api/tests/test_submit_collision.py (1)
50-70: LGTM!Also applies to: 72-94, 96-119
frontends/aiq_api/tests/test_job_access.py (1)
115-133: LGTM!Also applies to: 175-194
frontends/aiq_api/tests/test_job_submit_data_sources.py (1)
146-170: LGTM!frontends/aiq_api/src/aiq_api/jobs/runner.py (1)
250-251: LGTM!Also applies to: 283-284, 491-492, 660-661, 692-699, 718-719
tests/aiq_agent/jobs/test_runner.py (1)
438-472: LGTM!Also applies to: 1496-1595
src/aiq_agent/agents/chat_researcher/models/intent.py (1)
23-25: LGTM!Also applies to: 38-40
src/aiq_agent/agents/chat_researcher/models/state.py (1)
64-64: LGTM!tests/aiq_agent/agents/chat_researcher/models/test_intent.py (1)
37-41: LGTM!Also applies to: 42-54, 55-59
tests/aiq_agent/agents/chat_researcher/models/test_state.py (1)
109-109: LGTM!Also applies to: 129-137
src/aiq_agent/agents/chat_researcher/utils.py (1)
21-33: LGTM!Also applies to: 109-139, 210-214
tests/aiq_agent/agents/chat_researcher/test_utils.py (1)
25-25: LGTM!Also applies to: 202-237
src/aiq_agent/agents/chat_researcher/prompts/intent_classification.j2 (1)
19-32: LGTM!Also applies to: 41-45, 57-63
tests/aiq_agent/agents/chat_researcher/nodes/test_intent_classifier.py (1)
107-217: LGTM!tests/aiq_agent/agents/chat_researcher/test_agent.py (1)
246-421: LGTM!tests/aiq_agent/agents/chat_researcher/test_register_helpers.py (1)
20-95: LGTM!
Resolve the 13 actionable CodeRabbit comments on PR NVIDIA-AI-Blueprints#271: - routes/jobs.py: reject whitespace-only `input` on report-edit and submit requests (422) via a field validator; map post-auth RuntimeError to 503 (availability) instead of 403, without echoing internal detail. - runner.py: build job output so the canonical `report` can never be overwritten by a stray `report` key in output_metadata. - report_context.py: tolerate a transient event-log fetch failure when the report is already available (fall back to report-only sources). - register.py: bound the report-ask LLM call with a timeout so a stalled provider degrades gracefully; make parent-report context enrichment optional for delta research (don't abort the deep job if context lookup fails). - intent_classifier.py: normalize string booleans for use_parent_report_context ("false" must not be truthy). - utils.py: preserve an explicit empty data_sources list ([] = no data-source tools) instead of falling through to other sources. - agent.py: log error type, not raw exception text, in report follow-up paths. - report_rewriter/agent.py: dispatch the final report to all registered callbacks (remove early break). - ci/markdown-link-check-config.json: narrow the FastAPI/Dask ignores to the exact referenced URLs instead of whole domains. - docs: document the report-edit 500 case. - tests: assert authorization on the GET report route; add coverage for blank input (422), string-boolean normalization, and empty data_sources preservation. Backend suites (aiq_api + aiq_agent) and UI specs pass; ruff and tsc clean. Verified live: blank input -> 422, report ask answers, job_id collision -> 409. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Ajay Thorve <athorve@nvidia.com>
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/integration/rest-api.md (1)
80-102: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument the new non-blank validation and 422 responses.
Both request models now strip and reject whitespace-only
input, but the docs still say only “min 1 character,” and report-edit omits the resulting422validation case.As per path instructions, “Review documentation for command accuracy ... stale examples.”
Suggested docs update
-| `input` | `string` | Yes | Research query (min 1 character) | +| `input` | `string` | Yes | Research query. Must be non-blank after trimming | @@ -| `422` | One or more unknown data source IDs. Response `detail` includes `message`, `invalid_ids`, and `known_ids` for client-side recovery UX | +| `422` | Validation error, including blank/whitespace-only `input`, invalid request fields, or one or more unknown data source IDs. Data source errors include `message`, `invalid_ids`, and `known_ids` for client-side recovery UX | @@ -| `input` | `string` | Yes | Edit instruction for the parent report (min 1 character) | +| `input` | `string` | Yes | Edit instruction for the parent report. Must be non-blank after trimming | @@ | `409` | Parent job is incomplete, has no durable report, or the supplied child `job_id` collides | +| `422` | Validation error, including blank/whitespace-only `input`, invalid child `job_id`, or invalid `expiry_seconds` | | `500` | Failed to submit the report edit job |Also applies to: 120-145
🤖 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/integration/rest-api.md` around lines 80 - 102, Update the API documentation to reflect the new whitespace validation for the `input` parameter. First, modify the `input` parameter description to clarify that it rejects whitespace-only strings in addition to requiring minimum 1 character. Second, add a 422 error response case to the error responses table that documents the validation failure when `input` contains only whitespace. This change should be applied to both sections mentioned in the comment (the main section and the additional section).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/integration/rest-api.md`:
- Around line 80-102: Update the API documentation to reflect the new whitespace
validation for the `input` parameter. First, modify the `input` parameter
description to clarify that it rejects whitespace-only strings in addition to
requiring minimum 1 character. Second, add a 422 error response case to the
error responses table that documents the validation failure when `input`
contains only whitespace. This change should be applied to both sections
mentioned in the comment (the main section and the additional section).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: f58391a2-9ae9-4551-a57e-4dfe594d430c
📒 Files selected for processing (13)
ci/markdown-link-check-config.jsondocs/source/integration/rest-api.mdfrontends/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_edit.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/nodes/intent_classifier.pysrc/aiq_agent/agents/chat_researcher/register.pysrc/aiq_agent/agents/chat_researcher/utils.pysrc/aiq_agent/agents/report_rewriter/agent.pytests/aiq_agent/agents/chat_researcher/nodes/test_intent_classifier.pytests/aiq_agent/agents/chat_researcher/test_utils.py
💤 Files with no reviewable changes (1)
- src/aiq_agent/agents/report_rewriter/agent.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**
⚙️ 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:
ci/markdown-link-check-config.jsondocs/source/integration/rest-api.mdtests/aiq_agent/agents/chat_researcher/test_utils.pysrc/aiq_agent/agents/chat_researcher/nodes/intent_classifier.pytests/aiq_agent/agents/chat_researcher/nodes/test_intent_classifier.pysrc/aiq_agent/agents/chat_researcher/utils.pyfrontends/aiq_api/src/aiq_api/jobs/report_context.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pysrc/aiq_agent/agents/chat_researcher/agent.pyfrontends/aiq_api/tests/test_report_edit.pysrc/aiq_agent/agents/chat_researcher/register.pyfrontends/aiq_api/src/aiq_api/routes/jobs.py
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock}
⚙️ CodeRabbit configuration file
{.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,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 the docs under docs/source/ when behavior, configuration, or workflows change
Files:
docs/source/integration/rest-api.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.md
**/*.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/chat_researcher/test_utils.pysrc/aiq_agent/agents/chat_researcher/nodes/intent_classifier.pytests/aiq_agent/agents/chat_researcher/nodes/test_intent_classifier.pysrc/aiq_agent/agents/chat_researcher/utils.pyfrontends/aiq_api/src/aiq_api/jobs/report_context.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pysrc/aiq_agent/agents/chat_researcher/agent.pyfrontends/aiq_api/tests/test_report_edit.pysrc/aiq_agent/agents/chat_researcher/register.pyfrontends/aiq_api/src/aiq_api/routes/jobs.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_utils.pytests/aiq_agent/agents/chat_researcher/nodes/test_intent_classifier.pyfrontends/aiq_api/tests/test_report_edit.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/nodes/intent_classifier.pysrc/aiq_agent/agents/chat_researcher/utils.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/register.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/nodes/intent_classifier.pysrc/aiq_agent/agents/chat_researcher/utils.pysrc/aiq_agent/agents/chat_researcher/agent.pysrc/aiq_agent/agents/chat_researcher/register.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/jobs/report_context.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/aiq_api/src/aiq_api/routes/jobs.py
🧠 Learnings (1)
📚 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_edit.py
🔇 Additional comments (11)
ci/markdown-link-check-config.json (1)
13-16: LGTM!src/aiq_agent/agents/chat_researcher/utils.py (1)
153-157: LGTM!Also applies to: 178-179, 202-203
tests/aiq_agent/agents/chat_researcher/test_utils.py (1)
239-256: LGTM!src/aiq_agent/agents/chat_researcher/nodes/intent_classifier.py (1)
172-172: LGTM!Also applies to: 268-280
tests/aiq_agent/agents/chat_researcher/nodes/test_intent_classifier.py (1)
28-48: LGTM!frontends/aiq_api/src/aiq_api/jobs/report_context.py (1)
207-213: LGTM!src/aiq_agent/agents/chat_researcher/agent.py (1)
88-92: LGTM!Also applies to: 109-110, 122-123, 332-364, 366-373, 419-420, 431-432, 446-447, 477-477
src/aiq_agent/agents/chat_researcher/register.py (1)
18-18: LGTM!Also applies to: 53-55, 96-100, 314-360, 399-419, 452-453, 517-560
frontends/aiq_api/src/aiq_api/jobs/runner.py (1)
230-252: LGTM!Also applies to: 280-284, 484-492, 525-528
frontends/aiq_api/src/aiq_api/routes/jobs.py (1)
46-46: LGTM!Also applies to: 91-98, 291-318, 390-394, 499-500, 535-542, 576-620, 749-759
frontends/aiq_api/tests/test_report_edit.py (1)
153-165: LGTM!Also applies to: 201-224
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/integration/rest-api.md (1)
42-45:⚠️ Potential issue | 🟡 MinorClarify that the internal-only agent error message includes the agent type name.
The documentation correctly states that internal agents are rejected with HTTP
400, but the error message format is incomplete. The actual error is"Agent type is internal-only: <agent_type>"(with the agent type interpolated), not just"Agent type is internal-only". Update line 42–45 to show this format for client-side error handling.The GET
/v1/jobs/async/agentsendpoint correctly filters to public agents only (confirmed byif config.publicat line 393 of the route handler), and the non-blank validation on bothJobSubmitRequestandReportEditRequestis correct.🤖 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/integration/rest-api.md` around lines 42 - 45, The error message format for internal-only agents in the documentation is incomplete. Update the text in lines 42-45 describing the POST /v1/jobs/async/submit endpoint error response to clarify that the actual error message includes the agent type name itself. Change the documented error message from "Agent type is internal-only" to "Agent type is internal-only: <agent_type>" (where agent_type is interpolated with the actual name) to accurately reflect what clients will receive when attempting to submit with an internal-only agent.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/integration/rest-api.md`:
- Around line 42-45: The error message format for internal-only agents in the
documentation is incomplete. Update the text in lines 42-45 describing the POST
/v1/jobs/async/submit endpoint error response to clarify that the actual error
message includes the agent type name itself. Change the documented error message
from "Agent type is internal-only" to "Agent type is internal-only:
<agent_type>" (where agent_type is interpolated with the actual name) to
accurately reflect what clients will receive when attempting to submit with an
internal-only agent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: c2744254-7423-436d-bb31-7b1760c09885
📒 Files selected for processing (1)
docs/source/integration/rest-api.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
docs/source/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update the docs under docs/source/ when behavior, configuration, or workflows change
Files:
docs/source/integration/rest-api.md
**
⚙️ 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:
docs/source/integration/rest-api.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.md
🔇 Additional comments (1)
docs/source/integration/rest-api.md (1)
120-120: No action required.ReportEditRequesthas an identical_input_not_blankvalidator toJobSubmitRequest(lines 308–315 offrontends/aiq_api/src/aiq_api/routes/jobs.py). The validator strips whitespace and rejects blank input, so the documentation claim is accurate and supported by the implementation.> Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontends/aiq_api/src/aiq_api/jobs/submit.py (1)
268-273: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not log raw rollback exceptions.
cleanup_errorcan include driver messages or DB connection details. Log only the exception type or a sanitized error code so rollback failures cannot leak secrets through logs.Proposed fix
- except Exception as cleanup_error: + except Exception as cleanup_error: logger.warning( - "Failed to roll back partial async job submission for %s: %s", + "Failed to roll back partial async job submission for %s (%s)", resolved_job_id, - cleanup_error, + type(cleanup_error).__name__, )As per coding guidelines, “Never print or log secret values, including in tool output or error messages.”
🤖 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 `@frontends/aiq_api/src/aiq_api/jobs/submit.py` around lines 268 - 273, The logger.warning call in the exception handler for cleanup_error is logging the raw exception, which can contain sensitive information such as database connection details or driver messages. Instead of logging cleanup_error directly, extract only the exception type (such as type(cleanup_error).__name__) or a sanitized error code and log that instead. This ensures that sensitive details are not leaked through logs while still providing enough information to identify that a rollback failure occurred.Source: Coding guidelines
🤖 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 `@frontends/aiq_api/src/aiq_api/jobs/access.py`:
- Around line 49-60: The _LATEST_REPORT_JOB_BASE query and its derived queries
(_LATEST_REPORT_JOB_SQL_ANY and _LATEST_REPORT_JOB_SQL_OWNED) currently select
any successful, non-expired job without filtering for report-producing jobs
specifically. This allows a newer non-report job to incorrectly become the
active_report_job_id. Add a filter condition to the WHERE clause in
_LATEST_REPORT_JOB_BASE to check for report-producing jobs using a marker such
as result_kind or agent type, ensuring both _LATEST_REPORT_JOB_SQL_ANY and
_LATEST_REPORT_JOB_SQL_OWNED inherit this filter. Additionally, add a regression
test that verifies a newer non-report job success does not win as the
active_report_job_id.
---
Outside diff comments:
In `@frontends/aiq_api/src/aiq_api/jobs/submit.py`:
- Around line 268-273: The logger.warning call in the exception handler for
cleanup_error is logging the raw exception, which can contain sensitive
information such as database connection details or driver messages. Instead of
logging cleanup_error directly, extract only the exception type (such as
type(cleanup_error).__name__) or a sanitized error code and log that instead.
This ensures that sensitive details are not leaked through logs while still
providing enough information to identify that a rollback failure occurred.
🪄 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: 3e4d385e-2099-4194-b95b-75d7b8358682
📒 Files selected for processing (10)
deploy/compose/init-db.sqldocs/source/integration/rest-api.mdfrontends/aiq_api/src/aiq_api/jobs/access.pyfrontends/aiq_api/src/aiq_api/jobs/submit.pyfrontends/aiq_api/tests/test_job_access.pyfrontends/aiq_api/tests/test_submit_collision.pyskills/aiq-research/SKILL.mdskills/aiq-research/scripts/aiq.pysrc/aiq_agent/agents/chat_researcher/register.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Run Harbor skill eval
🧰 Additional context used
📓 Path-based instructions (13)
**
⚙️ 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:
deploy/compose/init-db.sqlskills/aiq-research/scripts/aiq.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.pyfrontends/aiq_api/tests/test_submit_collision.pyfrontends/aiq_api/src/aiq_api/jobs/submit.pydocs/source/integration/rest-api.mdfrontends/aiq_api/tests/test_job_access.pyfrontends/aiq_api/src/aiq_api/jobs/access.pyskills/aiq-research/SKILL.mdsrc/aiq_agent/agents/chat_researcher/register.py
{deploy/**,configs/**}
⚙️ CodeRabbit configuration file
{deploy/**,configs/**}: Review deployment and config changes for secret separation, safe defaults, local-vs-production behavior, Helm and
Docker portability, and documentation parity. Flag committed credentials, environment-specific NVIDIA internals in
public defaults, and changes that make examples diverge from CI-tested paths.
Files:
deploy/compose/init-db.sql
**/*.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:
skills/aiq-research/scripts/aiq.pytests/aiq_agent/agents/chat_researcher/test_register_helpers.pyfrontends/aiq_api/tests/test_submit_collision.pyfrontends/aiq_api/src/aiq_api/jobs/submit.pyfrontends/aiq_api/tests/test_job_access.pyfrontends/aiq_api/src/aiq_api/jobs/access.pysrc/aiq_agent/agents/chat_researcher/register.py
skills/aiq-research/**/*.py
📄 CodeRabbit inference engine (skills/aiq-research/SKILL.md)
skills/aiq-research/**/*.py: Python version requirement: Python 3.11+ available aspython3for running AI-Q research scripts
Use the AIQ helper script atscripts/aiq.pyfor all AI-Q research queries instead of direct API calls
Always runhealthcheck before sending research requests to verify backend availability
UseAIQ_CONVERSATION_IDenvironment variable as a stable identifier to pin one conversation for the entire sequence of original research and follow-up queries
Poll asynchronous deep research jobs usingresearch_pollwhen AI-Q returns ajob_idinstead of inline results
Stop on failed jobs and show returned errors without automatic retry
Use non-blocking or background execution when polling asynchronous jobs, and request explicit user approval if escalated permissions are needed
Do not truncate citations or source URLs from returned reports in any output or transformation
Report backend failures and HTTP 500 errors instead of fabricating research answers when the backend is unavailable or lacks async agents
Files:
skills/aiq-research/scripts/aiq.py
skills/aiq-research/**/*.{py,sh}
📄 CodeRabbit inference engine (skills/aiq-research/SKILL.md)
skills/aiq-research/**/*.{py,sh}: Do not include credentials, cookies, bearer tokens, or secret values in query text sent to AI-Q backend
Preserve and present all citations and source URLs from returned AI-Q reports intact
Generate a unique conversation ID usingpython3 -c "import uuid;print(uuid.uuid4())"and export it asAIQ_CONVERSATION_IDbefore running research and follow-up queries
State the exact AI-Q backend URL before sending user queries and confirm non-local URLs are trusted
Export a stable conversation ID once and reuse it for the original research and every follow-up without passing a report ID
Treat returned reports as potentially sensitive if the backend uses private data sources
Files:
skills/aiq-research/scripts/aiq.py
skills/aiq-research/**
⚙️ CodeRabbit configuration file
skills/aiq-research/**: ---
name: aiq-research
description: |
Use when asked to run deep research or AI-Q research through a reachable NVIDIA AI-Q Blueprint backend.
license: Apache-2.0
permissions:
env:
- AIQ_SERVER_URL
network:
- http://localhost:8000
compatibility: |
Designed for Claude Code, OpenCode, Codex, and Agent Skills-compatible tools. Requires Python 3.11+ and network
access to a running local AI-Q Blueprint server athttp://localhost:8000by default. Non-local backends must be
explicitly trusted by the user and granted by the host tool outside this public skill.
metadata:
version: "2.1.0"
author: "NVIDIA AI-Q Blueprint Team aiq-blueprint@nvidia.com"
github-url: "https://github.com/NVIDIA-AI-Blueprints/aiq"
tags:
- nvidia
- aiq
- blueprint
- deep-research
- research-agents
- agent-skills
languages:
- python
- bash
domain: "research-agents"
allowed-tools: Read BashAIQ Research Skill
Purpose
Use this skill to call a locally running NVIDIA AI-Q Blueprint server through the helper script at
scripts/aiq.py.Use this skill for research-shaped requests, including:
- "deep research on ..."
- "AIQ research ..."
- "research ..."
- "use AI-Q to answer ..."
- "ask AI-Q about ..."
Do not use this skill for install, deploy, start, stop, UI, CLI, Docker, Helm, or troubleshooting requests. Those
belong toaiq-deploy.Prerequisites
Users need:
- Python 3.11+ available as
python3.- A reachable local or self-hosted AI-Q Blueprint backend.
AIQ_SERVER_URLset when the backend is not running athttp://localhost:8000; non-local values must be trusted by
the user before any query is sent.- A backend configured with authentication disabled for this public helper, or a separate authenticated AI-Q skill for
authenticated environments.- Network access from the local machine to the AI-Q backend URL.
- Credentials configured in the backend environment, not in this skill. Thi...
Files:
skills/aiq-research/scripts/aiq.pyskills/aiq-research/SKILL.md
{skills/**,.agents/skills/**,.claude/skills/**,.github/skill-eval/**}
⚙️ CodeRabbit configuration file
{skills/**,.agents/skills/**,.claude/skills/**,.github/skill-eval/**}: Review Agent Skill and skill-eval changes for valid skill metadata, deterministic eval specs, safe handling of
credentials, and clear generated-output boundaries. Do not flag SKILL.md files for missing SPDX headers when the
entrypoint intentionally starts with YAML frontmatter.
Files:
skills/aiq-research/scripts/aiq.pyskills/aiq-research/SKILL.md
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/agents/chat_researcher/test_register_helpers.pyfrontends/aiq_api/tests/test_submit_collision.pyfrontends/aiq_api/tests/test_job_access.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/jobs/submit.pyfrontends/aiq_api/src/aiq_api/jobs/access.py
docs/source/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update the docs under docs/source/ when behavior, configuration, or workflows change
Files:
docs/source/integration/rest-api.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.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/chat_researcher/register.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/register.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA-AI-Blueprints/aiq
Timestamp: 2026-06-24T00:04:10.415Z
Learning: Do not use this skill for install, deploy, start, stop, UI, CLI, Docker, or Helm requests; use `aiq-deploy` skill instead
📚 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_submit_collision.pyfrontends/aiq_api/tests/test_job_access.py
🪛 LanguageTool
skills/aiq-research/SKILL.md
[style] ~162-~162: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...r a report is presented, the user often wants to ask about it, revise it, or go deeper. ...
(REP_WANT_TO_VB)
🪛 SQLFluff (4.2.2)
deploy/compose/init-db.sql
[error] 68-68: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
[error] 70-70: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
🔇 Additional comments (9)
docs/source/integration/rest-api.md (1)
149-163: LGTM!src/aiq_agent/agents/chat_researcher/register.py (1)
108-139: LGTM!Also applies to: 582-594, 607-607
tests/aiq_agent/agents/chat_researcher/test_register_helpers.py (1)
486-538: LGTM!deploy/compose/init-db.sql (1)
55-70: LGTM!frontends/aiq_api/src/aiq_api/jobs/access.py (1)
21-21: LGTM!Also applies to: 33-44, 81-87, 241-267, 268-308
frontends/aiq_api/tests/test_job_access.py (1)
51-88: LGTM!Also applies to: 103-172, 199-207, 255-271
frontends/aiq_api/src/aiq_api/jobs/submit.py (2)
79-86: LGTM!Also applies to: 144-201, 313-320
298-306: 🗄️ Data Integrity & IntegrationNarrow the IntegrityError catch to verify it's a job-id collision constraint.
The current catch assumes every
IntegrityErrorfromjob_store.submit_job()is a duplicatejob_id, but this is not verified against NAT's implementation. If other database constraints exist on thejob_infotable (or related tables), skipping rollback would incorrectly leave an orphaned row.Inspect the exception to confirm the constraint name before raising
JobIdConflictError:except IntegrityError as e: # Verify this is a job_id collision, not another constraint violation. # NAT's _create_job inserts job_info first, so collision fails before # any state of OURS is created. The colliding job belongs to someone else — # we must NOT roll back, which unconditionally deletes that job's info/events/access rows. if e.orig and "job_id" in str(e.orig).lower(): logger.info("Rejected colliding job_id %s on async submit", resolved_job_id) raise JobIdConflictError(f"Job already exists: {resolved_job_id}") from e # Not a collision; fall through to general exception handler for rollback raiseAlternatively, document that NAT constrains
job_info.job_idas the only unique constraint on that table before submission completes.frontends/aiq_api/tests/test_submit_collision.py (1)
122-144: LGTM!
|
Both report ask and report edit inline the full parent report into an LLM prompt. Since we don't have token budget or truncation, I'm worried that large completed reports can exceed the model context window or crowd out the user’s actual question/edit instruction. Not sure what the right thing to do here is. For first pass, we could probably add a size/token guard here? But wondering if it would be possible to use a simple agent with filesystem tools rather than individual LLM calls? |
@cdgamarose-nv Good callout — I dug into this and it breaks down by path:
On the actual risk: both run on large context llm (gpt-oss-120b, large context window), and if a report ever did exceed the window, report_ask_node and report_edit_node each wrap the call in try/except and degrade to a "please try again" message — no crash, no data loss. So today it's a graceful tail-case, not a correctness bug. On the filesystem-tools idea — that's the right long-term shape, and the deep_researcher already does exactly that (deepagents read_file/grep over the report seeded into its virtual FS). The subtlety: a lazy read_file only defers loading — once it returns, the whole report is in context. What actually bounds the window is selective reads (grep/offset) or truncation. So the proper version for ask is a small read/grep agent that pulls only the relevant sections — a real rearchitecture, and the synchronous CLI path has no shared FS so it'd need a temp-file shim. My plan is to defer the hard guard for this PR (tail risk + graceful failure + large-context model) and track the read/grep-agent version as a follow-up |
e51d225 to
99a9c0e
Compare
Resolve the 13 actionable CodeRabbit comments on PR NVIDIA-AI-Blueprints#271: - routes/jobs.py: reject whitespace-only `input` on report-edit and submit requests (422) via a field validator; map post-auth RuntimeError to 503 (availability) instead of 403, without echoing internal detail. - runner.py: build job output so the canonical `report` can never be overwritten by a stray `report` key in output_metadata. - report_context.py: tolerate a transient event-log fetch failure when the report is already available (fall back to report-only sources). - register.py: bound the report-ask LLM call with a timeout so a stalled provider degrades gracefully; make parent-report context enrichment optional for delta research (don't abort the deep job if context lookup fails). - intent_classifier.py: normalize string booleans for use_parent_report_context ("false" must not be truthy). - utils.py: preserve an explicit empty data_sources list ([] = no data-source tools) instead of falling through to other sources. - agent.py: log error type, not raw exception text, in report follow-up paths. - report_rewriter/agent.py: dispatch the final report to all registered callbacks (remove early break). - ci/markdown-link-check-config.json: narrow the FastAPI/Dask ignores to the exact referenced URLs instead of whole domains. - docs: document the report-edit 500 case. - tests: assert authorization on the GET report route; add coverage for blank input (422), string-boolean normalization, and empty data_sources preservation. Backend suites (aiq_api + aiq_agent) and UI specs pass; ruff and tsc clean. Verified live: blank input -> 422, report ask answers, job_id collision -> 409. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Ajay Thorve <athorve@nvidia.com>
70d66f1 to
c82d9db
Compare
e505160 to
fa84388
Compare
Resolve the 13 actionable CodeRabbit comments on PR NVIDIA-AI-Blueprints#271: - routes/jobs.py: reject whitespace-only `input` on report-edit and submit requests (422) via a field validator; map post-auth RuntimeError to 503 (availability) instead of 403, without echoing internal detail. - runner.py: build job output so the canonical `report` can never be overwritten by a stray `report` key in output_metadata. - report_context.py: tolerate a transient event-log fetch failure when the report is already available (fall back to report-only sources). - register.py: bound the report-ask LLM call with a timeout so a stalled provider degrades gracefully; make parent-report context enrichment optional for delta research (don't abort the deep job if context lookup fails). - intent_classifier.py: normalize string booleans for use_parent_report_context ("false" must not be truthy). - utils.py: preserve an explicit empty data_sources list ([] = no data-source tools) instead of falling through to other sources. - agent.py: log error type, not raw exception text, in report follow-up paths. - report_rewriter/agent.py: dispatch the final report to all registered callbacks (remove early break). - ci/markdown-link-check-config.json: narrow the FastAPI/Dask ignores to the exact referenced URLs instead of whole domains. - docs: document the report-edit 500 case. - tests: assert authorization on the GET report route; add coverage for blank input (422), string-boolean normalization, and empty data_sources preservation. Backend suites (aiq_api + aiq_agent) and UI specs pass; ruff and tsc clean. Verified live: blank input -> 422, report ask answers, job_id collision -> 409. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Ajay Thorve <athorve@nvidia.com>
fa84388 to
523602d
Compare
|
/nvskills-ci |
|
on do you need to run through the citations verification/sanitization again? |
cdgamarose-nv
left a comment
cdgamarose-nv
left a comment
There was a problem hiding this comment.
Left some cosmetic comments. Everything worked functionally for me! Good to go in.
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
…iple YAML files Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
4e709d6 to
15bef02
Compare
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Resolve the 13 actionable CodeRabbit comments on PR NVIDIA-AI-Blueprints#271: - routes/jobs.py: reject whitespace-only `input` on report-edit and submit requests (422) via a field validator; map post-auth RuntimeError to 503 (availability) instead of 403, without echoing internal detail. - runner.py: build job output so the canonical `report` can never be overwritten by a stray `report` key in output_metadata. - report_context.py: tolerate a transient event-log fetch failure when the report is already available (fall back to report-only sources). - register.py: bound the report-ask LLM call with a timeout so a stalled provider degrades gracefully; make parent-report context enrichment optional for delta research (don't abort the deep job if context lookup fails). - intent_classifier.py: normalize string booleans for use_parent_report_context ("false" must not be truthy). - utils.py: preserve an explicit empty data_sources list ([] = no data-source tools) instead of falling through to other sources. - agent.py: log error type, not raw exception text, in report follow-up paths. - report_rewriter/agent.py: dispatch the final report to all registered callbacks (remove early break). - ci/markdown-link-check-config.json: narrow the FastAPI/Dask ignores to the exact referenced URLs instead of whole domains. - docs: document the report-edit 500 case. - tests: assert authorization on the GET report route; add coverage for blank input (422), string-boolean normalization, and empty data_sources preservation. Backend suites (aiq_api + aiq_agent) and UI specs pass; ruff and tsc clean. Verified live: blank input -> 422, report ask answers, job_id collision -> 409. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Resolve the 13 actionable CodeRabbit comments on PR NVIDIA-AI-Blueprints#271: - routes/jobs.py: reject whitespace-only `input` on report-edit and submit requests (422) via a field validator; map post-auth RuntimeError to 503 (availability) instead of 403, without echoing internal detail. - runner.py: build job output so the canonical `report` can never be overwritten by a stray `report` key in output_metadata. - report_context.py: tolerate a transient event-log fetch failure when the report is already available (fall back to report-only sources). - register.py: bound the report-ask LLM call with a timeout so a stalled provider degrades gracefully; make parent-report context enrichment optional for delta research (don't abort the deep job if context lookup fails). - intent_classifier.py: normalize string booleans for use_parent_report_context ("false" must not be truthy). - utils.py: preserve an explicit empty data_sources list ([] = no data-source tools) instead of falling through to other sources. - agent.py: log error type, not raw exception text, in report follow-up paths. - report_rewriter/agent.py: dispatch the final report to all registered callbacks (remove early break). - ci/markdown-link-check-config.json: narrow the FastAPI/Dask ignores to the exact referenced URLs instead of whole domains. - docs: document the report-edit 500 case. - tests: assert authorization on the GET report route; add coverage for blank input (422), string-boolean normalization, and empty data_sources preservation. Backend suites (aiq_api + aiq_agent) and UI specs pass; ruff and tsc clean. Verified live: blank input -> 422, report ask answers, job_id collision -> 409. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Ajay Thorve <athorve@nvidia.com>
3 participants
Overview
Adds report-aware follow-up to the deep-research blueprint. After a report job completes, a user can ask about it, make cosmetic edits, or start delta research that reuses the prior report — without forcing every follow-up through a dedicated "report mode." A chat router picks a semantic route from the message plus an optional
active_report_job_id; the active report is treated as context, never as a command.Modes
report_rewriterasync child job for mechanical/aesthetic edits that do not need new evidence. The parent report stays immutable.Intent routing examples
what are the risks in this report?→report_askmake this shorter→report_cosmetic_editformat the key takeaways as bullets→report_cosmetic_editrewrite this report from a player-performance POV→report_delta_researchredo this with newer evidence on 2026 logistics→report_delta_researchwrite a separate report on player performance trends across 2014, 2018, and 2022→standalone_researchKey pieces
report_rewriter— registered withpublic=False(hidden fromGET /agents; direct/submitof internal-only agents is rejected, and the gate is also enforced at the submission boundary)./shared/*files into child runs.POST /v1/jobs/async/job/{job_id}/report/edit(per-job ownership auth);GET .../reportextended withparent_job_id/interaction_action/result_kind.POST /chat/ WebSocket) acceptsactive_report_job_id; the UI forwards the latest completed report job id and renders the report-edit child job through the existing report-streaming path./shared/*parent-context files so delta research can read/shared/original_report.mdand/shared/source_summary.mdreliably.Validation
All commands run from the repo root unless noted.
Lint / format / unit tests (backend)
UI
End-to-end (manual) — report follow-up needs async jobs, so bring up the full stack (PostgreSQL job store + embedded Dask scheduler/worker + web), e.g. via
deploy/composeorscripts/start_server_in_debug_mode.shwithdeploy/.env, then exercise the surfaces below.What to test
GET /v1/jobs/async/agentsdoes not listreport_rewriter;POST /v1/jobs/async/submitwithagent_type=report_rewriterreturns400 Agent type is internal-only: report_rewriter.POST /v1/jobs/async/job/{id}/report/editon a completed report → areport_rewriterchild job;GET .../reporton the child returns a revised report plusparent_job_id,interaction_action="edit",result_kind="report". The parent report is unchanged.active_report_job_id— verify each semantic route:What are the top three takeaways from this report?;Where does the report say the evidence is weak or incomplete?Make this report shorter while preserving the sources.;Format the key takeaways as bullets.;Remove the one-table comparison section and keep the rest unchanged.Rewrite this report from a player-performance POV.;Redo this report with newer evidence on 2026 logistics and host-city operations.;Add a section on fan travel emissions for 2026.Write a separate report on player performance trends across the 2014, 2018, and 2022 World Cups.;Research the economics of Olympic host cities since 2000./shared/original_report.mdand/shared/source_summary.mdsuccessfully, not see/shared/shared/, and not fail withstring indices must be integersor'str' object has no attribute 'get'from filesystem tools.REQUIRE_AUTH=false; underREQUIRE_AUTH=true, a non-owner is rejected with404before any report content is read.job_idreturns409and does not delete the existing job; whitespace-onlyinputreturns422; report ask/edit degrade to a chat message (not an opaque workflow error) if context resolution fails.Evidence — backend suites (
frontends/aiq_api/tests+tests/aiq_agent/...) and UI specs pass;ruffandtscclean. Verified live against a Postgres + Dask stack withdeploy/.env: internal agent hidden +/submitrejected (400); HTTP and chat report-edit produce a revised child report with correct lineage; chat report-ask answers from the report only;job_idcollision → 409 with the victim preserved; blank input → 422; ownership-mismatch → 404 underREQUIRE_AUTH=true; report-delta routing kicks off deep research with seeded parent context.git commit -sor an equivalent sign-off.Where should reviewers start?
Read in this order — security-sensitive paths first:
frontends/aiq_api/src/aiq_api/jobs/report_context.py— durable report/source reconstruction andresolve_authorized_report_context(), which authorizes the caller before any read and seeds/shared/*for child runs. The core security boundary.frontends/aiq_api/src/aiq_api/routes/jobs.py— thereport/editendpoint, thepublicagent filter on/agents+/submit, the newJobReportResponsefields, and the request validators (blank-input → 422,job_idcollision → 409).frontends/aiq_api/src/aiq_api/jobs/submit.py—submit_agent_job()ownership recording, theJobIdConflictError/InternalAgentErrorgates, and the rollback that only deletes state this submission created.src/aiq_agent/agents/chat_researcher/nodes/intent_classifier.py+agent.py+register.py— the semantic route classifier (report_ask/report_cosmetic_edit/report_delta_research/standalone_research), the bounded tool-free report-ask path, and how each entry point resolves the principal (same contract as the HTTP routes).src/aiq_agent/agents/deep_researcher/deepagents_runtime.py— route-aware/shared/*file seeding for delta research.src/aiq_agent/agents/report_rewriter/— the single new internal agent (a bounded, tool-less single-LLM rewrite).frontends/ui/src/features/chat/hooks/use-websocket-chat.ts+adapters/api/websocket-client.ts— forwardsactive_report_job_idand routes the report-edit child job through the existing report-streaming path.Tests mirror these:
test_report_context.py,test_report_edit.py,test_submit_collision.py,test_submit_internal_agent.py,test_job_access.py,test_agent_registry_visibility.py,test_intent_classifier.py,test_deepagents_runtime.py(backend), anduse-websocket-chat.spec.ts(UI).Related Issues
develop).Summary by CodeRabbit
Release Notes
New Features
Improvements