Add Bounded Research Specialists - #107
Conversation
|
Warning Review limit reached
Next review available in: 40 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis release adds a validated catalog of bounded research specialists. The gateway exposes specialist settings through REST, CLI, terminal, notebook, and Web UI interfaces. OpenHands delegation supports sequential execution, cancellation, resumption, failure projection, persistence, and replay. Tool-enabled analysis implementation remains unavailable. Documentation, smoke tests, screenshots, and package versions target Sequence Diagram(s)sequenceDiagram
participant ParentAgent
participant Gateway
participant Specialist
participant Client
ParentAgent->>Gateway: submit bounded specialist task
Gateway->>Specialist: execute catalog-approved role
Specialist-->>Gateway: return review result
Gateway-->>ParentAgent: project lifecycle and result
Gateway-->>Client: expose settings and status
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 8
🧹 Nitpick comments (8)
packages/gateway/src/heartwood/gateway/_specialists.py (1)
253-266: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConfirm the intended dead combination for project-actions roles.
Lines 255 and 259 combine into an unreachable state: a
project-actionsspecialist that isavailablemust declare tools (line 255) and must not declare tools (line 259). Every tool-using role therefore has to stayunavailable. That matches the PR objective, so no change is required now. Add a short comment that records this constraint, so a later reader does not read line 255 as a supported path.🤖 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 `@packages/gateway/src/heartwood/gateway/_specialists.py` around lines 253 - 266, Add a brief comment near the capability and availability validation in the specialist definition flow, anchored around SpecialistCapability.PROJECT_ACTIONS and SpecialistAvailability.AVAILABLE, documenting that project-actions specialists must remain unavailable because available specialists cannot declare tools. Do not alter the existing validation behavior.packages/gateway/src/heartwood/gateway/_openhands_sdk.py (2)
1556-1562: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider preserving the failure class for specialist failures.
Every failed
TaskObservationnow projects the same sentence. A researcher cannot tell a budget exhaustion from an iteration-limit stop or a model error. The child text stays out of the projection, which is the intended boundary, but a bounded, enumerated reason keeps the parent view actionable.Consider mapping the observation to a small fixed set of reasons, for example "budget exhausted", "iteration limit reached", or "specialist error", and appending that reason to the message. Keep the free-form child text excluded.
🤖 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 `@packages/gateway/src/heartwood/gateway/_openhands_sdk.py` around lines 1556 - 1562, Update the failed TaskObservation branch in the observation result projection to classify failures using a small fixed set of bounded reasons, such as budget exhaustion, iteration-limit reached, or specialist error, and append the selected reason to the parent-facing message. Preserve the existing exclusion of free-form child text and keep result_truncated false for these classified failures.
1453-1459: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify the description check.
The outer truthiness test on
action.descriptionis redundant. The inner check already rejects empty and whitespace-only values.♻️ Proposed simplification
def _tool_summary(event: ActionEvent, *, tool_name: str) -> str: action = event.action - if isinstance(action, TaskAction) and action.description: - description = action.description.strip() - if description: - return description + if isinstance(action, TaskAction): + description = action.description.strip() + if description: + return description return event.summary or f"run {tool_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 `@packages/gateway/src/heartwood/gateway/_openhands_sdk.py` around lines 1453 - 1459, In _tool_summary, remove the redundant outer truthiness check around action.description and let the existing strip-and-nonempty check handle empty or whitespace-only descriptions while preserving the fallback to event.summary or “run {tool_name}”.packages/gateway/tests/test_specialists.py (1)
82-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse one substitution idiom for the boundary-widening cases.
Lines 97-102 assign
sourcetwice. The firstreplacelooks forpermission_mode: inheritin thepermission_modecase, which never matches, and the branch then discards the result. The same file already uses a clearer(old, new, message)parametrization at lines 179-192. Reuse it here.♻️ Proposed refactor
`@pytest.mark.parametrize`( - ("field", "value", "message"), + ("old", "new", "message"), [ - ("model", "openai/unreviewed", "inherit the parent model route"), - ("permission_mode", "never_confirm", "must use always_confirm"), + ("model: inherit", "model: openai/unreviewed", "inherit the parent model route"), + ( + "permission_mode: always_confirm", + "permission_mode: never_confirm", + "must use always_confirm", + ), ], ) def test_catalog_rejects_role_boundary_widening( - field: str, - value: str, + old: str, + new: str, message: str, tmp_path: Path, ) -> None: agents_dir = tmp_path / "agents" agents_dir.mkdir() - source = _valid_definition().replace(f"{field}: inherit", f"{field}: {value}") - if field == "permission_mode": - source = _valid_definition().replace( - "permission_mode: always_confirm", - f"permission_mode: {value}", - ) + source = _valid_definition().replace(old, new) (agents_dir / "bounded-reviewer.md").write_text(source, encoding="utf-8")🤖 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 `@packages/gateway/tests/test_specialists.py` around lines 82 - 106, Refactor test_catalog_rejects_role_boundary_widening to parameterize each case with the original text, replacement text, and expected message, matching the existing pattern used later in the file. Remove the field-based conditional and perform one source.replace operation per case so both model and permission_mode substitutions target the correct baseline values.packages/gateway/tests/test_openhands_sdk.py (2)
3519-3523: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
_specialist_catalog()reloads the catalog from disk on every call. The helper reads and parses every file underagents/verifiedandskills/verifiedeach time. Several tests call it repeatedly, and line 2560 calls it inside a generator expression, so it runs once per evaluated event.
packages/gateway/tests/test_openhands_sdk.py#L3519-L3523: cache the loaded catalog, for example withfunctools.lru_cacheon the helper or a module-level session fixture.packages/gateway/tests/test_openhands_sdk.py#L2560-L2560: hoist the expected label into a local variable before theassert any(...)so the catalog is not reloaded per event.🤖 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 `@packages/gateway/tests/test_openhands_sdk.py` around lines 3519 - 3523, Cache the catalog returned by _specialist_catalog() so its disk loading and parsing occur only once per test process or session; use functools.lru_cache or an equivalent session-scoped fixture. In packages/gateway/tests/test_openhands_sdk.py lines 3519-3523, apply the cache to _specialist_catalog. In packages/gateway/tests/test_openhands_sdk.py line 2560, hoist the expected label into a local variable before the any(...) assertion so the catalog lookup is not repeated for each event.
2828-2834: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid hardcoding the catalog label in the replay assertion.
Line 2831 hardcodes
"Cohort and Feature Reviewer". The other specialist test reads the label from the catalog. If the label inagents/verified/cohort-feature-reviewer.mdchanges, this test fails for a reason unrelated to replay. Read the label from the catalog, as line 2560 does.♻️ Proposed change
+ expected_label = _specialist_catalog().role("cohort-feature-reviewer").label assert any( isinstance(event, BackendSubagentEvent) and event.subagent.agent_name == "cohort-feature-reviewer" - and event.subagent.role_label == "Cohort and Feature Reviewer" + and event.subagent.role_label == expected_label and event.subagent.status == BackendSubagentStatus.COMPLETED for event in replayed )🤖 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 `@packages/gateway/tests/test_openhands_sdk.py` around lines 2828 - 2834, Update the replay assertion for cohort-feature-reviewer to obtain the expected role label from the catalog, matching the existing pattern used by the specialist test near line 2560, instead of hardcoding "Cohort and Feature Reviewer"; keep the remaining BackendSubagentEvent and completion checks unchanged.packages/webui/src/components/UtilitySheet.tsx (1)
344-347: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRender the specialist summary from shared role metadata.
SpecialistSettingsalready carriescapabilityandmodel_route, but this component hard-codesAdvisoryandUses the active model. The browser will display incorrect metadata if the gateway enables a role with different values, and it can diverge from the CLI and notebook projections. Use a gateway-provided summary or a shared formatter.Suggested direction
- Advisory · Uses the active model · Up to {role.max_iterations}{" "} - steps + {specialistPresentationSummary(role)}Based on learnings, keep business rules and labels in the gateway or owning typed package; browser code should only adapt shared projections.
🤖 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 `@packages/webui/src/components/UtilitySheet.tsx` around lines 344 - 347, Update the specialist summary rendering in UtilitySheet to consume the shared gateway role metadata or formatter, using SpecialistSettings.capability and model_route instead of hard-coded “Advisory” and “Uses the active model” labels. Keep the browser component limited to adapting the shared projection while preserving the role.max_iterations step count.Source: Learnings
packages/webui/src/e2e/app.spec.ts (1)
93-99: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winCover the visible-but-unavailable role in the browser contract test.
Lines 93-99 verify only the available
Research Planner.
The mocked response in Lines 1029-1048 omitsAnalysis Implementer, although this release must keep that role visible withavailability: "unavailable".
Add the unavailable role to the mock and assert its label and unavailable reason in the panel.
Otherwise, a regression that removes the unavailable entry or exposes it as available can pass this test.As per coding guidelines, changed web-UI behavior requires tests; the PR objective requires
Analysis Implementerto remain visible but unavailable until restart-safe child approval and cancellation are supported.Also applies to: 1029-1048
🤖 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 `@packages/webui/src/e2e/app.spec.ts` around lines 93 - 99, Update the browser contract test around the Specialists panel and its mocked response to include an Analysis Implementer entry with availability set to unavailable. Assert that its label remains visible and that the panel displays the expected unavailable reason, while preserving the existing Research Planner assertion.Source: Coding guidelines
🤖 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 `@documentation/assets/screenshots/browser-specialists.png.license`:
- Around line 1-3: Update the SPDX attribution in the browser-specialists
license sidecar to include the required Schmiedmayer Lab at Stanford University
attribution while preserving the canonical “2026 Stanford University and the
project authors (see CONTRIBUTORS.md)” copyright text; do not duplicate or alter
that SPDX copyright line.
In `@documentation/reference/glossary.md`:
- Around line 111-113: Update the Research specialist glossary definition to
state that its inherited project, credential, model, network, policy, and
confirmation boundaries may only be inherited or narrowed, never broadened
beyond the parent agent’s boundaries.
In `@documentation/use/browser.md`:
- Line 60: Update the navigation instruction in browser.md to use “research
specialists” consistently instead of “bounded reviewers,” reflecting that the
catalog includes planning and review roles and matching the existing navigation
and glossary terminology.
In `@documentation/use/specialists.md`:
- Around line 62-64: Update the Analysis Implementer documentation to
distinguish catalog visibility from execution: state that the role remains
visible in the gateway-owned catalog but is unavailable, while Heartwood does
not register it as an executable OpenHands specialist or grant it project tools.
Preserve the conservative condition that it remains unavailable until
restart-safe child approval and cancellation are supported.
- Line 3: Update the SPDX copyright header in specialists.md to use the
repository’s canonical attribution naming only the Schmiedmayer Lab at Stanford
University, without adding legacy organizational attributions.
In `@images/generic/scripts/local_model_stub.py`:
- Around line 238-242: Update specialist completion detection in the current
task flow around has_specialist_result and has_execution_result to inspect only
tool_results produced after task_index, rather than serialized_messages
containing the full conversation history. Ensure each cohort request
independently invokes research-planner when needed and retains the existing
terminal-action behavior after current-task completion; add a regression case
covering two cohort requests in one session.
In `@packages/schemas/tests/test_api_contracts.py`:
- Around line 89-111: Refactor
test_specialist_response_rejects_unsafe_contract_drift to build a valid
SpecialistSettingsResponse payload, then add parametrized cases that mutate
exactly one policy boundary at a time, covering route, tool, permission,
max_iterations, and max_budget_usd constraints. Keep each case’s expected
ValidationError assertion independent so a regression in any single boundary is
detected.
In `@packages/webui/src/styles.css`:
- Around line 1720-1722: Update the .specialist-row.unavailable style to remove
the opacity declaration so inherited text remains fully opaque, and distinguish
unavailable rows using a border or background treatment instead.
---
Nitpick comments:
In `@packages/gateway/src/heartwood/gateway/_openhands_sdk.py`:
- Around line 1556-1562: Update the failed TaskObservation branch in the
observation result projection to classify failures using a small fixed set of
bounded reasons, such as budget exhaustion, iteration-limit reached, or
specialist error, and append the selected reason to the parent-facing message.
Preserve the existing exclusion of free-form child text and keep
result_truncated false for these classified failures.
- Around line 1453-1459: In _tool_summary, remove the redundant outer truthiness
check around action.description and let the existing strip-and-nonempty check
handle empty or whitespace-only descriptions while preserving the fallback to
event.summary or “run {tool_name}”.
In `@packages/gateway/src/heartwood/gateway/_specialists.py`:
- Around line 253-266: Add a brief comment near the capability and availability
validation in the specialist definition flow, anchored around
SpecialistCapability.PROJECT_ACTIONS and SpecialistAvailability.AVAILABLE,
documenting that project-actions specialists must remain unavailable because
available specialists cannot declare tools. Do not alter the existing validation
behavior.
In `@packages/gateway/tests/test_openhands_sdk.py`:
- Around line 3519-3523: Cache the catalog returned by _specialist_catalog() so
its disk loading and parsing occur only once per test process or session; use
functools.lru_cache or an equivalent session-scoped fixture. In
packages/gateway/tests/test_openhands_sdk.py lines 3519-3523, apply the cache to
_specialist_catalog. In packages/gateway/tests/test_openhands_sdk.py line 2560,
hoist the expected label into a local variable before the any(...) assertion so
the catalog lookup is not repeated for each event.
- Around line 2828-2834: Update the replay assertion for cohort-feature-reviewer
to obtain the expected role label from the catalog, matching the existing
pattern used by the specialist test near line 2560, instead of hardcoding
"Cohort and Feature Reviewer"; keep the remaining BackendSubagentEvent and
completion checks unchanged.
In `@packages/gateway/tests/test_specialists.py`:
- Around line 82-106: Refactor test_catalog_rejects_role_boundary_widening to
parameterize each case with the original text, replacement text, and expected
message, matching the existing pattern used later in the file. Remove the
field-based conditional and perform one source.replace operation per case so
both model and permission_mode substitutions target the correct baseline values.
In `@packages/webui/src/components/UtilitySheet.tsx`:
- Around line 344-347: Update the specialist summary rendering in UtilitySheet
to consume the shared gateway role metadata or formatter, using
SpecialistSettings.capability and model_route instead of hard-coded “Advisory”
and “Uses the active model” labels. Keep the browser component limited to
adapting the shared projection while preserving the role.max_iterations step
count.
In `@packages/webui/src/e2e/app.spec.ts`:
- Around line 93-99: Update the browser contract test around the Specialists
panel and its mocked response to include an Analysis Implementer entry with
availability set to unavailable. Assert that its label remains visible and that
the panel displays the expected unavailable reason, while preserving the
existing Research Planner assertion.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ca556431-8dde-4bc5-a4a2-2f6926929954
⛔ Files ignored due to path filters (9)
documentation/assets/screenshots/browser-action-review.pngis excluded by!**/*.pngdocumentation/assets/screenshots/browser-action-settings.pngis excluded by!**/*.pngdocumentation/assets/screenshots/browser-changes.pngis excluded by!**/*.pngdocumentation/assets/screenshots/browser-conversation.pngis excluded by!**/*.pngdocumentation/assets/screenshots/browser-files.pngis excluded by!**/*.pngdocumentation/assets/screenshots/browser-specialists.pngis excluded by!**/*.pngpackages/webui/package-lock.jsonis excluded by!**/package-lock.jsonpackages/webui/src/apiTypes.generated.tsis excluded by!**/*.generated.*uv.lockis excluded by!**/*.lock
📒 Files selected for processing (92)
AGENTS.mdREADME.mdVERSION.tomlagents/verified/analysis-implementer.mdagents/verified/cohort-feature-reviewer.mdagents/verified/data-quality-reviewer.mdagents/verified/reproducibility-reviewer.mdagents/verified/research-planner.mdagents/verified/statistical-reviewer.mddeploy/tests/native_installer_real_smoke.shdocker-bake.hcldocumentation/architecture/system.mddocumentation/assets/screenshots/browser-specialists.png.licensedocumentation/index.mddocumentation/models/offline.mddocumentation/platforms/carina.mddocumentation/platforms/containers.mddocumentation/platforms/native-linux.mddocumentation/platforms/terra.mddocumentation/reference/cli.mddocumentation/reference/glossary.mddocumentation/use/actions-audit.mddocumentation/use/browser.mddocumentation/use/index.mddocumentation/use/notebooks.mddocumentation/use/specialists.mddocumentation/use/terminal.mdfixtures/synthetic/skills/omop-cohort-summary/SKILL.mdfixtures/synthetic/skills/omop-cohort-summary/metadata.jsonimages/generic/scripts/local_model_stub.pypackages/adapters/pyproject.tomlpackages/adapters/src/heartwood/adapters/__init__.pypackages/audit/pyproject.tomlpackages/audit/src/heartwood/audit/__init__.pypackages/cli/pyproject.tomlpackages/cli/src/heartwood/cli/__init__.pypackages/cli/src/heartwood/cli/_interactive.pypackages/cli/tests/test_cli.pypackages/cli/tests/test_interactive.pypackages/compliance/pyproject.tomlpackages/compliance/src/heartwood/compliance/__init__.pypackages/compliance/tests/test_container_assets.pypackages/core-adapter/pyproject.tomlpackages/core-adapter/src/heartwood/core_adapter/__init__.pypackages/core-adapter/src/heartwood/core_adapter/_facade.pypackages/core-adapter/src/heartwood/core_adapter/_service.pypackages/detector/pyproject.tomlpackages/detector/src/heartwood/detector/__init__.pypackages/fixtures/pyproject.tomlpackages/fixtures/src/heartwood/fixtures/__init__.pypackages/gateway/pyproject.tomlpackages/gateway/src/heartwood/gateway/_gateway.pypackages/gateway/src/heartwood/gateway/_openhands_sdk.pypackages/gateway/src/heartwood/gateway/_rest.pypackages/gateway/src/heartwood/gateway/_session_projection.pypackages/gateway/src/heartwood/gateway/_specialists.pypackages/gateway/tests/test_gateway_contract.pypackages/gateway/tests/test_openhands_sdk.pypackages/gateway/tests/test_specialists.pypackages/model-policy/pyproject.tomlpackages/model-policy/src/heartwood/model_policy/__init__.pypackages/notebook/pyproject.tomlpackages/notebook/src/heartwood/notebook/__init__.pypackages/notebook/src/heartwood/notebook/_view_model.pypackages/notebook/tests/test_notebook.pypackages/persistence/pyproject.tomlpackages/persistence/src/heartwood/persistence/__init__.pypackages/schemas/pyproject.tomlpackages/schemas/src/heartwood/schemas/__init__.pypackages/schemas/src/heartwood/schemas/_api.pypackages/schemas/tests/test_api_contracts.pypackages/session/pyproject.tomlpackages/session/src/heartwood/session/__init__.pypackages/skills/pyproject.tomlpackages/webui/package.jsonpackages/webui/scripts/smoke-reference-analysis.cjspackages/webui/src/App.test.tsxpackages/webui/src/App.tsxpackages/webui/src/client.test.tspackages/webui/src/client.tspackages/webui/src/components/SessionRail.tsxpackages/webui/src/components/UtilitySheet.tsxpackages/webui/src/e2e/app.spec.tspackages/webui/src/styles.csspackages/webui/src/types.tsskills/verified/aggregate-export/SKILL.mdskills/verified/aggregate-export/metadata.jsonskills/verified/baseline-model/SKILL.mdskills/verified/baseline-model/metadata.jsonskills/verified/omop-cohort-summary/SKILL.mdskills/verified/omop-cohort-summary/metadata.jsonzensical.toml
There was a problem hiding this comment.
🧹 Nitpick comments (8)
packages/gateway/tests/test_specialist_task.py (1)
24-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
_validated_roles.
_validated_rolesis the input boundary that builds the allowlist. Its three rejection paths are untested: an empty specialist list, incomplete metadata, and duplicatespecialist_idvalues. Add cases so a regression in the allowlist construction fails a test.♻️ Proposed additional tests
def test_validated_roles_rejects_empty_incomplete_and_duplicate_metadata() -> None: with pytest.raises(ValueError, match="At least one available specialist"): _validated_roles([]) with pytest.raises(ValueError, match="metadata must be complete"): _validated_roles( [{"specialist_id": "research-planner", "label": " ", "description": "Plan."}] ) role: SpecialistToolRole = { "specialist_id": "research-planner", "label": "Research Planner", "description": "Plan the analysis.", } with pytest.raises(ValueError, match="identifiers must be unique"): _validated_roles([role, dict(role)]) # type: ignore[list-item]As per coding guidelines: "Add or update tests when changing detector logic, policy decisions, adapter behavior, skill validation, audit records, attestation export, CLI output, notebook view models, or web-UI view models."
🤖 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 `@packages/gateway/tests/test_specialist_task.py` around lines 24 - 45, Add a test covering all three rejection paths in _validated_roles: empty specialist input, incomplete metadata, and duplicate specialist_id values. Assert each raises ValueError with the expected message, using a valid SpecialistToolRole duplicated for the uniqueness case.Source: Coding guidelines
packages/gateway/tests/test_openhands_persistence.py (1)
143-151: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlso assert the lifecycle fields survive minimization.
The projection needs
task_idandsubagentfrom the errored observation to render the specialist as stopped with an error. Add assertions so a future change to_minimize_eventcannot drop them silently.♻️ Proposed additional assertions
persisted = (root / "events" / "event-00000-12345678.json").read_text(encoding="utf-8") assert "participant-secret" not in persisted assert "/private/project" not in persisted assert "The agent could not complete the requested action" in persisted + assert "task_00000001" in persisted + assert "data-quality-reviewer" in persisted🤖 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 `@packages/gateway/tests/test_openhands_persistence.py` around lines 143 - 151, Extend the persistence assertions for the minimized errored observation written by store.write to verify that its task_id and subagent lifecycle fields remain present in persisted. Keep the existing secret, path, and error-message assertions unchanged, and assert the expected field values from the event fixture.packages/notebook/src/heartwood/notebook/_widgets.py (2)
99-114: 📐 Maintainability & Code Quality | 🔵 TrivialCapability wording differs from WebUI (see consolidated comment).
This is addressed in the consolidated comments section together with the equivalent CLI and WebUI locations.
🤖 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 `@packages/notebook/src/heartwood/notebook/_widgets.py` around lines 99 - 114, Update the capability label formatting in the action display loop around action.details.capability to match the wording and formatting used by the WebUI and CLI implementations; keep the existing task-only conditional and capability presence checks unchanged.
127-129: 🎯 Functional Correctness | 🔵 TrivialTask-heading fallback order differs from CLI (see consolidated comment).
This is addressed in the consolidated comments section together with the equivalent CLI and WebUI locations.
🤖 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 `@packages/notebook/src/heartwood/notebook/_widgets.py` around lines 127 - 129, Align the task-heading fallback expression assigned to label with the canonical fallback order used by the CLI and WebUI. Update the ordering of details.role_label, details.description, details.subagent_type, and action.summary consistently, preserving the existing values and fallback behavior.packages/webui/src/components/ConversationWorkspace.tsx (2)
767-798: 📐 Maintainability & Code Quality | 🔵 TrivialCapability wording is hardcoded and diverges from CLI/Notebook (see consolidated comment).
This is addressed in the consolidated comments section together with the equivalent CLI and Notebook locations.
🤖 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 `@packages/webui/src/components/ConversationWorkspace.tsx` around lines 767 - 798, Update the capability label rendering in ConversationWorkspace around specialist.capability to reuse the shared capability-to-display-label mapping used by the CLI and Notebook, instead of hardcoding “Advisory review” and “Project actions.” Preserve the existing conditional rendering and risk badge behavior.Source: Coding guidelines
566-572: 🎯 Functional Correctness | 🔵 TrivialTask-heading fallback order differs from CLI (see consolidated comment).
This is addressed in the consolidated comments section together with the equivalent CLI and Notebook locations.
🤖 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 `@packages/webui/src/components/ConversationWorkspace.tsx` around lines 566 - 572, Align the task-heading fallback chain in ConversationWorkspace with the canonical ordering used by the equivalent CLI and Notebook implementations. Update the action.details.kind === "task" branch to use the same field precedence, preserving the final action.summary fallback.images/generic/scripts/local_model_stub.py (1)
252-256: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate specialist-result check.
has_execution_resultre-implements the "specialist review complete:" substring check already encapsulated in_has_specialist_result. Derive it from that helper instead of duplicating the literal string check.♻️ Proposed refactor
- has_execution_result = any( - "specialist review complete:" not in json.dumps(result).lower() - for result in tool_results - ) + has_execution_result = bool(tool_results) and not all( + "specialist review complete:" in json.dumps(result).lower() + for result in tool_results + )🤖 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 `@images/generic/scripts/local_model_stub.py` around lines 252 - 256, Update the result-status logic near _has_specialist_result so has_execution_result derives from the existing has_specialist_result value instead of repeating the "specialist review complete:" check over tool_results. Remove the duplicated json.dumps-based condition while preserving the intended execution-result behavior.packages/cli/src/heartwood/cli/_interactive.py (1)
532-534: 🎯 Functional Correctness | 🔵 TrivialTask-heading fallback order differs from Notebook/WebUI (see consolidated comment).
This is addressed in the consolidated comments section together with the equivalent Notebook and WebUI locations.
🤖 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 `@packages/cli/src/heartwood/cli/_interactive.py` around lines 532 - 534, Update the specialist/task-heading fallback expression in the interactive CLI to match the consolidated Notebook and WebUI fallback order, reusing the same precedence and source fields across all three locations. Change the expression assigned to specialist while preserving its final fallback to action.tool_name.
🤖 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.
Nitpick comments:
In `@images/generic/scripts/local_model_stub.py`:
- Around line 252-256: Update the result-status logic near
_has_specialist_result so has_execution_result derives from the existing
has_specialist_result value instead of repeating the "specialist review
complete:" check over tool_results. Remove the duplicated json.dumps-based
condition while preserving the intended execution-result behavior.
In `@packages/cli/src/heartwood/cli/_interactive.py`:
- Around line 532-534: Update the specialist/task-heading fallback expression in
the interactive CLI to match the consolidated Notebook and WebUI fallback order,
reusing the same precedence and source fields across all three locations. Change
the expression assigned to specialist while preserving its final fallback to
action.tool_name.
In `@packages/gateway/tests/test_openhands_persistence.py`:
- Around line 143-151: Extend the persistence assertions for the minimized
errored observation written by store.write to verify that its task_id and
subagent lifecycle fields remain present in persisted. Keep the existing secret,
path, and error-message assertions unchanged, and assert the expected field
values from the event fixture.
In `@packages/gateway/tests/test_specialist_task.py`:
- Around line 24-45: Add a test covering all three rejection paths in
_validated_roles: empty specialist input, incomplete metadata, and duplicate
specialist_id values. Assert each raises ValueError with the expected message,
using a valid SpecialistToolRole duplicated for the uniqueness case.
In `@packages/notebook/src/heartwood/notebook/_widgets.py`:
- Around line 99-114: Update the capability label formatting in the action
display loop around action.details.capability to match the wording and
formatting used by the WebUI and CLI implementations; keep the existing
task-only conditional and capability presence checks unchanged.
- Around line 127-129: Align the task-heading fallback expression assigned to
label with the canonical fallback order used by the CLI and WebUI. Update the
ordering of details.role_label, details.description, details.subagent_type, and
action.summary consistently, preserving the existing values and fallback
behavior.
In `@packages/webui/src/components/ConversationWorkspace.tsx`:
- Around line 767-798: Update the capability label rendering in
ConversationWorkspace around specialist.capability to reuse the shared
capability-to-display-label mapping used by the CLI and Notebook, instead of
hardcoding “Advisory review” and “Project actions.” Preserve the existing
conditional rendering and risk badge behavior.
- Around line 566-572: Align the task-heading fallback chain in
ConversationWorkspace with the canonical ordering used by the equivalent CLI and
Notebook implementations. Update the action.details.kind === "task" branch to
use the same field precedence, preserving the final action.summary fallback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7d9c9833-53ce-4cee-8673-ed734f0c9e29
⛔ Files ignored due to path filters (9)
documentation/assets/screenshots/browser-action-review.pngis excluded by!**/*.pngdocumentation/assets/screenshots/browser-action-settings.pngis excluded by!**/*.pngdocumentation/assets/screenshots/browser-changes.pngis excluded by!**/*.pngdocumentation/assets/screenshots/browser-conversation.pngis excluded by!**/*.pngdocumentation/assets/screenshots/browser-files.pngis excluded by!**/*.pngdocumentation/assets/screenshots/browser-specialists.pngis excluded by!**/*.pngpackages/webui/src/apiTypes.generated.tsis excluded by!**/*.generated.*packages/webui/src/sessionProjection.generated.tsis excluded by!**/*.generated.*packages/webui/src/sessionProjectionSchema.generated.tsis excluded by!**/*.generated.*
📒 Files selected for processing (37)
agents/verified/analysis-implementer.mddocumentation/architecture/system.mddocumentation/reference/glossary.mddocumentation/use/browser.mddocumentation/use/specialists.mdimages/generic/scripts/local_model_stub.pyimages/generic/scripts/offline_stack_smoke.shimages/generic/scripts/terra_jupyter_demo_smoke.pypackages/cli/src/heartwood/cli/_interactive.pypackages/cli/tests/test_interactive.pypackages/compliance/tests/test_container_assets.pypackages/core-adapter/src/heartwood/core_adapter/_facade.pypackages/core-adapter/src/heartwood/core_adapter/_service.pypackages/gateway/src/heartwood/gateway/_action_presentation.pypackages/gateway/src/heartwood/gateway/_openhands_persistence.pypackages/gateway/src/heartwood/gateway/_openhands_sdk.pypackages/gateway/src/heartwood/gateway/_session_projection.pypackages/gateway/src/heartwood/gateway/_specialist_task.pypackages/gateway/src/heartwood/gateway/_specialists.pypackages/gateway/tests/test_action_settings.pypackages/gateway/tests/test_openhands_persistence.pypackages/gateway/tests/test_openhands_sdk.pypackages/gateway/tests/test_session_projection.pypackages/gateway/tests/test_specialist_task.pypackages/gateway/tests/test_specialists.pypackages/notebook/src/heartwood/notebook/_widgets.pypackages/notebook/tests/test_notebook.pypackages/schemas/src/heartwood/schemas/_api.pypackages/schemas/tests/test_api_contracts.pypackages/webui/scripts/smoke-reference-analysis.cjspackages/webui/src/App.test.tsxpackages/webui/src/actionPresentation.test.tspackages/webui/src/client.test.tspackages/webui/src/components/ConversationWorkspace.tsxpackages/webui/src/components/UtilitySheet.tsxpackages/webui/src/e2e/app.spec.tspackages/webui/src/styles.css
🚧 Files skipped from review as they are similar to previous changes (14)
- packages/webui/src/client.test.ts
- packages/core-adapter/src/heartwood/core_adapter/_service.py
- packages/schemas/tests/test_api_contracts.py
- packages/webui/src/e2e/app.spec.ts
- documentation/use/specialists.md
- documentation/reference/glossary.md
- agents/verified/analysis-implementer.md
- packages/webui/src/components/UtilitySheet.tsx
- packages/webui/src/styles.css
- packages/schemas/src/heartwood/schemas/_api.py
- packages/webui/scripts/smoke-reference-analysis.cjs
- packages/webui/src/App.test.tsx
- packages/gateway/src/heartwood/gateway/_specialists.py
- documentation/architecture/system.md
♻️ Current Situation & Problem
Closes #99.
Heartwood had one tool-free planner but no maintained research-specialist catalog or shared discovery surface. Tool-enabled child work must remain disabled until OpenHands provides restart-safe child approval and cancellation.
⚙️ Release Notes
0.3.0-beta.1.📚 Documentation
✅ Testing
Code of Conduct & Contributing Guidelines
By creating and submitting this pull request, you agree to follow our Code of Conduct and Contributing Guidelines: