fix(config): enable Modal artifact capture - #372
Conversation
Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (3)
🧰 Additional context used📓 Path-based instructions (1){.github/**,ci/**,.pre-commit-config.yaml,pyproject.toml,uv.lock,mcp/pyproject.toml,mcp/uv.lock}⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (1)
WalkthroughThe Modal deep research sandbox now enables bounded artifact capture with an extension allowlist. Runtime validation covers the shipped profile, the skills-sandbox documentation explains job-scoped storage and provider options, and Markdown link checking ignores the Exa homepage URL. ChangesDurable artifact capture
Markdown link checking
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
tanleach
left a comment
There was a problem hiding this comment.
LGTM once pipeline passes
Signed-off-by: Kyle Zheng <kyzheng@nvidia.com>
|
/merge |
1208b72
into
NVIDIA-AI-Blueprints:release/2.2
#### Overview Deep-research currently accepts any non-empty `/shared/output.md` as the final report, including stale planner/orchestrator content from the same state. This prerequisite establishes a small writer-owned commit boundary before the broader convergence/artifact work in #373. - Create one run-local `FinalReportCommitTracker` per `DeepResearcherAgent.run()`. - Deny non-writer `write_file`/`edit_file` mutations of `/shared/output.md` and its route-local `/output.md` state alias with `final_report_writer_only`. - Upsert writer `write_file` calls through the existing backend, refresh the exact UTF-8 SHA-256 after successful edits, and sanitize backend failures as `writer_output_commit_failed`. - Require a successful writer mutation, non-empty current content, and an exact digest match before writer completion or final extraction. - Preserve the existing one-turn correction, then fail closed with `writer_output_not_committed`. - Remove production inline-orchestrator report salvage. - Document that the proof is run-local and byte-exact; it is not persistent across restart and does not claim cross-provider filesystem atomicity. This changes no API fields, configuration schema, database schema, deployment behavior, or sandbox-provider implementation. After this prerequisite merges, #373 can rebase onto `release/2.2` and use its validated ResearchNotes fallback when no committed writer report exists. #### DCO sign-off for the squash commit Signed-off-by: Kyle Zheng <kyzheng@nvidia.com> #### Validation - [x] `uv run ruff check .` — passed - [x] `uv run ruff format --check .` — passed (372 files) - [x] Focused deep-research/job-runner regressions — 265 passed - [x] `uv run pytest` — 1,836 passed, 13 skipped - [x] `pre-commit run --all-files` — all 13 hooks passed, including secrets, lockfiles, links, and notebook output - [x] `cd docs && make html` — passed - [x] Live Modal smoke — `deep_researcher` reached success with a non-empty 12,351-byte report - [x] Live OpenShell 0.0.80 local-demo smoke — strict readiness passed; job reached success with a non-empty 7,360-byte report; attestation and cleanup events were recorded; no owned sandbox remained - [x] I added or updated tests for behavior changes. - [x] I updated documentation for contributor-facing architecture behavior. - [x] I confirmed this PR does not include secrets, credentials, local configs, generated policy files, or internal-only data. - [x] I certify this contribution under the Developer Certificate of Origin (DCO) and signed the commit with `git commit -s`. - [x] I replaced the DCO sign-off placeholder with my GitHub commit identity and kept the required angle brackets. #### Where should reviewers start? 1. `FinalReportCommitTracker`, `FinalReportOwnershipGuardMiddleware`, and `FinalReportCommitMiddleware` in `custom_middleware.py`. 2. Per-run tracker construction/extraction in `agent.py` and role-specific wiring in `factory.py`. 3. The routed `StateBackend`/`CompositeBackend` stale-planner overwrite regression in `test_custom_middleware.py`. #### Related Issues - Prerequisite for #373. - Independent of #372; no artifact-capture configuration is included here. ## Summary by CodeRabbit - **Bug Fixes** - Final research reports are now accepted only after the writer commits `/shared/output.md` (or the supported backend alias) during the current run. - Commit verification is digest-based, rejecting stale/modified/missing/whitespace-only output and detecting any post-write tampering. - Only the designated writer can create/update the final report artifact; completion is blocked until the verified commit occurs. - Removed inline fallback/salvage behavior—if the commit proof isn’t present, runs fail with `writer_output_not_committed`. - **Documentation** - Updated deep-research architecture docs to describe the stricter writer/output commit contract, ownership guardrails, and failure-closed handoffs. - **Tests** - Expanded coverage for digest-verified commit tracking, writer-only ownership enforcement, and updated failure reason/validation scenarios. Authors: - Kyle Zheng (https://github.com/KyleZheng1284) - Ajay Thorve (https://github.com/AjayThorve) Approvers: - Ajay Thorve (https://github.com/AjayThorve) URL: #375
Overview
Enable bounded artifact capture in the shipped Modal-backed domain-routing-and-skills profile so async jobs can persist supported CSV and chart outputs. Add a Pydantic-backed reference-profile regression test and document the required async job database plus production S3-compatible artifact storage settings.
This PR changes configuration, documentation, and profile validation only. It makes no agent, API, or sandbox-provider behavior changes.
DCO sign-off for the squash commit
Signed-off-by: Kyle Zheng kyzheng@nvidia.com
Validation
.venv/bin/pytest -q tests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.py— 38 passed.venv/bin/ruff check tests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.py.venv/bin/ruff format --check tests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.py.venv/bin/nat validate --config_file configs/config_domain_routing_and_skills.yml.venv/bin/sphinx-build -W --keep-going -b html docs/source /tmp/aiq-pr1-docsgit commit -sor an equivalent sign-off.Where should reviewers start?
Start with
configs/config_domain_routing_and_skills.yml, then the profile assertion intests/aiq_agent/agents/deep_researcher/test_deepagents_runtime.pyand the storage requirements indocs/source/examples/skills-sandbox/index.md.Related Issues
Summary by CodeRabbit
https://exa.ai/URLs.