Skip to content

feat(artifacts): add S3-compatible blob storage#300

Open
soumilinandi wants to merge 7 commits into
NVIDIA-AI-Blueprints:developfrom
soumilinandi:feat/object-storage
Open

feat(artifacts): add S3-compatible blob storage#300
soumilinandi wants to merge 7 commits into
NVIDIA-AI-Blueprints:developfrom
soumilinandi:feat/object-storage

Conversation

@soumilinandi

@soumilinandi soumilinandi commented Jul 2, 2026

Copy link
Copy Markdown

Overview

Adds opt-in S3-compatible storage for sandbox-generated artifact bytes using boto3. Artifact metadata remains in SQL, while bytes can be stored in AWS S3, MinIO, Ceph, R2, or another compatible service. SQL BLOB storage remains the default.

Known considerations:

  • One artifact backend is configured for the deployment.
  • The S3 bucket must already exist.
  • S3 mode does not fall back to SQL if an upload fails.
  • A process crash between the S3 upload and SQL insert may leave an orphaned object. Reconciliation or lifecycle cleanup is deferred to a follow-up.

Validation

uv run pytest \
  tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py \
  tests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.py \
  frontends/aiq_api/tests/test_periodic_cleanup.py

Result: 71 passed.

Manual validation:

  • Verified SQL artifact upload, download, and deletion.
  • Verified MinIO artifact upload, download, and deletion.
  • Verified generated PNG artifacts through the backend and UI report flow.

Artifact generated from miniO server
image

Artifacts generated as blob in default path
image

  • I ran the relevant local checks or explained why they are not applicable.
  • I added or updated tests for behavior changes.
  • I updated documentation for user-facing or contributor-facing changes.
  • I confirmed this PR does not include secrets, credentials, or internal-only data.
  • I certify this contribution under the Developer Certificate of Origin (DCO) and signed my commits with git commit -s or an equivalent sign-off.

Where should reviewers start?

  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/blob_store.py
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.py
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/factory.py
  • tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py

Related Issues

Summary by CodeRabbit

  • New Features
    • Added configurable artifact byte storage: default SQL, or S3-compatible blob storage via AIQ_ARTIFACT_BLOB_PROVIDER=s3 (requires bucket; supports optional endpoint/region/prefix).
    • Artifact metadata remains in the job database, while bytes follow the selected provider.
  • Documentation
    • Expanded “Artifact Storage” docs (architecture + deployment), including Docker Compose/MinIO guidance and full environment variable details.
  • Bug Fixes
    • Job artifact endpoints and retention cleanup now use the shared artifact-store configuration, with improved per-artifact byte deletion/retry behavior for blob storage.
  • Tests
    • Added/extended coverage for SQL and S3-backed stores, factory/provider selection, and cleanup behavior.
  • Chores
    • Updated the deployment image to include S3/object-storage dependencies for the S3 mode.

Signed-off-by: Soumili Nandi <soumilin@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 2, 2026

Copy link
Copy Markdown

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.

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds configurable artifact byte storage with SQL and S3-compatible backends, factory-based construction, runtime wiring, and docs/tests covering the new storage modes.

Changes

Artifact byte storage feature

Layer / File(s) Summary
Blob store interface and implementations
src/aiq_agent/agents/deep_researcher/sandbox/artifacts/blob_store.py, src/aiq_agent/agents/deep_researcher/sandbox/artifacts/__init__.py
Defines the blob-store contract, implements SQL and S3-compatible backends, and exports the new symbols.
Factory and S3 packaging
src/aiq_agent/agents/deep_researcher/sandbox/artifacts/factory.py, pyproject.toml, deploy/Dockerfile
Adds environment-driven store construction, the s3 optional dependency extra, and Docker builder installation for S3 support.
SqlArtifactStore delegation
src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.py
Routes artifact bytes, reads, deletion, cleanup, and validation through the configured blob store.
API and runtime wiring
frontends/aiq_api/src/aiq_api/routes/jobs.py, src/aiq_agent/agents/deep_researcher/deepagents_runtime.py
Switches job routes, health checks, retention cleanup, and artifact-manager setup to the factory-built store.
Documentation and tests
deploy/.env.example, deploy/compose/README.md, docs/source/architecture/agents/sandbox.md, docs/source/deployment/docker-compose.md, docs/source/deployment/production.md, src/aiq_agent/agents/deep_researcher/sandbox/README.md, tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py
Documents the storage configuration and adds coverage for cleanup, S3-backed storage, factory selection, and failure handling.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant JobsRoute as jobs.py
  participant Factory as build_artifact_store
  participant SqlArtifactStore
  participant BlobStore as ArtifactBlobStore

  Client->>JobsRoute: request artifact content or health
  JobsRoute->>Factory: build_artifact_store(db_url)
  Factory->>SqlArtifactStore: construct with resolved blob_store
  JobsRoute->>SqlArtifactStore: get / validate / cleanup_old_artifacts
  SqlArtifactStore->>BlobStore: open_bytes / validate / delete / put
  BlobStore-->>SqlArtifactStore: bytes or status
  SqlArtifactStore-->>JobsRoute: artifact data or health result
Loading

Possibly related PRs

  • NVIDIA-AI-Blueprints/aiq#280: Extends the same deep-research sandbox artifact runtime that this PR refactors into a pluggable blob-store/factory design.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title follows Conventional Commits, stays under 72 characters, and accurately summarizes the S3 blob storage addition.
Description check ✅ Passed The description matches the required template with overview, validation, reviewer guidance, and related issue details filled in.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@soumilinandi soumilinandi marked this pull request as ready for review July 2, 2026 03:46

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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/routes/jobs.py`:
- Around line 821-826: Add route-level tests around the `artifacts` handlers in
`jobs.py` so `build_artifact_store` is exercised through the actual route flow,
not just lower-level mocks. Cover both the `/artifacts` list/download paths and
the artifact cleanup branch by invoking the route handlers with a real store
provider wiring, ensuring a regression in `build_artifact_store` or an
accidental `SqlArtifactStore` mock would fail these tests. Focus on the
`build_artifact_store` call site and the artifact route handlers that use its
returned store.

In `@src/aiq_agent/agents/deep_researcher/sandbox/artifacts/blob_store.py`:
- Around line 95-114: The S3 client setup in the blob store constructor
currently relies on botocore’s default timeouts, which can leave artifact
upload/download calls hanging too long on stalled endpoints. Update the boto3
client creation in the blob store initialization path to pass explicit shorter
connect and read timeout values through the Config used in the client setup,
alongside the existing retry and addressing_style settings. Keep the change
localized to the client creation branch that imports boto3/Config so the
synchronous S3 path remains responsive.
- Around line 134-141: The S3 read in blob_store.py is happening lazily inside
open_bytes, so a missing object can fail only after StreamingResponse has
started. Update open_bytes (and its call sites if needed) so the object lookup
via _client.get_object happens eagerly before returning/constructing the stream,
and surface the missing-object error first; keep the streaming behavior for
Body.iter_chunks and body.close once the object is confirmed to exist.

In `@src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.py`:
- Around line 357-372: The _delete_artifact flow deletes blob bytes first but
leaves the metadata delete/commit unguarded, so a DB failure after a successful
blob delete can leave a dangling artifact row. Add exception handling around the
SQL DELETE and commit in _delete_artifact, matching the defensive pattern
already used for self._blob_store.delete(artifact), and make sure the failure is
logged and the method returns without aborting delete_job/cleanup_old_artifacts.
Also add focused tests/docs for this failure-handling regression under
src/aiq_agent/agents/**/*.
🪄 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: 93c9563f-4ef0-4aba-80d8-70286863364b

📥 Commits

Reviewing files that changed from the base of the PR and between 09da539 and 97bb13c.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (15)
  • deploy/.env.example
  • deploy/Dockerfile
  • deploy/compose/README.md
  • docs/source/architecture/agents/sandbox.md
  • docs/source/deployment/docker-compose.md
  • docs/source/deployment/production.md
  • frontends/aiq_api/src/aiq_api/routes/jobs.py
  • pyproject.toml
  • src/aiq_agent/agents/deep_researcher/deepagents_runtime.py
  • src/aiq_agent/agents/deep_researcher/sandbox/README.md
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/__init__.py
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/blob_store.py
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/factory.py
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.py
  • tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: Run Harbor skill eval
  • GitHub Check: UI Unit Tests
⚠️ CI failures not shown inline (2)

GitHub Actions: AIQ CI / Script Validation: Merge branch 'develop' into feat/object-storage

Conclusion: failure

View job details

##[group]Run . .venv/bin/activate
 �[36;1m. .venv/bin/activate�[0m
 �[36;1mchmod +x ci/scripts/test_scripts.sh�[0m
 �[36;1mci/scripts/test_scripts.sh --skip-setup�[0m
 shell: /usr/bin/bash -e {0}
 env:
   pythonLocation: /opt/hostedtoolcache/Python/3.13.14/x64
   PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.13.14/x64/lib/pkgconfig
   Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.13.14/x64
   Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.13.14/x64
   Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.13.14/x64
   LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.13.14/x64/lib
   UV_CACHE_DIR: /home/runner/work/_temp/setup-uv-cache
 ##[endgroup]
 ================================================
   AI-Q Blueprint - Script Tests
 ================================================
 Repository: /home/runner/work/aiq/aiq
 Scripts:    /home/runner/work/aiq/aiq/scripts
 ============================================
   Testing Bash Syntax
 ============================================
 �[0;32m✅ PASS�[0m: dev.sh - valid bash syntax
 �[0;32m✅ PASS�[0m: setup.sh - valid bash syntax
 �[0;32m✅ PASS�[0m: setup_openshell.sh - valid bash syntax
 �[0;32m✅ PASS�[0m: start_as_skill.sh - valid bash syntax
 �[0;32m✅ PASS�[0m: start_cli.sh - valid bash syntax
 �[0;32m✅ PASS�[0m: start_e2e.sh - valid bash syntax
 �[0;32m✅ PASS�[0m: start_server_in_debug_mode.sh - valid bash syntax
 �[1;33m⏭️  SKIP�[0m: setup.sh - skipped (--skip-setup flag)
 ============================================
   Testing --help Flags
 ============================================
 �[0;32m✅ PASS�[0m: start_cli.sh --help
 �[0;32m✅ PASS�[0m: start_server_in_debug_mode.sh --help
 ============================================
   Testing Virtual Environment Checks
 ============================================
 �[0;32m✅ PASS�[0m: start_cli.sh - venv check works
 �[0;32m✅ PASS�[0m: start_server_in_debug_mode.sh - venv check works
 ============================================
   Testing Pytest Integration
 =====================...

GitHub Actions: AIQ CI / 1_Script Validation.txt: Merge branch 'develop' into feat/object-storage

Conclusion: failure

View job details

Current runner version: '2.335.1'
 ##[group]Runner Image Provisioner
 Hosted Compute Agent
 Version: 20260611.554
 Commit: 5e0782fdc9014723d3be820dd114dd31555c2bd1
 Build Date:
 Worker ID: {3be1a0c8-59cb-45a6-b104-29dd056ebe01}
 Azure Region: centralus
 ##[endgroup]
 ##[group]Operating System
 Ubuntu
 24.04.4
 LTS
 ##[endgroup]
 ##[group]Runner Image
 Image: ubuntu-24.04
 Version: 20260622.220.1
 Included Software: https://github.com/actions/runner-images/blob/ubuntu24/20260622.220/images/ubuntu/Ubuntu2404-Readme.md
 Image Release: https://github.com/actions/runner-images/releases/tag/ubuntu24%2F20260622.220
 ##[endgroup]
 ##[group]GITHUB_TOKEN Permissions
 Contents: read
 Metadata: read
 ##[endgroup]
 Secret source: Actions
 Prepare workflow directory
 Prepare all required actions
 Getting action download info
 Download action repository 'actions/checkout@v4' (SHA:34e114876b0b11c390a56381ad16ebd13914f8d5)
 Download action repository 'actions/setup-python@v5' (SHA:a26af69be951a213d495a4c3e4e4022e16d87065)
 Download action repository 'astral-sh/setup-uv@v4' (SHA:38f3f104447c67c051c4a08e39b64a148898af3a)
 Complete job name: Script Validation
 Node 20 is being deprecated. This workflow is running with Node 24 by default. If you need to temporarily use Node 20, you can set the ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION=true environment variable. For more information see: https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/
 ##[group]Run actions/checkout@v4
 with:
   repository: NVIDIA-AI-Blueprints/aiq
   ***REDACTED***
   ssh-strict: true
   ssh-user: git
   persist-credentials: true
   clean: true
   sparse-checkout-cone-mode: true
   fetch-depth: 1
   fetch-tags: false
   show-progress: true
   lfs: false
   submodules: false
   set-safe-directory: true
 ##[endgroup]
 Syncing repository: NVIDIA-AI-Blueprints/aiq
 ##[group]Getting Git version info
 Working directory is '/home/runner/work/aiq/aiq'
 [command]/usr/bin/git version
 gi...
🧰 Additional context used
📓 Path-based instructions (10)
**/*.py

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Run ruff check and ruff format validation for Python code changes

**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style

Files:

  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/__init__.py
  • src/aiq_agent/agents/deep_researcher/deepagents_runtime.py
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/factory.py
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/blob_store.py
  • frontends/aiq_api/src/aiq_api/routes/jobs.py
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.py
  • tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py
src/aiq_agent/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/aiq_agent/**/*.py: Respect authenticated data sources by honoring requires_auth, per-user token pass-through, and backend token validators; apply owner guardrails before loading protected report or artifact context into an agent
Do not weaken or bypass AuthMiddleware, validators, or auth gating without a prior design discussion

Files:

  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/__init__.py
  • src/aiq_agent/agents/deep_researcher/deepagents_runtime.py
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/factory.py
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/blob_store.py
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.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 in frontends/ui/, eval harnesses
    in frontends/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. Treat sources/* 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_bench
deploy/ Docker Compose and Helm/Kubernetes assets; deploy/.env for secrets
docs/source/ ...

Files:

  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/__init__.py
  • deploy/compose/README.md
  • deploy/Dockerfile
  • src/aiq_agent/agents/deep_researcher/deepagents_runtime.py
  • pyproject.toml
  • docs/source/deployment/docker-compose.md
  • docs/source/deployment/production.md
  • docs/source/architecture/agents/sandbox.md
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/factory.py
  • src/aiq_agent/agents/deep_researcher/sandbox/README.md
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/blob_store.py
  • frontends/aiq_api/src/aiq_api/routes/jobs.py
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.py
  • tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py
src/aiq_agent/agents/**/*

⚙️ CodeRabbit configuration file

src/aiq_agent/agents/**/*: Review agent changes for research workflow correctness, graph state transitions, prompt/tool contracts,
HITL behavior, and failure handling. Flag changes that weaken source attribution, report generation,
async cancellation, checkpointing, or data-source selection without focused tests and docs.

Files:

  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/__init__.py
  • src/aiq_agent/agents/deep_researcher/deepagents_runtime.py
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/factory.py
  • src/aiq_agent/agents/deep_researcher/sandbox/README.md
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/blob_store.py
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.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/README.md
  • deploy/Dockerfile
{.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:

  • pyproject.toml
docs/source/**/*

📄 CodeRabbit inference engine (AGENTS.md)

Update the docs under docs/source/ when behavior, configuration, or workflows change

Files:

  • docs/source/deployment/docker-compose.md
  • docs/source/deployment/production.md
  • docs/source/architecture/agents/sandbox.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/deployment/docker-compose.md
  • docs/source/deployment/production.md
  • docs/source/architecture/agents/sandbox.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
**/*test*.py

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Run pytest for all behavior changes in Python code

Files:

  • tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py
🪛 ast-grep (0.44.0)
src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.py

[warning] 336-339: Do not use text() as it leads to SQL injection
Context: text(
f"SELECT {_META_COLUMNS} FROM artifacts "
"WHERE created_at < NOW() - :seconds * INTERVAL '1 second'"
)
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').

(disable-sqlalchemy-text)


[warning] 348-348: Do not use text() as it leads to SQL injection
Context: text(f"SELECT {_META_COLUMNS} FROM artifacts WHERE created_at < datetime('now', :interval)")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').

(disable-sqlalchemy-text)


[warning] 367-367: Do not use text() as it leads to SQL injection
Context: text("DELETE FROM artifacts WHERE job_id = :job_id AND artifact_id = :artifact_id")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').

(disable-sqlalchemy-text)

tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py

[warning] 327-327: Do not make http calls without encryption
Context: "http://minio:9000"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.

(requests-http)

🪛 LanguageTool
docs/source/deployment/docker-compose.md

[style] ~105-~105: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... metadata always uses the job database. Artifact bytes use SQL BLOB storage by default. ...

(ENGLISH_WORD_REPEAT_BEGINNING_RULE)

🔇 Additional comments (15)
deploy/.env.example (1)

50-64: LGTM!

deploy/compose/README.md (1)

78-88: LGTM!

docs/source/architecture/agents/sandbox.md (1)

37-46: LGTM!

docs/source/deployment/docker-compose.md (1)

103-126: LGTM!

docs/source/deployment/production.md (1)

52-68: LGTM!

src/aiq_agent/agents/deep_researcher/sandbox/README.md (1)

66-68: LGTM!

Also applies to: 159-159, 253-298

tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py (1)

17-43: LGTM!

Also applies to: 66-77, 231-233, 254-267, 270-305, 306-356, 358-378, 380-418

frontends/aiq_api/src/aiq_api/routes/jobs.py (1)

821-826: LGTM!

Also applies to: 845-850, 1191-1195

src/aiq_agent/agents/deep_researcher/deepagents_runtime.py (1)

275-281: LGTM!

pyproject.toml (1)

55-57: LGTM!

deploy/Dockerfile (1)

72-74: 🗄️ Data Integrity & Integration

No action needed for the s3 extra. uv.lock already includes boto3 with extra == 's3', so this uv sync --frozen --no-dev --no-install-workspace --extra s3 path is fine.

			> Likely an incorrect or invalid review comment.
src/aiq_agent/agents/deep_researcher/sandbox/artifacts/blob_store.py (1)

19-93: LGTM!

Also applies to: 142-153

src/aiq_agent/agents/deep_researcher/sandbox/artifacts/__init__.py (1)

20-23: LGTM!

Also applies to: 45-48

src/aiq_agent/agents/deep_researcher/sandbox/artifacts/factory.py (1)

16-49: LGTM!

src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.py (1)

193-268: LGTM!

Comment thread frontends/aiq_api/src/aiq_api/routes/jobs.py
Comment thread src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.py Outdated
region=region,
prefix=prefix,
)
return SqlArtifactStore(db_url, blob_store=blob_store)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we validate the configured bucket as part of startup/readiness? I ran the Compose stack with S3 selected and a reachable MinIO endpoint, but without the configured bucket. /health still returned healthy and the first artifact write failed with NoSuchBucket. We should fail readiness until credentials, endpoint, and bucket access are usable.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented startup validation using head_bucket(). With S3 selected, missing buckets, invalid credentials, or unreachable endpoints now prevent startup. Verified with MinIO: a missing bucket failed startup, while an existing bucket started successfully

Comment thread docs/source/deployment/docker-compose.md Outdated
Signed-off-by: Soumili Nandi <soumilin@nvidia.com>

# Conflicts:
#	deploy/.env.example
#	deploy/Dockerfile
Signed-off-by: Soumili Nandi <soumilin@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@frontends/aiq_api/src/aiq_api/routes/jobs.py`:
- Around line 602-612: The health check in jobs.py still uses the shared default
executor for store.validate, so timed-out S3 probes can leave background work
occupying worker threads. Update the health-check path around
build_artifact_store/store.validate to avoid blocking the loop’s shared
executor—either use a shorter timeout budget on the S3ArtifactBlobStore created
for this probe or run the validation in a dedicated executor for the health
check. Keep the existing try/except and JSONResponse degraded handling in place.
🪄 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: 5d4e1b12-e12d-4503-907c-6537bbfbde66

📥 Commits

Reviewing files that changed from the base of the PR and between 97bb13c and 86f6aa7.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (9)
  • deploy/.env.example
  • deploy/Dockerfile
  • deploy/compose/README.md
  • docs/source/deployment/docker-compose.md
  • frontends/aiq_api/src/aiq_api/routes/jobs.py
  • pyproject.toml
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/blob_store.py
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.py
  • tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: UI Unit Tests
  • GitHub Check: Run Harbor skill eval
  • GitHub Check: Lint and Hooks
🧰 Additional context used
📓 Path-based instructions (10)
**

⚙️ 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 in frontends/ui/, eval harnesses
    in frontends/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. Treat sources/* 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_bench
deploy/ Docker Compose and Helm/Kubernetes assets; deploy/.env for secrets
docs/source/ ...

Files:

  • pyproject.toml
  • deploy/compose/README.md
  • deploy/Dockerfile
  • docs/source/deployment/docker-compose.md
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/blob_store.py
  • frontends/aiq_api/src/aiq_api/routes/jobs.py
  • tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.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:

  • pyproject.toml
{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/README.md
  • deploy/Dockerfile
docs/source/**/*

📄 CodeRabbit inference engine (AGENTS.md)

Update the docs under docs/source/ when behavior, configuration, or workflows change

Files:

  • docs/source/deployment/docker-compose.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/deployment/docker-compose.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:

  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/blob_store.py
  • frontends/aiq_api/src/aiq_api/routes/jobs.py
  • tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.py
src/aiq_agent/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/aiq_agent/**/*.py: Respect authenticated data sources by honoring requires_auth, per-user token pass-through, and backend token validators; apply owner guardrails before loading protected report or artifact context into an agent
Do not weaken or bypass AuthMiddleware, validators, or auth gating without a prior design discussion

Files:

  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/blob_store.py
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.py
src/aiq_agent/agents/**/*

⚙️ CodeRabbit configuration file

src/aiq_agent/agents/**/*: Review agent changes for research workflow correctness, graph state transitions, prompt/tool contracts,
HITL behavior, and failure handling. Flag changes that weaken source attribution, report generation,
async cancellation, checkpointing, or data-source selection without focused tests and docs.

Files:

  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/blob_store.py
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.py
{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}

⚙️ CodeRabbit configuration file

{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}: Treat API, auth, and job-runner changes as externally visible contracts. Check authorization boundaries,
request tracing, async job lifecycle, websocket reconnect behavior, error responses, and cross-user data isolation.
Require tests for route behavior, access decisions, and job state transitions when those surfaces change.

Files:

  • frontends/aiq_api/src/aiq_api/routes/jobs.py
**/*test*.py

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Run pytest for all behavior changes in Python code

Files:

  • tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py
🪛 ast-grep (0.44.0)
src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.py

[warning] 376-376: Do not use text() as it leads to SQL injection
Context: text("DELETE FROM artifacts WHERE job_id = :job_id AND artifact_id = :artifact_id")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').

(disable-sqlalchemy-text)

🔇 Additional comments (15)
src/aiq_agent/agents/deep_researcher/sandbox/artifacts/store.py (3)

374-386: The metadata-delete-after-blob-delete path is now guarded and retains the row for retry on DB failure, matching test_failed_metadata_delete_is_retried. Resolves the prior review concern.

The line 376 text() SQL-injection hint from static analysis is a false positive — the query is fully parameterized with bound :job_id/:artifact_id.


197-230: LGTM!

Also applies to: 267-272, 361-363


94-97: 🩺 Stability & Availability

No action needed

SqlArtifactStore is the only ArtifactStore implementation here, and it already defines validate(). The new abstract method does not break any in-repo concrete subclass.

			> Likely an incorrect or invalid review comment.
deploy/.env.example (1)

50-64: LGTM!

deploy/compose/README.md (1)

78-88: LGTM!

Also applies to: 134-156

docs/source/deployment/docker-compose.md (1)

103-154: LGTM!

tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py (1)

254-267: LGTM!

Also applies to: 284-308, 310-407, 410-446

src/aiq_agent/agents/deep_researcher/sandbox/artifacts/blob_store.py (3)

89-123: 🩺 Stability & Availability

Timeout fix confirmed.

Explicit connect_timeout/read_timeout now set on the S3 client Config, addressing the previously flagged unbounded-hang risk.


143-150: 🩺 Stability & Availability

Lazy get_object inside generator — previously discussed and deferred.

This was already raised (missing S3 object surfaces only after StreamingResponse starts, yielding a truncated 200 instead of 404). Author confirmed this is an accepted edge case given the upload-then-metadata ordering, deferred to a follow-up. No new action here.


19-46: LGTM!

Also applies to: 82-84, 156-158

pyproject.toml (1)

57-59: LGTM!

deploy/Dockerfile (1)

72-79: 🗄️ Data Integrity & Integration

uv.lock already includes boto3 under the s3 extra, so uv sync --frozen --extra s3 is fine.

			> Likely an incorrect or invalid review comment.
frontends/aiq_api/src/aiq_api/routes/jobs.py (3)

912-927: LGTM! Swapping direct SqlArtifactStore(db_url) construction for build_artifact_store(db_url) in list_job_artifacts and get_job_artifact_content is a clean, behavior-preserving change — authorization order, response filtering, and streaming/header logic are all untouched.

Also applies to: 936-972


198-242: LGTM! The per-user MCP auth wiring (source availability, preflight, /v1/data_sources per-user status, submit-time 409 handling) looks internally consistent with the documented preflight/submit contract, and doesn't touch the artifact-store call sites.

Also applies to: 323-340, 439-453, 456-492, 517-539, 615-635, 636-727, 696-703


1524-1552: LGTM! SSE keepalive timers are reset whenever real events are yielded and only fire after SSE_KEEPALIVE_INTERVAL of true silence in both the pub-sub and polling generators — no double-keepalive or busy-loop risk spotted.

Also applies to: 1663-1669, 1724-1753, 1792-1793, 1849-1854

Comment thread frontends/aiq_api/src/aiq_api/routes/jobs.py Outdated
@soumilinandi soumilinandi requested a review from AjayThorve July 2, 2026 20:51
Signed-off-by: Soumili Nandi <soumilin@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py`:
- Around line 350-363: The current test only verifies that S3 health checks use
health_client in S3ArtifactBlobStore.validate(), but it does not assert the new
timeout behavior for bounded health probes. Update the relevant test around
S3ArtifactBlobStore and/or SqlArtifactStore.validate() to confirm the health
client is created with a botocore.config.Config that sets connect_timeout and
read_timeout, or use a fake that would hang without those limits. Keep the
client-routing assertion, but add an explicit check on the configured timeout
bound so the intended startup/health-check safeguard is covered.
🪄 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: cd6c8d1b-6629-4bf6-b90c-1aa5f60a1349

📥 Commits

Reviewing files that changed from the base of the PR and between 86f6aa7 and a3b9fa1.

📒 Files selected for processing (2)
  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/blob_store.py
  • tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: Run Harbor skill eval
  • GitHub Check: Lint and Hooks
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Run ruff check and ruff format validation for Python code changes

**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style

Files:

  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/blob_store.py
  • tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py
src/aiq_agent/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/aiq_agent/**/*.py: Respect authenticated data sources by honoring requires_auth, per-user token pass-through, and backend token validators; apply owner guardrails before loading protected report or artifact context into an agent
Do not weaken or bypass AuthMiddleware, validators, or auth gating without a prior design discussion

Files:

  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/blob_store.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 in frontends/ui/, eval harnesses
    in frontends/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. Treat sources/* 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_bench
deploy/ Docker Compose and Helm/Kubernetes assets; deploy/.env for secrets
docs/source/ ...

Files:

  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/blob_store.py
  • tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py
src/aiq_agent/agents/**/*

⚙️ CodeRabbit configuration file

src/aiq_agent/agents/**/*: Review agent changes for research workflow correctness, graph state transitions, prompt/tool contracts,
HITL behavior, and failure handling. Flag changes that weaken source attribution, report generation,
async cancellation, checkpointing, or data-source selection without focused tests and docs.

Files:

  • src/aiq_agent/agents/deep_researcher/sandbox/artifacts/blob_store.py
**/*test*.py

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Run pytest for all behavior changes in Python code

Files:

  • tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py
🔇 Additional comments (4)
src/aiq_agent/agents/deep_researcher/sandbox/artifacts/blob_store.py (2)

89-126: LGTM!

Also applies to: 137-172


127-136: 🩺 Stability & Availability

connect_timeout=1 is very tight for the S3 health probe. validate() does a single head_bucket with no retries, so cold TLS setup, cross-region access, or VPC latency can still mark a healthy bucket as down. If this path gates readiness or startup, a slightly larger connect budget would reduce false negatives.

tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py (2)

254-267: LGTM!

Also applies to: 270-309, 310-348, 365-396, 416-452


398-413: 🎯 Functional Correctness

Retry path is safe here. _FakeS3Client.delete_object() uses pop(location, None), so a second _delete_artifact() call on the same key is a no-op.

			> Likely an incorrect or invalid review comment.

Comment thread tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py Outdated
This reverts commit a3b9fa1.

Signed-off-by: Soumili Nandi <soumilin@nvidia.com>
Signed-off-by: Soumili Nandi <soumilin@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/routes/jobs.py (1)

908-923: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Consider caching the built artifact store instead of constructing per-request.

build_artifact_store(db_url) is called fresh on every list_job_artifacts and get_job_artifact_content invocation. With the S3 provider now available, this means a new boto3 client (session/credential resolution) is created per request on the list and download hot paths, in addition to whatever SQL-engine construction already happens. Since db_url and the provider env vars are fixed per deployment, building the store once (e.g., during register_job_routes alongside job_store/scheduler_address) and reusing it across requests would avoid this per-call overhead — boto3 clients are safe to share across threads.

♻️ Sketch of caching the store once
     loop = asyncio.get_running_loop()
     await loop.run_in_executor(None, ensure_job_access_table, db_url)
     await loop.run_in_executor(None, _validate_artifact_store, db_url)
+
+    from aiq_agent.agents.deep_researcher.sandbox.artifacts import build_artifact_store
+    artifact_store = build_artifact_store(db_url)

Then reuse artifact_store in list_job_artifacts and get_job_artifact_content instead of re-calling build_artifact_store(db_url).

Also applies to: 932-939

🤖 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/routes/jobs.py` around lines 908 - 923, Cache
the artifact store instead of rebuilding it on every request in the job routes.
`list_job_artifacts` and `get_job_artifact_content` currently call
`build_artifact_store(db_url)` each time, which adds unnecessary S3/boto3 and
SQL-engine setup overhead on hot paths. Build the store once in
`register_job_routes` alongside `job_store` and `scheduler_address`, then pass
or close over that shared `artifact_store` and reuse it in both
`list_job_artifacts` and `get_job_artifact_content`.
🤖 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/routes/jobs.py`:
- Around line 579-581: The startup artifact-store validation in the jobs router
is too broad because `_validate_artifact_store()` runs before all job API routes
are registered, blocking `/health`, submit, state, stream, cancel, artifacts,
and report when storage is down. Move this check out of the shared
initialization in the jobs route setup (near `ensure_job_access_table` /
`_validate_artifact_store`) and limit it to artifact-specific paths or convert
it into a non-fatal startup warning so the rest of the async job API still comes
up.

---

Outside diff comments:
In `@frontends/aiq_api/src/aiq_api/routes/jobs.py`:
- Around line 908-923: Cache the artifact store instead of rebuilding it on
every request in the job routes. `list_job_artifacts` and
`get_job_artifact_content` currently call `build_artifact_store(db_url)` each
time, which adds unnecessary S3/boto3 and SQL-engine setup overhead on hot
paths. Build the store once in `register_job_routes` alongside `job_store` and
`scheduler_address`, then pass or close over that shared `artifact_store` and
reuse it in both `list_job_artifacts` and `get_job_artifact_content`.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 9cfd5721-3c2d-48b3-855c-525ac36a5901

📥 Commits

Reviewing files that changed from the base of the PR and between 5df9526 and c66dbba.

📒 Files selected for processing (2)
  • frontends/aiq_api/src/aiq_api/routes/jobs.py
  • frontends/aiq_api/tests/test_job_submit_data_sources.py
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
  • GitHub Check: Run Harbor skill eval
  • GitHub Check: Lint and Hooks
  • GitHub Check: Pytest and Coverage
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Run ruff check and ruff format validation for Python code changes

**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style

Files:

  • frontends/aiq_api/tests/test_job_submit_data_sources.py
  • frontends/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:

  • frontends/aiq_api/tests/test_job_submit_data_sources.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 in frontends/ui/, eval harnesses
    in frontends/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. Treat sources/* 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_bench
deploy/ Docker Compose and Helm/Kubernetes assets; deploy/.env for secrets
docs/source/ ...

Files:

  • frontends/aiq_api/tests/test_job_submit_data_sources.py
  • frontends/aiq_api/src/aiq_api/routes/jobs.py
{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}

⚙️ CodeRabbit configuration file

{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}: Treat API, auth, and job-runner changes as externally visible contracts. Check authorization boundaries,
request tracing, async job lifecycle, websocket reconnect behavior, error responses, and cross-user data isolation.
Require tests for route behavior, access decisions, and job state transitions when those surfaces change.

Files:

  • frontends/aiq_api/src/aiq_api/routes/jobs.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_job_submit_data_sources.py
🔇 Additional comments (2)
frontends/aiq_api/src/aiq_api/routes/jobs.py (1)

67-73: LGTM!

Also applies to: 583-609, 910-915, 934-939

frontends/aiq_api/tests/test_job_submit_data_sources.py (1)

68-68: LGTM!

Also applies to: 131-151

Comment thread frontends/aiq_api/src/aiq_api/routes/jobs.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants