fix(aiq-research): handle interrupted jobs and JSON escalation contract - #360
Conversation
The aiq-research helper had two contract mismatches with the 26.07 backend.
- Terminal states: `interrupted` was absent from `_DONE_JOB_STATES` and
`_FAILED_JOB_STATES`, so a cancelled job kept polling every 15s until the
3600s timeout. Add `interrupted` to both sets so polling stops immediately
and the job is reported as failed rather than completed.
- Escalation detection: `_command_chat` recognized only the legacy
`Job ID: <uuid>` text, so the backend's JSON
`{"type":"job_escalation","kind":"deep_research","job_id":"..."}` response
was printed raw and never surfaced as `deep_research_running`. Add
`_detect_deep_research_escalation` (matching the escalation object both as
the top-level /chat result and as JSON embedded in message content, with a
UUID-validated job_id) and try it before the legacy regex, which is
retained for backward compatibility.
Add tests/scripts/test_aiq_research_helper.py covering interrupted terminal
handling, the JSON escalation response, the retained legacy format, and the
malformed / non-escalation / unsupported-kind / invalid-job_id guards.
Signed-off-by: Tanner Leach <tleach@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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (3)**/*.py📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
{skills/**,.agents/skills/**,.claude/skills/**,.github/skill-eval/**}⚙️ CodeRabbit configuration file
Files:
**/*test*.py📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
🪛 ast-grep (0.44.1)skills/aiq-research/scripts/aiq.py[info] 505-505: use jsonify instead of json.dumps for JSON output (use-jsonify) [info] 507-507: use jsonify instead of json.dumps for JSON output (use-jsonify) 🔇 Additional comments (2)
WalkthroughThe AIQ research CLI now treats interrupted jobs as terminal failures and detects validated deep-research escalation payloads from structured or embedded chat responses, while preserving validated legacy job-ID parsing and raw-response fallback behavior. ChangesAIQ research CLI behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant _command_chat
participant chat_request
participant _detect_deep_research_escalation
participant stdout
_command_chat->>chat_request: submit chat request
chat_request-->>_command_chat: return chat result
_command_chat->>_detect_deep_research_escalation: inspect result and embedded content
_detect_deep_research_escalation-->>_command_chat: return validated job_id or None
_command_chat->>stdout: print deep_research_running status or raw result
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 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 `@skills/aiq-research/scripts/aiq.py`:
- Around line 499-501: Validate the legacy job ID captured by _CHAT_JOB_ID_RE
through _validate_job_id before reporting deep-research status; if validation
fails, fall through to the raw result output. Add a malformed 36-character
legacy-match test in tests/scripts/test_aiq_research_helper.py covering this
fallback and asserting raw output instead of deep_research_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: 195e8813-b1cf-47b0-b20b-aea61415e8ea
📒 Files selected for processing (2)
skills/aiq-research/scripts/aiq.pytests/scripts/test_aiq_research_helper.py
📜 Review details
🧰 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:
skills/aiq-research/scripts/aiq.pytests/scripts/test_aiq_research_helper.py
{skills/**,.agents/skills/**,.claude/skills/**,.github/skill-eval/**}
⚙️ CodeRabbit configuration file
{skills/**,.agents/skills/**,.claude/skills/**,.github/skill-eval/**}: Review Agent Skill and skill-eval changes for valid skill metadata, deterministic eval specs, safe handling of
credentials, and clear generated-output boundaries. Do not flag SKILL.md files for missing SPDX headers when the
entrypoint intentionally starts with YAML frontmatter.
Files:
skills/aiq-research/scripts/aiq.py
**/*test*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run pytest for all behavior changes in Python code
Files:
tests/scripts/test_aiq_research_helper.py
🪛 ast-grep (0.44.1)
skills/aiq-research/scripts/aiq.py
[warning] 87-87: Regex pattern passed to re is built from a non-literal (variable, call, concatenation, or f-string) value. If that value is attacker-controlled it can introduce a malicious pattern with catastrophic backtracking (ReDoS). Use a hardcoded literal pattern, or validate/escape untrusted input with re.escape() and bound the regex complexity before compiling.
Context: re.compile(rf"Job ID:\s*([0-9a-f-]{{{JOB_ID_HEX_DASH_LENGTH}}})", re.IGNORECASE)
Note: [CWE-1333] Inefficient Regular Expression Complexity.
(redos-non-literal-regex-python)
[info] 496-496: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"status": _STATUS_DEEP_RESEARCH_RUNNING, "job_id": job_id})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 500-500: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"status": _STATUS_DEEP_RESEARCH_RUNNING, "job_id": match.group(CAPTURE_GROUP_JOB_ID)})
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
tests/scripts/test_aiq_research_helper.py
[info] 94-94: use jsonify instead of json.dumps for JSON output
Context: json.dumps(payload)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🔇 Additional comments (4)
skills/aiq-research/scripts/aiq.py (2)
84-93: LGTM!
457-488: LGTM!tests/scripts/test_aiq_research_helper.py (2)
1-43: LGTM!Also applies to: 48-82, 88-106
123-153: LGTM!
The legacy `Job ID: <uuid>` fallback in `_command_chat` reported any 36-character `[0-9a-f-]` capture as an active deep-research job, so a malformed match could produce a false `deep_research_running`. Route the captured id through `_validate_job_id` and fall through to raw result output when it is not a valid UUID, matching the JSON escalation path. Add a malformed 36-character legacy-match test asserting raw output rather than `deep_research_running`. Signed-off-by: Tanner Leach <tleach@nvidia.com>
|
/merge |
b90d8a1
into
NVIDIA-AI-Blueprints:release/2.2
2 participants
Overview
The
aiq-researchskill helper (skills/aiq-research/scripts/aiq.py) had two contract mismatches with the 26.07 backend. Both were verified to still reproduce atrelease/2.2HEAD, and the fix was checked against the backend source that defines each contract.1.
interruptedwas not a terminal job state._DONE_JOB_STATESand_FAILED_JOB_STATESomittedinterrupted, but the backend sets a cancelled job to that state (JobStatus.INTERRUPTED = "interrupted"innatasync_jobs/job_store.py; the cancel route infrontends/aiq_api/.../routes/jobs.pyreturns{"status": "interrupted"}). Because the poll loop only exits on a member of_DONE_JOB_STATES, a cancelled job kept polling every 15s until the 3600s timeout. Addinginterruptedto both sets makes polling stop immediately and report the job as failed rather than completed successfully.2. Deep-research escalation was detected only in the legacy text format.
_command_chatmatched onlyJob ID: <uuid>, so the backend's structured escalation payload was printed raw and never surfaced asdeep_research_running, leaving the documentedSKILL.mdStep 2/3 flow stuck. The backend emits the escalation as a compact JSON object placed in the assistant message content (_job_escalation_messageinsrc/aiq_agent/agents/chat_researcher/agent.py):{"type": "job_escalation", "kind": "deep_research", "job_id": "<uuid>"}A new
_detect_deep_research_escalationhelper recognizes this object both as the top-level/chatresult and as JSON embedded in the message content, validates thejob_idas a UUID, and emits the documented{"status": "deep_research_running", "job_id": "<uuid>"}. It is tried before the legacyJob ID:regex, which is retained for backward compatibility. Non-deep_researchkinds (e.g.report_edit), malformed JSON, non-escalation JSON, and missing/invalid job IDs are deliberately ignored so they cannot produce a falsedeep_research_running.No
SKILL.mdprose change is required: the emitted shape is unchanged, so the existing Steps 2–4 flow now advances as documented.DCO sign-off for the squash commit
Signed-off-by: Tanner Leach tleach@nvidia.com
Validation
Run from the repo root:
Contract verification against
release/2.2source (no live 26.07 backend was required, and the fix was not validated against assumptions alone):JobStatus.INTERRUPTED = "interrupted"(natasync_jobs/job_store.py); cancel route returns{"status": "interrupted"}(frontends/aiq_api/src/aiq_api/routes/jobs.py)._job_escalation_message(kind, job_id)returns{"type":"job_escalation","kind":..., "job_id":...}and is delivered asAIMessage(content=...)(src/aiq_agent/agents/chat_researcher/agent.py), matching the "escalation embedded in message content" path the helper handles.New test matrix (
tests/scripts/test_aiq_research_helper.py, loaded viaimportlibto match the standalone-script tests already intests/scripts/):interruptedmembership in the terminal/failed state sets;poll_until_completereturns on the firstinterruptedstatus (one status call, zero sleeps);research_pollexits failure oninterruptedwithout another poll cycle and without fetching a report;JSON escalation detected as top-level result and as embedded content;
_command_chatemitsdeep_research_running;legacy
Job ID: <uuid>still detected;guards: malformed JSON, non-escalation JSON, unsupported
kind, missing/invalid/non-stringjob_id, and shallow-answer fall-through.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 -sor an equivalent sign-off.I replaced the DCO sign-off placeholder with my GitHub commit identity and kept the required angle brackets around the email address.
Where should reviewers start?
skills/aiq-research/scripts/aiq.py: the state-set change (_DONE_JOB_STATES/_FAILED_JOB_STATES) and the new_escalation_job_id/_detect_deep_research_escalationhelpers plus the reworked_command_chatordering (JSON escalation first, legacy regex retained).tests/scripts/test_aiq_research_helper.py: the guard cases confirm no falsedeep_research_runningon malformed / non-escalation / unsupported-kind / invalid-job_idpayloads.Related Issues
Summary by CodeRabbit
Bug Fixes
deep_research_runningstatus.Tests