Repository navigation
fix(api): honor async submit tool and agent availability - #385
Conversation
Treat empty tool lists as inherited and limit async agents to those configured in the active workflow. Signed-off-by: Tanner Leach <tleach@nvidia.com>
|
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 (3)**/*.py📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*test*.py📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (1)📚 Learning: 2026-06-14T17:49:00.640ZApplied to files:
🔇 Additional comments (2)
WalkthroughAsync job routes now list only public agents configured in the active workflow and reject unconfigured agents with structured ChangesConfigured Agent Availability
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AsyncJobsAPI
participant WorkflowBuilder
participant JobSubmission
Client->>AsyncJobsAPI: POST /v1/jobs/async/submit
AsyncJobsAPI->>WorkflowBuilder: Resolve agent function config
WorkflowBuilder-->>AsyncJobsAPI: Configured function config
AsyncJobsAPI->>WorkflowBuilder: Resolve configured tools and data sources
WorkflowBuilder-->>AsyncJobsAPI: Available source IDs
AsyncJobsAPI->>JobSubmission: Submit validated job
JobSubmission-->>Client: Job response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/ok to test a07f6dd |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/source/integration/rest-api.md`:
- Around line 108-126: Update the 500 response description in the jobs route’s
OpenAPI responses mapping to include unexpected agent/tool configuration lookup
failures alongside encryption and authorization persistence failures. Update the
matching assertion in test_content_encryption_routes.py to expect the expanded
description, keeping the documented causes consistent across the schema and
test.
🪄 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: e40664b2-6cbe-4e74-b9aa-b591f2c23101
📒 Files selected for processing (7)
docs/source/integration/rest-api.mdfrontends/aiq_api/README.mdfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/tests/test_agent_registry_visibility.pyfrontends/aiq_api/tests/test_content_encryption_routes.pyfrontends/aiq_api/tests/test_job_submit_data_sources.pytests/aiq_agent/fastapi_extensions/test_job_agent_availability.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Pytest and Coverage
- GitHub Check: Script Validation
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
frontends/aiq_api/tests/test_content_encryption_routes.pytests/aiq_agent/fastapi_extensions/test_job_agent_availability.pyfrontends/aiq_api/tests/test_agent_registry_visibility.pyfrontends/aiq_api/src/aiq_api/routes/jobs.pyfrontends/aiq_api/tests/test_job_submit_data_sources.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
frontends/aiq_api/tests/test_content_encryption_routes.pytests/aiq_agent/fastapi_extensions/test_job_agent_availability.pyfrontends/aiq_api/tests/test_agent_registry_visibility.pyfrontends/aiq_api/tests/test_job_submit_data_sources.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/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}
⚙️ CodeRabbit configuration file
{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}: Treat API, auth, and job-runner changes as externally visible contracts. Check authorization boundaries,
request tracing, async job lifecycle, websocket reconnect behavior, error responses, and cross-user data isolation.
Require tests for route behavior, access decisions, and job state transitions when those surfaces change.
Files:
frontends/aiq_api/src/aiq_api/routes/jobs.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_content_encryption_routes.pyfrontends/aiq_api/tests/test_agent_registry_visibility.pyfrontends/aiq_api/tests/test_job_submit_data_sources.py
🔇 Additional comments (15)
frontends/aiq_api/src/aiq_api/routes/jobs.py (7)
625-641: Downstream effect of the helper's fragility, covered at Line 221-228.The lack of per-agent exception isolation here is addressed in the comment on
_get_configured_agent_function_config(Line 221-228); no separate action needed here beyond that fix.
231-274: LGTM!
277-353: LGTM!
466-469: LGTM!
690-737: LGTM! Structuredagent_not_configured400 payload and ordering (public check → configured check → auth) match the test expectations precisely.
757-764: LGTM!
221-228: 🩺 Stability & AvailabilityNo issue here This just centralizes the existing missing-function check; the same
ValueErrortext is already used by the agent-availability path and tests, andget_function_config()is called synchronously here.> Likely an incorrect or invalid review comment.frontends/aiq_api/tests/test_agent_registry_visibility.py (2)
13-23: LGTM!
61-159: LGTM!frontends/aiq_api/tests/test_job_submit_data_sources.py (2)
30-31: LGTM!Also applies to: 39-59, 63-133
158-177: LGTM! Exclusion-list changes and new unconfigured-agent tests correctly track the updatedfn_config-based validation logic.Also applies to: 276-334, 403-620, 747-813
tests/aiq_agent/fastapi_extensions/test_job_agent_availability.py (1)
1-181: LGTM!docs/source/integration/rest-api.md (1)
29-54: LGTM!Also applies to: 92-92, 476-476
frontends/aiq_api/README.md (1)
70-70: LGTM!Also applies to: 81-85, 109-123
frontends/aiq_api/tests/test_content_encryption_routes.py (1)
457-467: LGTM!
Signed-off-by: Tanner Leach <tleach@nvidia.com>
|
/ok to test 13cbd03 |
|
/merge |
380a801
into
NVIDIA-AI-Blueprints:release/2.2
Fix async submit data-source validation for inherited agent tools and reject unconfigured public agents consistently.
Overview
POST /v1/jobs/async/submitrejected valid registered data sources on v2.2.0-rc3 and rc4 when an agent omitted itstoolsfield. Typed researcher configs normalize the omitted field totools=[], which means "inherit all tools from the data-source registry." Submit validation instead treated onlyNoneas inheritance, resolved no tools, and classified every requested source as unavailable.The global agent catalog also advertised agents absent from the active workflow. Selecting such an agent could return an unstructured HTTP 500 or enqueue an unusable job when source validation was skipped.
This change:
GET /v1/jobs/async/agentsto public agents configured in the active workflow.For
config_domain_routing_and_skills.yml,deep_researchernow acceptsweb_search: althoughweb_search_toolis excluded,advanced_web_search_toolsurvives and remains mapped to the source. The globally registered but unconfiguredshallow_researchernow returns:{ "detail": { "code": "agent_not_configured", "message": "Agent 'shallow_researcher' is not configured in the active workflow", "agent_type": "shallow_researcher", "config_name": "shallow_research_agent" } }DCO sign-off for the squash commit
Signed-off-by: Tanner Leach tleach@nvidia.com
Validation
Automated checks:
Live REST validation with a deep-only workflow:
GET /v1/jobs/async/agentsdata_sourcesdata_sources: nulldata_sources: []data_sources: ["web_search"]data_sources: ["does_not_exist"]Live validation with both agents configured confirmed that deep and shallow both accept
data_sources: ["web_search"]. A meaningful shallow query also completed successfully with a cited report.git commit -sor an equivalent sign-off.Where should reviewers start?
Start with:
frontends/aiq_api/src/aiq_api/routes/jobs.py_get_configured_agent_function_config_get_agent_available_source_idslist_agentssubmit_jobtests/aiq_agent/fastapi_extensions/test_job_agent_availability.pyfor the CI-collected contract coverage.frontends/aiq_api/tests/test_job_submit_data_sources.pyfor the complete deep/shallow and source-selection matrix.The key design decision is that empty typed
toolslists preserve the existing runtime inheritance contract; this change does not reinterprettools=[]as "no tools" or introduce a new shared resolver.Summary by CodeRabbit
data_sourcesavailability and added/updated structured error cases (including400 agent_not_configured).400 agent_not_configuredwith a structured payload when the agent isn’t configured.