feat: persist and surface sandbox artifacts#314
Conversation
Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
|
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:
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 (5)
📜 Recent review details⏰ Context from checks skipped due to timeout. (2)
🧰 Additional context used📓 Path-based instructions (8)**/*.py📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
src/aiq_agent/**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
**⚙️ CodeRabbit configuration file
Files:
src/aiq_agent/agents/**/*⚙️ CodeRabbit configuration file
Files:
frontends/ui/**/*.{js,ts,jsx,tsx,vue}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
frontends/ui/**/*.{ts,tsx,jsx,js}📄 CodeRabbit inference engine (AGENTS.md)
Files:
frontends/ui/**/*⚙️ CodeRabbit configuration file
Files:
**/*test*.py📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
🔇 Additional comments (8)
WalkthroughAdds checkpoint and terminal artifact harvesting for sandboxed deep-research runs, changes the artifact SSE payload to ChangesSandbox Artifact Harvesting and Teardown
Durable File Artifact Metadata in UI
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
AjayThorve
left a comment
There was a problem hiding this comment.
Two lifecycle/replay issues to address before this lands.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 6
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/runner.py (1)
937-945: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winInconsistent secret-safe logging vs. the adjacent new code.
The new
finalize_artifactserror handler above (lines 927-936) intentionally logs only the exception type to avoid leaking sensitive content. Theclose()/terminate()error handler right below it still logs withexc_info=True, which includes the full exception message/traceback — if the sandbox provider's cleanup error contains a credential or internal hostname, it would be logged in full. As per coding guidelines, "Never print or log secret values, including in tool output or error messages." Consider aligning this handler with the type-only logging pattern used just above it.🔒️ Proposed fix
try: teardown() - except Exception: # noqa: BLE001 - cleanup must never raise on the terminal path - logger.warning("Sandbox cleanup failed for job %s", job_id, exc_info=True) + except Exception as exc: # noqa: BLE001 - cleanup must never raise on the terminal path + logger.warning( + "Sandbox cleanup failed for job %s exception=%s", job_id, exc.__class__.__name__ + )🤖 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/runner.py` around lines 937 - 945, The sandbox cleanup handler in the runner’s teardown path logs too much detail compared with the adjacent finalize_artifacts error handling. Update the exception handling around sandbox_runtime.terminate/close so it follows the same secret-safe pattern as finalize_artifacts, logging only the exception type or a sanitized summary instead of exc_info=True. Keep the warning context with job_id, but avoid emitting the full traceback or exception message from the teardown branch.Source: Coding guidelines
♻️ Duplicate comments (1)
frontends/aiq_api/src/aiq_api/jobs/runner.py (1)
799-829: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftUnresolved: full sandbox teardown (incl.
close()) still gates the terminal status write.This mirrors a prior review comment:
_teardown_sandboxbundles artifact harvest andclose()/terminate(), and the full call is still awaited (viaasyncio.to_thread) beforeSUCCESSis written. The comment at line 801-802 explains why harvesting must precedeSUCCESS, but doesn't address thatclose()has no explicit timeout — if the underlying SDK session-close call hangs, this job stays un-terminalized (client-visibleRUNNING, SSE never completes), even thoughasyncio.to_threadkeeps other jobs on the worker unaffected.Consider splitting
_teardown_sandboxsofinalize_artifacts(+ flush) runs before the terminal status write, whileclose()/terminate()— or a bounded/timeout-guarded version of it — runs after (e.g., infinally, which already calls it idempotently).🤖 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/runner.py` around lines 799 - 829, The terminal status write is still blocked by the full sandbox teardown because runner logic awaits _teardown_sandbox before update_job_output, and that helper includes close()/terminate() with no timeout. Split _teardown_sandbox in aiq_api.jobs.runner so artifact finalization and event_store.flush happen before JobStatus.SUCCESS is persisted, then move close()/terminate() to a separate bounded or timeout-protected step (or rely on the existing finally path) so a hung SDK shutdown cannot leave the job stuck RUNNING.
🤖 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/ui/src/adapters/api/deep-research-client.spec.ts`:
- Around line 57-108: Add a test in deep-research-client.spec.ts to cover the
fallback branch in createDeepResearchClient’s artifact.update handling when
content_url or url is present without artifact_id. Verify that the emitted
onFileUpdate payload still maps the URL correctly and preserves the other file
metadata, so the behavior in the deep-research-client.ts artifact mapping logic
is covered.
In `@frontends/ui/src/adapters/api/deep-research-client.ts`:
- Line 15: The adapter deep-research-client currently imports
artifactContentPath from a renderer-specific component path, which creates an
invalid dependency direction. Move artifactContentPath into a neutral shared
utility location such as shared/utils or a common API helper, then update
deep-research-client and MarkdownRenderer to import it from that shared symbol
instead of the component folder.
- Around line 515-528: The local artifact wrapper type in deep-research-client
is re-declaring a narrower shape and then falling back to Record<string,
unknown> for the file case, which duplicates the existing ArtifactUpdateEvent
contract. Update artifactWrapper to reuse or extend the exported artifact data
shape used by ArtifactUpdateEvent.data.data so the added metadata fields
(artifact_id, file_path, kind, mime_type, size_bytes, sha256, title, caption,
inline) are preserved with type safety, and remove the manual unknown
casting/typeof guards in the file handling path.
- Around line 548-549: The fallback in the deep research artifact parsing can
collapse multiple pathless artifacts into the same filename key, causing silent
overwrites. Update the filename derivation in the artifact mapping logic around
the filePath/fileName assignment to use a unique fallback such as artifactId or
another generated identifier when file_path, path, and url are all missing, so
downstream consumers that key by filename do not merge unrelated artifacts.
In `@src/aiq_agent/agents/deep_researcher/sandbox/README.md`:
- Around line 156-164: The README text has an incomplete sentence in the
cancellation behavior description. Update the sentence in the deep_researcher
sandbox README section so it has a full verb phrase after “Cancellation attempts
that scan only when…”, keeping the same meaning by clearly stating that scans
happen only when the provider operation lease is immediately available and that
busy sandboxes are terminated immediately.
In `@tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py`:
- Around line 184-193: Add a test in test_artifacts.py for the
checkpoint-to-final-harvest sequence on the same manifest: after calling
harvest_after_execute() on the manager created by _make_manager, call
final_harvest() and assert that previously checkpointed artifacts are not
re-emitted or re-counted. Use the existing test helpers and symbols
(harvest_after_execute, final_harvest, _make_manager, SqlArtifactStore) to
verify that the manifest-backed file is captured only once and that
final_harvest still picks up only the uncaptured stray file discovered by the
directory scan.
---
Outside diff comments:
In `@frontends/aiq_api/src/aiq_api/jobs/runner.py`:
- Around line 937-945: The sandbox cleanup handler in the runner’s teardown path
logs too much detail compared with the adjacent finalize_artifacts error
handling. Update the exception handling around sandbox_runtime.terminate/close
so it follows the same secret-safe pattern as finalize_artifacts, logging only
the exception type or a sanitized summary instead of exc_info=True. Keep the
warning context with job_id, but avoid emitting the full traceback or exception
message from the teardown branch.
---
Duplicate comments:
In `@frontends/aiq_api/src/aiq_api/jobs/runner.py`:
- Around line 799-829: The terminal status write is still blocked by the full
sandbox teardown because runner logic awaits _teardown_sandbox before
update_job_output, and that helper includes close()/terminate() with no timeout.
Split _teardown_sandbox in aiq_api.jobs.runner so artifact finalization and
event_store.flush happen before JobStatus.SUCCESS is persisted, then move
close()/terminate() to a separate bounded or timeout-protected step (or rely on
the existing finally path) so a hung SDK shutdown cannot leave the job stuck
RUNNING.
🪄 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: 330ede88-45d4-4291-83cc-668c00e1ed57
📒 Files selected for processing (26)
docs/source/architecture/agents/sandbox.mdfrontends/aiq_api/README.mdfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/adapters/api/deep-research-client.tsfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/features/chat/hooks/use-deep-research.spec.tsfrontends/ui/src/features/chat/hooks/use-deep-research.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/chat/types.tsfrontends/ui/src/features/layout/components/FileCard.spec.tsxfrontends/ui/src/features/layout/components/FileCard.tsxsrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.pysrc/aiq_agent/agents/deep_researcher/factory.pysrc/aiq_agent/agents/deep_researcher/sandbox/README.mdsrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pytests/aiq_agent/agents/deep_researcher/test_custom_middleware.pytests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.pytests/aiq_agent/agents/deep_researcher/test_factory.pytests/aiq_agent/jobs/test_runner.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (11)
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/features/layout/components/FileCard.spec.tsxfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/chat/types.tsfrontends/ui/src/features/chat/hooks/use-deep-research.tsfrontends/ui/src/features/layout/components/FileCard.tsxfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/features/chat/hooks/use-deep-research.spec.tsfrontends/ui/src/adapters/api/deep-research-client.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/features/layout/components/FileCard.spec.tsxfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/chat/types.tsfrontends/ui/src/features/chat/hooks/use-deep-research.tsfrontends/ui/src/features/layout/components/FileCard.tsxfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/features/chat/hooks/use-deep-research.spec.tsfrontends/ui/src/adapters/api/deep-research-client.ts
**
⚙️ 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:
frontends/ui/src/features/layout/components/FileCard.spec.tsxsrc/aiq_agent/agents/deep_researcher/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/agent.pyfrontends/aiq_api/README.mdtests/aiq_agent/jobs/test_runner.pyfrontends/ui/src/adapters/api/index.tssrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pyfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/features/chat/store.tsdocs/source/architecture/agents/sandbox.mdfrontends/ui/src/features/chat/types.tssrc/aiq_agent/agents/deep_researcher/sandbox/README.mdfrontends/ui/src/features/chat/hooks/use-deep-research.tssrc/aiq_agent/agents/deep_researcher/deepagents_runtime.pytests/aiq_agent/agents/deep_researcher/test_custom_middleware.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pyfrontends/ui/src/features/layout/components/FileCard.tsxfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/features/chat/hooks/use-deep-research.spec.tssrc/aiq_agent/agents/deep_researcher/factory.pytests/aiq_agent/agents/deep_researcher/test_factory.pytests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.pyfrontends/ui/src/adapters/api/deep-research-client.tsfrontends/aiq_api/src/aiq_api/jobs/runner.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py
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/features/layout/components/FileCard.spec.tsxfrontends/ui/src/adapters/api/index.tsfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/features/chat/store.tsfrontends/ui/src/features/chat/types.tsfrontends/ui/src/features/chat/hooks/use-deep-research.tsfrontends/ui/src/features/layout/components/FileCard.tsxfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/features/chat/hooks/use-deep-research.spec.tsfrontends/ui/src/adapters/api/deep-research-client.ts
**/*.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/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/agent.pytests/aiq_agent/jobs/test_runner.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.pytests/aiq_agent/agents/deep_researcher/test_custom_middleware.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/factory.pytests/aiq_agent/agents/deep_researcher/test_factory.pytests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pytests/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/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/deepagents_runtime.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/factory.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/custom_middleware.pysrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pysrc/aiq_agent/agents/deep_researcher/sandbox/base.pysrc/aiq_agent/agents/deep_researcher/sandbox/README.mdsrc/aiq_agent/agents/deep_researcher/deepagents_runtime.pysrc/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.pysrc/aiq_agent/agents/deep_researcher/factory.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/aiq_agent/jobs/test_runner.pytests/aiq_agent/agents/deep_researcher/test_custom_middleware.pytests/aiq_agent/agents/deep_researcher/test_factory.pytests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py
docs/source/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update the docs under docs/source/ when behavior, configuration, or workflows change
Files:
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/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/jobs/runner.py
🔇 Additional comments (28)
frontends/ui/src/adapters/api/deep-research-client.ts (2)
153-167: LGTM!Also applies to: 238-238
555-561: 🔒 Security & PrivacyKeep the legacy file-event fallback. Generated artifacts already include
artifact_idand same-origincontent_url; theraw.content_url/raw.urlbranch preserves older text-file events and should stay for compatibility.> Likely an incorrect or invalid review comment.frontends/ui/src/adapters/api/index.ts (1)
125-125: LGTM!frontends/ui/src/features/chat/hooks/use-load-job-data.ts (1)
31-31: LGTM! Buffering and store-commit changes correctly consume the newFileArtifactUpdateshape, keyed by filename with last-write-wins semantics matching the store's merge behavior.Also applies to: 375-375, 431-435, 577-579
frontends/ui/src/features/chat/hooks/use-deep-research.ts (1)
216-216: 🎯 Functional CorrectnessBuffered replay still drops content on duplicate filename — matches prior feedback, not fully fixed.
buf.files.set(file.filename, file)(Line 491) fully replaces the previous buffered entry.store.ts'saddDeepResearchFilemerge fix only covers the live path (Line 493); the buffer path used during reconnect/replay still loses a priorcontentvalue when a later durable-artifact-only event (nocontent) arrives for the same filename.♻️ Suggested fix to merge buffered entries
onFileUpdate: (file) => { - if (buf.active) { buf.files.set(file.filename, file); return } + if (buf.active) { + const existing = buf.files.get(file.filename) + buf.files.set(file.filename, existing ? { ...existing, ...file } : file) + return + }Also applies to: 491-491
frontends/ui/src/features/chat/hooks/use-deep-research.spec.ts (1)
940-940: LGTM!Also applies to: 953-953, 963-963, 973-973
frontends/ui/src/features/chat/store.ts (1)
2911-2922: LGTM! Correct merge-on-update semantics for the live path.frontends/ui/src/features/chat/types.ts (1)
358-378: LGTM!frontends/ui/src/features/layout/components/FileCard.tsx (1)
22-40: LGTM! Gating logic and metadata rendering are consistent with the new durable artifact contract.Also applies to: 71-76, 99-99, 119-125, 143-168
frontends/ui/src/features/layout/components/FileCard.spec.tsx (1)
65-88: LGTM!frontends/aiq_api/src/aiq_api/jobs/runner.py (3)
842-845: 🩺 Stability & AvailabilitySame unresolved teardown-ordering issue as the success path (see comment above).
Cancellation also awaits full
_teardown_sandbox(harvest +terminate()) before the terminal event/status update.
862-867: 🩺 Stability & AvailabilitySame unresolved teardown-ordering issue as the success path (see comment above).
The failure path also awaits full
_teardown_sandboxbefore the terminal event/FAILUREstatus update.
854-861: LGTM!Also applies to: 894-895, 918-936
src/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.py (1)
86-113: LGTM!src/aiq_agent/agents/deep_researcher/factory.py (1)
265-291: 🎯 Functional CorrectnessConfirm orchestrator intentionally excluded from artifact checkpointing.
artifact_manageris threaded into researcher/planner/writer stacks but not intobuild_orchestrator_middleware. If the orchestrator itself ever calls the sandboxexecutetool directly (vs. purely delegating to subagents), those calls won't get incremental checkpoints — only the terminalfinal_harvestinagent.pywould capture them. Please confirm this is intentional given the orchestrator's tool set.src/aiq_agent/agents/deep_researcher/agent.py (1)
138-143: LGTM!tests/aiq_agent/agents/deep_researcher/test_custom_middleware.py (1)
630-698: LGTM!tests/aiq_agent/agents/deep_researcher/test_factory.py (1)
190-211: LGTM!tests/aiq_agent/jobs/test_runner.py (2)
913-943: LGTM!
2481-2492: LGTM!src/aiq_agent/agents/deep_researcher/custom_middleware.py (1)
509-528: 🗄️ Data Integrity & IntegrationNo concurrency bug here
ArtifactManager._harvest()already serializesharvest_after_execute()andfinal_harvest()withself._lock.> Likely an incorrect or invalid review comment.tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py (1)
51-63: LGTM!Also applies to: 121-138, 179-193
frontends/aiq_api/README.md (1)
152-152: LGTM!src/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.py (1)
181-204: LGTM!src/aiq_agent/agents/deep_researcher/deepagents_runtime.py (1)
22-22: LGTM!Also applies to: 152-153, 220-221, 235-261
src/aiq_agent/agents/deep_researcher/sandbox/base.py (1)
36-36: LGTM!Also applies to: 144-153
tests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.py (1)
20-21: LGTM!Also applies to: 306-356
docs/source/architecture/agents/sandbox.md (1)
54-56: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/aiq_agent/agents/deep_researcher/agent.py`:
- Around line 184-187: The fallback handling in _extract_final_markdown is
grouped incorrectly, so the new files parameter is ignored whenever result is a
dict and missing a usable "files" entry. Update the files assignment in
_extract_final_markdown so it always applies the full fallback chain across both
dict and object result shapes, preserving state.files before defaulting to an
empty dict. Verify the call site that passes state.files still flows into the
file lookup path and that _salvage_inline_report is only used when no output
files are actually available.
🪄 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: 8188f047-fce4-49f1-92c0-46aedc1ac472
📒 Files selected for processing (2)
frontends/ui/src/features/chat/types.tssrc/aiq_agent/agents/deep_researcher/agent.py
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: AIQ CI / Script Validation: Merge branch 'develop' into codex/artifact-harvest-lifecycle
Conclusion: failure
##[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 / 0_Script Validation.txt: Merge branch 'develop' into codex/artifact-harvest-lifecycle
Conclusion: failure
Current runner version: '2.335.1'
##[group]Runner Image Provisioner
Hosted Compute Agent
Version: 20260624.560
Commit: 925d229a51159bc391ae97e54a2dd1fe20af789d
Build Date:
Worker ID: {52baac19-3151-4570-a8f4-e396491d20f4}
Azure Region: northcentralus
##[endgroup]
##[group]Operating System
Ubuntu
24.04.4
LTS
##[endgroup]
##[group]Runner Image
Image: ubuntu-24.04
Version: 20260628.225.1
Included Software: https://github.com/actions/runner-images/blob/ubuntu24/20260628.225/images/ubuntu/Ubuntu2404-Readme.md
Image Release: https://github.com/actions/runner-images/releases/tag/ubuntu24%2F20260628.225
##[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 versio...
🧰 Additional context used
📓 Path-based instructions (7)
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
src/aiq_agent/agents/deep_researcher/agent.py
src/aiq_agent/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/aiq_agent/**/*.py: Respect authenticated data sources by honoring requires_auth, per-user token pass-through, and backend token validators; apply owner guardrails before loading protected report or artifact context into an agent
Do not weaken or bypass AuthMiddleware, validators, or auth gating without a prior design discussion
Files:
src/aiq_agent/agents/deep_researcher/agent.py
**
⚙️ CodeRabbit configuration file
**:AI-Q Agent Guidance
Repository-global instructions for coding agents and for humans reviewing
agent-authored changes. These rules apply to every task in this repository.
Task-specific runbooks live in.agents/skills/— load the
relevant skill before starting a workflow it covers.Project overview
AI-Q is an NVIDIA AI Blueprint: an enterprise research agent built on the
NeMo Agent Toolkit (NAT). The deployed product is a research blueprint, not
a general skill runtime. New retrieval sources and tools are NAT functions;
agent behavior is driven by workflow YAML, Jinja2 prompts, and a data-source
registry — not by hard-coded logic.Primary boundaries:
- Backend Python package:
src/aiq_agent/.- Data-source and tool packages:
sources/(each is its own package).- Frontends and tooling:
frontends/(web UI infrontends/ui/, eval harnesses
infrontends/benchmarks/).- Configs, deployment, docs:
configs/,deploy/,docs/.Stay inside this repository. If your workspace also contains adjacent repos
(for example a sibling NeMo-Relay checkout), do not edit them as part of an AI-Q
change. Treatsources/*as independent packages: prefer the smallest change
scoped to the package you are touching.Repository structure
Path Purpose src/aiq_agent/Backend agent, FastAPI extensions, auth, observability, knowledge sources/Data-source / tool packages (e.g. tavily_web_search,google_scholar_paper_search)configs/Workflow YAML configs (e.g. config_cli_default.yml)frontends/ui/Next.js / React / TypeScript / Tailwind / KUI web UI frontends/benchmarks/Eval harnesses: freshqa,deepsearch_qa,deepresearch_benchdeploy/Docker Compose and Helm/Kubernetes assets; deploy/.envfor secretsdocs/source/...
Files:
src/aiq_agent/agents/deep_researcher/agent.pyfrontends/ui/src/features/chat/types.ts
src/aiq_agent/agents/**/*
⚙️ CodeRabbit configuration file
src/aiq_agent/agents/**/*: Review agent changes for research workflow correctness, graph state transitions, prompt/tool contracts,
HITL behavior, and failure handling. Flag changes that weaken source attribution, report generation,
async cancellation, checkpointing, or data-source selection without focused tests and docs.
Files:
src/aiq_agent/agents/deep_researcher/agent.py
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/features/chat/types.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/features/chat/types.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/features/chat/types.ts
🔇 Additional comments (4)
src/aiq_agent/agents/deep_researcher/agent.py (2)
276-276: 🎯 Functional Correctness | ⚡ Quick winCall site depends on fixing the fallback bug above.
state.filesis passed here as the intended fallback for_extract_final_markdown, but per the ternary bug at Line 187, this fallback is discarded wheneverresultis a dict. See the fix proposed there.
138-143: LGTM!frontends/ui/src/features/chat/types.ts (2)
363-375: LGTM!
64-64: 🎯 Functional CorrectnessNo remaining
plan_approvalreferences infrontends/ui.
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/aiq_agent/agents/deep_researcher/agent.py`:
- Around line 184-187: The fallback handling in _extract_final_markdown is
grouped incorrectly, so the new files parameter is ignored whenever result is a
dict and missing a usable "files" entry. Update the files assignment in
_extract_final_markdown so it always applies the full fallback chain across both
dict and object result shapes, preserving state.files before defaulting to an
empty dict. Verify the call site that passes state.files still flows into the
file lookup path and that _salvage_inline_report is only used when no output
files are actually available.
🪄 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: 8188f047-fce4-49f1-92c0-46aedc1ac472
📒 Files selected for processing (2)
frontends/ui/src/features/chat/types.tssrc/aiq_agent/agents/deep_researcher/agent.py
📜 Review details
🔇 Additional comments (4)
src/aiq_agent/agents/deep_researcher/agent.py (2)
276-276: 🎯 Functional Correctness | ⚡ Quick winCall site depends on fixing the fallback bug above.
state.filesis passed here as the intended fallback for_extract_final_markdown, but per the ternary bug at Line 187, this fallback is discarded wheneverresultis a dict. See the fix proposed there.
138-143: LGTM!frontends/ui/src/features/chat/types.ts (2)
363-375: LGTM!
64-64: 🎯 Functional CorrectnessNo remaining
plan_approvalreferences infrontends/ui.
🛑 Comments failed to post (1)
src/aiq_agent/agents/deep_researcher/agent.py (1)
184-187: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Ternary precedence drops the new
filesfallback whenresultis a dict.
files = result.get("files", None) if isinstance(result, dict) else getattr(result, "files", None) or files or {}parses as(result.get("files", None)) if isinstance(result, dict) else (getattr(result, "files", None) or files or {}). Theor files or {}fallback chain only applies in theelsebranch. Whenresultis a dict (the common case) and lacks a"files"key or it's falsy,filesbecomesNone, silently discarding the newly-addedfilesparameter (passed asstate.filesat line 276) and skipping straight to_salvage_inline_reporteven though valid output files exist.🐛 Proposed fix
- files = result.get("files", None) if isinstance(result, dict) else getattr(result, "files", None) or files or {} + result_files = result.get("files", None) if isinstance(result, dict) else getattr(result, "files", None) + files = result_files or files or {}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.def _extract_final_markdown(self, result: dict | Any, files: dict[str, Any] | None = None) -> str | None: """Extract final Markdown from output files.""" output_paths = ("/shared/output.md", "/output.md") result_files = result.get("files", None) if isinstance(result, dict) else getattr(result, "files", None) files = result_files or files or {}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/aiq_agent/agents/deep_researcher/agent.py` around lines 184 - 187, The fallback handling in _extract_final_markdown is grouped incorrectly, so the new files parameter is ignored whenever result is a dict and missing a usable "files" entry. Update the files assignment in _extract_final_markdown so it always applies the full fallback chain across both dict and object result shapes, preserving state.files before defaulting to an empty dict. Verify the call site that passes state.files still flows into the file lookup path and that _salvage_inline_report is only used when no output files are actually available.
- runner.py: harvest artifacts before SUCCESS/FAILURE and defer the provider's unbounded close()/terminate() to finally, so a hung SDK cleanup can't strand a completed job in RUNNING; secret-safe teardown logging (exception type only). - agent.py: fix ternary-precedence bug in _extract_final_markdown that dropped the files fallback when result is a dict. - UI replay: merge buffered file updates (use-deep-research, use-load-job-data) so a later metadata-only event keeps earlier content. - deep-research-client.ts: unique filename fallback (artifactId) instead of a shared 'unknown' key. - Move MarkdownRenderer/artifact-url -> shared/utils to fix the adapter->component dependency inversion; update importers. - README grammar; add backend + frontend regression tests. Signed-off-by: Kyle Zheng <kyzheng@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)
frontends/aiq_api/src/aiq_api/jobs/runner.py (1)
844-862: 🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy liftCancellation teardown should not gate job interruption.
frontends/aiq_api/src/aiq_api/jobs/runner.py:844-862runs_teardown_sandbox(..., interrupted=True)before writingJobStatus.INTERRUPTED, and the underlyingterminate()/close()paths have no timeout guard. A hung SDK call can leave the job inRUNNINGand stall SSE shutdown. Thefinallyfallback makes cleanup idempotent, but this branch still pays for a duplicate teardown on cancellation.🤖 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/runner.py` around lines 844 - 862, Cancellation cleanup is blocking job interruption because `_teardown_sandbox(...)` runs before the status update and can hang on SDK terminate/close calls. In `runner.py`’s cancellation path, move the `job_store.update_status(..., JobStatus.INTERRUPTED, ...)` (or an equivalent best-effort status write) ahead of sandbox teardown, and make `_teardown_sandbox` fire after the interruption is recorded so SSE shutdown is not gated by a stuck SDK call. Keep the existing best-effort cleanup behavior in this branch, but avoid letting the teardown step prevent the job from transitioning out of RUNNING.Source: Path instructions
♻️ Duplicate comments (1)
frontends/ui/src/adapters/api/deep-research-client.ts (1)
514-528: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUntyped
Record<string, unknown>cast still duplicates theArtifactUpdateEventcontract.
artifactWrapper.datahere omitsartifact_id,file_path,kind,mime_type,size_bytes,sha256,title,caption,inline— already declared onArtifactUpdateEvent.data(Lines 174-189) — forcing the cast toRecord<string, unknown>and per-fieldtypeofguards. This was flagged previously and remains unresolved.Also applies to: 547-547
🤖 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/ui/src/adapters/api/deep-research-client.ts` around lines 514 - 528, The `artifactWrapper` shape is still duplicating the `ArtifactUpdateEvent` contract and omitting fields already present on `ArtifactUpdateEvent.data`, which is why the code falls back to `Record<string, unknown>` and repeated type guards. Update the parsing in `deep-research-client.ts` to reuse the existing `ArtifactUpdateEvent`/`data` type definition directly for `rawData` and `artifactWrapper.data`, and remove the ad hoc field-by-field casting so the existing schema drives validation and access consistently in the affected artifact-handling block.
🤖 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 `@frontends/aiq_api/src/aiq_api/jobs/runner.py`:
- Around line 844-862: Cancellation cleanup is blocking job interruption because
`_teardown_sandbox(...)` runs before the status update and can hang on SDK
terminate/close calls. In `runner.py`’s cancellation path, move the
`job_store.update_status(..., JobStatus.INTERRUPTED, ...)` (or an equivalent
best-effort status write) ahead of sandbox teardown, and make
`_teardown_sandbox` fire after the interruption is recorded so SSE shutdown is
not gated by a stuck SDK call. Keep the existing best-effort cleanup behavior in
this branch, but avoid letting the teardown step prevent the job from
transitioning out of RUNNING.
---
Duplicate comments:
In `@frontends/ui/src/adapters/api/deep-research-client.ts`:
- Around line 514-528: The `artifactWrapper` shape is still duplicating the
`ArtifactUpdateEvent` contract and omitting fields already present on
`ArtifactUpdateEvent.data`, which is why the code falls back to `Record<string,
unknown>` and repeated type guards. Update the parsing in
`deep-research-client.ts` to reuse the existing `ArtifactUpdateEvent`/`data`
type definition directly for `rawData` and `artifactWrapper.data`, and remove
the ad hoc field-by-field casting so the existing schema drives validation and
access consistently in the affected artifact-handling block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: b3a9f557-2bf4-433e-ac83-8ab0882a7a4e
📒 Files selected for processing (15)
frontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/adapters/api/deep-research-client.tsfrontends/ui/src/features/chat/hooks/use-deep-research.spec.tsfrontends/ui/src/features/chat/hooks/use-deep-research.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/shared/utils/artifact-url.spec.tsfrontends/ui/src/shared/utils/artifact-url.tssrc/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/sandbox/README.mdtests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pytests/aiq_agent/jobs/test_runner.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: UI Unit Tests
⚠️ CI failures not shown inline (2)
GitHub Actions: AIQ CI / 1_Script Validation.txt: fix(artifacts): address review feedback on #314
Conclusion: failure
Current runner version: '2.335.1'
##[group]Runner Image Provisioner
Hosted Compute Agent
Version: 20260624.560
Commit: 925d229a51159bc391ae97e54a2dd1fe20af789d
Build Date:
Worker ID: {e182aeb7-6828-41fd-b5e1-0d31c3268401}
Azure Region: centralus
##[endgroup]
##[group]Operating System
Ubuntu
24.04.4
LTS
##[endgroup]
##[group]Runner Image
Image: ubuntu-24.04
Version: 20260705.232.1
Included Software: https://github.com/actions/runner-images/blob/ubuntu24/20260705.232/images/ubuntu/Ubuntu2404-Readme.md
Image Release: https://github.com/actions/runner-images/releases/tag/ubuntu24%2F20260705.232
##[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...
GitHub Actions: AIQ CI / Script Validation: fix(artifacts): address review feedback on #314
Conclusion: failure
##[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
=====================...
🧰 Additional context used
📓 Path-based instructions (9)
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/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/features/chat/hooks/use-deep-research.tsfrontends/ui/src/features/chat/hooks/use-deep-research.spec.tsfrontends/ui/src/adapters/api/deep-research-client.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/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/features/chat/hooks/use-deep-research.tsfrontends/ui/src/features/chat/hooks/use-deep-research.spec.tsfrontends/ui/src/adapters/api/deep-research-client.ts
**
⚙️ 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:
frontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/MarkdownRenderer/index.tssrc/aiq_agent/agents/deep_researcher/agent.pyfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/features/chat/hooks/use-deep-research.tsfrontends/ui/src/features/chat/hooks/use-deep-research.spec.tssrc/aiq_agent/agents/deep_researcher/sandbox/README.mdfrontends/aiq_api/src/aiq_api/jobs/runner.pyfrontends/ui/src/adapters/api/deep-research-client.tstests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pytests/aiq_agent/jobs/test_runner.py
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/shared/components/MarkdownRenderer/MarkdownRenderer.tsxfrontends/ui/src/shared/components/MarkdownRenderer/index.tsfrontends/ui/src/adapters/api/deep-research-client.spec.tsfrontends/ui/src/pages/api/generate-pdf.tsfrontends/ui/src/features/chat/hooks/use-load-job-data.tsfrontends/ui/src/features/chat/hooks/use-deep-research.tsfrontends/ui/src/features/chat/hooks/use-deep-research.spec.tsfrontends/ui/src/adapters/api/deep-research-client.ts
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run ruff check and ruff format validation for Python code changes
**/*.py: Python code must be linted and formatted with Ruff using line length 120, target Python 3.11, rule sets E,F,W,I,PL,UP, and isort force-single-line configuration
Never commit secrets, tokens, or environment-specific hostnames; use environment variables and SecretStr instead, resolving API keys at runtime
Never print or log secret values, including in tool output or error messages
Missing-secret paths must degrade gracefully (stub/skip), not crash or leak
Do not hand-reformat unrelated code when making changes; match the existing import and formatting style
Files:
src/aiq_agent/agents/deep_researcher/agent.pyfrontends/aiq_api/src/aiq_api/jobs/runner.pytests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.pytests/aiq_agent/jobs/test_runner.py
src/aiq_agent/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/aiq_agent/**/*.py: Respect authenticated data sources by honoring requires_auth, per-user token pass-through, and backend token validators; apply owner guardrails before loading protected report or artifact context into an agent
Do not weaken or bypass AuthMiddleware, validators, or auth gating without a prior design discussion
Files:
src/aiq_agent/agents/deep_researcher/agent.py
src/aiq_agent/agents/**/*
⚙️ CodeRabbit configuration file
src/aiq_agent/agents/**/*: Review agent changes for research workflow correctness, graph state transitions, prompt/tool contracts,
HITL behavior, and failure handling. Flag changes that weaken source attribution, report generation,
async cancellation, checkpointing, or data-source selection without focused tests and docs.
Files:
src/aiq_agent/agents/deep_researcher/agent.pysrc/aiq_agent/agents/deep_researcher/sandbox/README.md
{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}
⚙️ CodeRabbit configuration file
{src/aiq_agent/fastapi_extensions/**,frontends/aiq_api/src/aiq_api/**}: Treat API, auth, and job-runner changes as externally visible contracts. Check authorization boundaries,
request tracing, async job lifecycle, websocket reconnect behavior, error responses, and cross-user data isolation.
Require tests for route behavior, access decisions, and job state transitions when those surfaces change.
Files:
frontends/aiq_api/src/aiq_api/jobs/runner.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.pytests/aiq_agent/jobs/test_runner.py
🔇 Additional comments (16)
frontends/ui/src/shared/components/MarkdownRenderer/MarkdownRenderer.tsx (1)
12-12: LGTM!frontends/ui/src/shared/components/MarkdownRenderer/index.ts (1)
6-15: LGTM!src/aiq_agent/agents/deep_researcher/agent.py (1)
139-143: LGTM!Also applies to: 184-202
frontends/ui/src/adapters/api/deep-research-client.spec.ts (1)
110-134: LGTM! Closes the previously-flagged coverage gap for thecontent_url/url-without-artifact_idfallback branch.frontends/ui/src/adapters/api/deep-research-client.ts (1)
15-15: LGTM! Dependency-direction and filename-collision fixes from prior review are correctly applied here.Also applies to: 546-571
tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py (1)
195-212: LGTM! Addresses the previously-requested checkpoint → final-harvest interplay coverage.tests/aiq_agent/jobs/test_runner.py (1)
913-943: LGTM!Also applies to: 2494-2518
frontends/ui/src/pages/api/generate-pdf.ts (1)
8-8: LGTM!frontends/ui/src/features/chat/hooks/use-load-job-data.ts (1)
31-31: LGTM!Also applies to: 375-375, 431-435, 577-581
frontends/ui/src/features/chat/hooks/use-deep-research.ts (1)
22-22: LGTM!Also applies to: 216-216, 242-242, 490-505
frontends/ui/src/features/chat/hooks/use-deep-research.spec.ts (1)
922-948: LGTM!Also applies to: 967-967, 980-980, 990-990, 1000-1000
src/aiq_agent/agents/deep_researcher/sandbox/README.md (1)
156-164: LGTM!frontends/aiq_api/src/aiq_api/jobs/runner.py (4)
801-812: LGTM!
864-881: LGTM!
883-902: LGTM!
920-963: LGTM!
|
/ok to test 0bf30d1 |
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)
frontends/aiq_api/src/aiq_api/jobs/runner.py (1)
847-850: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMove sandbox teardown out of the cancel branch
frontends/aiq_api/src/aiq_api/jobs/runner.py:847-850This branch calls
_teardown_sandbox(..., interrupted=True)before persistingjob.cancelled/INTERRUPTED. Since that path can block on providerterminate(), a stuck cleanup can strand the job inRUNNINGand prevent the SSE stream from closing. Harvest artifacts here and letfinallyhandle teardown; add a test that cancellation still writesINTERRUPTEDeven if teardown hangs or raises.🤖 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/runner.py` around lines 847 - 850, The cancel handling in runner.py should not perform sandbox teardown before the job state is persisted, since a blocked provider terminate() can leave the job stuck in RUNNING. Move the `_teardown_sandbox(..., interrupted=True)` work out of the cancel branch in `runner.py` and let the existing `finally` cleanup path handle it after the status update to `job.cancelled` / `INTERRUPTED`, while still harvesting any artifacts needed from the cancel path. Add or update a test around the job cancellation flow to verify the status is written and the SSE stream closes even if sandbox teardown hangs or raises.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 `@frontends/aiq_api/src/aiq_api/jobs/runner.py`:
- Around line 847-850: The cancel handling in runner.py should not perform
sandbox teardown before the job state is persisted, since a blocked provider
terminate() can leave the job stuck in RUNNING. Move the `_teardown_sandbox(...,
interrupted=True)` work out of the cancel branch in `runner.py` and let the
existing `finally` cleanup path handle it after the status update to
`job.cancelled` / `INTERRUPTED`, while still harvesting any artifacts needed
from the cancel path. Add or update a test around the job cancellation flow to
verify the status is written and the SSE stream closes even if sandbox teardown
hangs or raises.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 5418c919-47b2-4d1b-8aea-f115f39183ef
📒 Files selected for processing (1)
frontends/aiq_api/src/aiq_api/jobs/runner.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Lint and Hooks
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/src/aiq_api/jobs/runner.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:
frontends/aiq_api/src/aiq_api/jobs/runner.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/runner.py
🔇 Additional comments (1)
frontends/aiq_api/src/aiq_api/jobs/runner.py (1)
634-636: LGTM!Also applies to: 659-659, 716-724, 763-778, 804-813, 859-872, 883-884, 899-900
…etime An LLM can pass an oversized execute `timeout` (milliseconds read as seconds, or a large round number) that exceeds the sandbox provider's hard cap — OpenShell rejects an exec timeout above the gateway's 3600s max — so every `execute` fails and no sandbox code runs, blocking chart/artifact generation. Add ExecuteTimeoutClampMiddleware to bound the `execute` tool-call `timeout` to the configured sandbox lifetime before dispatch; ceiling comes from config via a new DeepAgentsRuntime.execute_timeout_seconds property (no hardcoded value), wired once into the cross-cutting middleware for the researcher, subagents, and orchestrator. Guards a different boundary than SandboxProvider._clamp_timeout (AI-Q's own provider-mediated calls). Adds ExecuteTimeoutClampMiddleware tests. Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
…arser Replace the ad-hoc artifactWrapper type and Record<string, unknown> cast in the artifact.update `file` handler with the exported ArtifactUpdateEvent data contract (extended with the two extra keys this parser reads: path, output_category), and drop the now-redundant per-field typeof guards. Resolves the CodeRabbit comment about duplicating the ArtifactUpdateEvent contract in two places. Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
|
/ok to test 9291911 |
f0e0fbc
into
NVIDIA-AI-Blueprints:develop
#### Overview Extract the provider-neutral artifact lifecycle and UI delivery work from NVIDIA-AI-Blueprints#305 onto current `develop`, so it can land independently of the OpenShell policy-attestation work in NVIDIA-AI-Blueprints#298. This change: - checkpoints artifact manifests after successful sandbox `execute` calls and performs one idempotent terminal harvest before sandbox teardown; - avoids blocking cancellation on an in-flight provider operation while preserving already checkpointed artifacts; - emits canonical `artifact.update` metadata without embedding artifact bytes or credentials; - delivers live and replayed sandbox artifacts to the Files tab, using the authenticated same-origin artifact endpoint for file access; and - preserves the artifact store selected by current configuration, including the existing SQL and S3-compatible implementations on `develop`. The existing NVIDIA-AI-Blueprints#305 branch is unchanged. This PR contains no OpenShell gateway, policy, attestation, Landlock, or cancellation-API changes. #### Validation ```bash PYTHONPATH=src:frontends/aiq_api/src uv run --no-sync pytest \ tests/aiq_agent/agents/deep_researcher/sandbox/test_artifacts.py \ tests/aiq_agent/agents/deep_researcher/test_custom_middleware.py \ tests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.py \ tests/aiq_agent/agents/deep_researcher/test_factory.py \ tests/aiq_agent/jobs/test_runner.py -q # 225 passed uv run --no-sync ruff check . # All checks passed uv run --no-sync ruff format --check . # 307 files already formatted cd frontends/ui npm run type-check # passed npm run lint # passed with 0 errors (existing warnings remain) npm run test:ci # 79 files passed; 1,324 tests passed; 1 skipped cd ../../docs make html # succeeded; 3 existing cross-reference/version-switcher warnings ``` - [x] I ran the relevant local checks or explained why they are not applicable. - [x] I added or updated tests for behavior changes. - [x] I updated documentation for user-facing or contributor-facing changes. - [x] I confirmed this PR does not include secrets, credentials, or internal-only data. - [x] 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? 1. `src/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.py` for the canonical metadata-only SSE contract. 2. `src/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.py`, `custom_middleware.py`, and `frontends/aiq_api/src/aiq_api/jobs/runner.py` for checkpoint and terminal-harvest ownership. 3. `frontends/ui/src/adapters/api/deep-research-client.ts` and `frontends/ui/src/features/layout/components/FileCard.tsx` for live/replayed Files-tab delivery. #### Related Issues - Relates to NVIDIA-AI-Blueprints#305 - Independent of NVIDIA-AI-Blueprints#298 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Deep research file artifacts now stream as structured `artifact.update` events, enabling durable file links with richer metadata (MIME, size, hashes, titles/captions, etc.). * File cards can display MIME/size and show an **Open file** button when a durable content URL is available. * **Bug Fixes** * Improved artifact harvesting across success, failure, and cancellation to better preserve completed outputs. * Updated streaming/replay behavior now merges incremental file metadata updates without overwriting earlier content. * **Documentation** * Refreshed architecture and SSE documentation to reflect the updated `artifact.update` payload shape. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Kyle Zheng <kyzheng@nvidia.com> Co-authored-by: Ajay Thorve <AjayThorve@users.noreply.github.com> Signed-off-by: Tanner Leach <tleach@nvidia.com>
Overview
Extract the provider-neutral artifact lifecycle and UI delivery work from #305 onto current
develop, so it can land independently of the OpenShell policy-attestation work in #298.This change:
executecalls and performs one idempotent terminal harvest before sandbox teardown;artifact.updatemetadata without embedding artifact bytes or credentials;develop.The existing #305 branch is unchanged. This PR contains no OpenShell gateway, policy, attestation, Landlock, or cancellation-API changes.
Validation
git commit -sor an equivalent sign-off.Where should reviewers start?
src/aiq_agent/agents/deep_researcher/sandbox/artifacts/models.pyfor the canonical metadata-only SSE contract.src/aiq_agent/agents/deep_researcher/sandbox/artifacts/manager.py,custom_middleware.py, andfrontends/aiq_api/src/aiq_api/jobs/runner.pyfor checkpoint and terminal-harvest ownership.frontends/ui/src/adapters/api/deep-research-client.tsandfrontends/ui/src/features/layout/components/FileCard.tsxfor live/replayed Files-tab delivery.Related Issues
Summary by CodeRabbit
artifact.updateevents, enabling durable file links with richer metadata (MIME, size, hashes, titles/captions, etc.).artifact.updatepayload shape.