Add Deployment Setup And NVIDIA Runtimes - #28
Conversation
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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 selected for processing (2)
📝 WalkthroughWalkthroughThis change adds Carina platform detection, deployment readiness inspection, persistent setup profiles, and shared CLI Sequence Diagram(s)sequenceDiagram
participant User
participant HeartwoodCLI
participant SessionGateway
participant LocalvLLM
User->>HeartwoodCLI: Run setup or chat
HeartwoodCLI->>SessionGateway: Inspect readiness and submit commands
SessionGateway->>LocalvLLM: Use configured model endpoint
LocalvLLM-->>SessionGateway: Return model responses
SessionGateway-->>HeartwoodCLI: Return events
HeartwoodCLI-->>User: Render conversation and diagnostics
Possibly related issues
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 |
There was a problem hiding this comment.
Pull request overview
This PR adds a state-aware, CLI-first deployment/setup workflow (with read-only diagnostics) and extends the container/deployment surface with explicit NVIDIA GPU runtime variants, aiming to make Carina-native evaluation and reproducible GPU inference contracts possible without changing the default portable image distribution.
Changes:
- Add
heartwood doctor, resumableheartwood setup, and state-aware bareheartwoodrouting driven by a shared readiness inspection. - Add deterministic Carina platform detection/adapter defaults plus native Carina bootstrap + Slurm launcher scripts and synthetic pilot documentation.
- Add explicit AMD64 NVIDIA image variants with an isolated pinned vLLM environment, plus CI workflows and contract tests to enforce the no-weights policy.
Reviewed changes
Copilot reviewed 40 out of 41 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Adds textual and its transitive dependencies to the locked workspace. |
| README.md | Links the new Carina CLI pilot documentation. |
| packages/gateway/tests/test_readiness.py | Adds coverage for readiness inspection and persisted setup profile behavior. |
| packages/gateway/src/heartwood/gateway/_readiness.py | Implements read-only deployment readiness inspection and atomic persistence of non-secret setup/policy/connection manifests. |
| packages/gateway/src/heartwood/gateway/_gateway.py | Loads persisted policy/connection manifests as fallbacks and selects platform adapter defaults. |
| packages/gateway/src/heartwood/gateway/init.py | Exposes readiness and setup persistence APIs from the gateway package. |
| packages/detector/tests/test_platform_detection.py | Adds Carina detection tests for explicit platform and cluster-name evidence. |
| packages/detector/src/heartwood/detector/_platforms.py | Introduces Carina as a detected platform via explicit env/Slurm cluster evidence. |
| packages/core-adapter/src/heartwood/core_adapter/_service.py | Selects platform adapter via detector evidence for local defaults. |
| packages/compliance/tests/test_container_assets.py | Extends container contract tests to include GPU variants, bake targets, and loopback enforcement. |
| packages/cli/tests/test_interactive.py | Adds tests for the interaction controller and Textual TUI behavior. |
| packages/cli/tests/test_cli.py | Adds tests for doctor, setup (interactive + non-interactive), and state-aware entry routing. |
| packages/cli/src/heartwood/cli/_tui.py | Adds a Textual full-screen terminal UI for interactive conversations. |
| packages/cli/src/heartwood/cli/_interactive.py | Adds a framework-neutral interaction controller mapping terminal commands to gateway commands. |
| packages/cli/src/heartwood/cli/init.py | Adds doctor, setup, state-aware bare invocation, and --plain fallback routing. |
| packages/cli/README.md | Updates CLI documentation for setup/readiness and TUI/plain modes. |
| packages/cli/pyproject.toml | Adds textual==8.2.8 to CLI dependencies. |
| packages/adapters/tests/test_generic_adapters.py | Adds conformance/default-policy assertions for the Carina platform adapter. |
| packages/adapters/src/heartwood/adapters/platform/carina.py | Adds the Carina platform adapter with conservative local-only policy defaults. |
| packages/adapters/src/heartwood/adapters/platform/init.py | Adds adapter selection helper based on detector evidence. |
| images/platforms.toml | Extends Terra platform metadata for GPU flavor tags/targets and pinned GPU runtime info. |
| images/platform/Dockerfile | Adds optional isolated vLLM env creation when HEARTWOOD_GPU_RUNTIME=vllm. |
| images/gpu/vllm-requirements.txt | Introduces a locked vLLM dependency set for GPU image/env builds. |
| images/gpu/start_vllm.sh | Adds a loopback-enforcing vLLM launcher script (no downloads/weights). |
| images/generic/image-flavors.toml | Adds explicit GPU flavor metadata for generic and Terra-derived NVIDIA variants. |
| images/generic/Dockerfile | Adds optional isolated vLLM env creation and sets default HEARTWOOD_HOME/WORKSPACE. |
| docs/README.md | Adds docs index entry for the Carina CLI pilot guide. |
| docs/platform-support.md | Updates platform matrix for Carina CLI and explicit GPU image variants. |
| docs/container-images.md | Documents new GPU image tags and describes isolated vLLM runtime behavior. |
| docs/carina-cli.md | Adds synthetic-only Carina CLI pilot runbook and safety boundaries. |
| docker-bake.hcl | Adds runtime-gpu-nvidia and terra-runtime-gpu-nvidia build targets/tags. |
| design/09-implementation-plan.md | Updates implementation plan to include readiness/setup flow, Carina/NVIDIA contracts, and event-stream limitations. |
| design/03-architecture.md | Documents Textual as terminal presentation and the interaction-controller boundary. |
| design/02-platforms.md | Documents rationale and constraints for explicit NVIDIA variants and Stanford gateway routing. |
| deploy/carina/synthetic-acceptance.sbatch | Adds a synthetic acceptance Slurm job wrapper for the Carina pilot. |
| deploy/carina/launch-interactive.sh | Adds Carina allocation launcher: model verification/staging, env scrubbing, vLLM startup, setup+chat. |
| deploy/carina/environment.yml | Adds a minimal Micromamba bootstrap environment definition. |
| deploy/carina/bootstrap.sh | Adds Micromamba+uv bootstrap for separate Heartwood and vLLM environments. |
| .github/workflows/gpu-container-image.yml | Adds workflow to validate/publish GPU images on main and validate contracts on PRs. |
| .github/workflows/container-smoke.yml | Extends smoke checks to include new GPU bake targets. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/detector/src/heartwood/detector/_platforms.py (1)
87-118: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winExplicit Carina override isn't deterministic — it can be outvoted by unrelated markers.
The explicit-override block only inserts into
matches[Platform.CARINA]; it doesn't short-circuit the later ambiguity check. If an environment happens to also carry a marker for another platform (e.g. a staleGOOGLE_PROJECT),len(matches) > 1triggers the ambiguous branch, and the tie-break ((len(evidence), platform.value)) can pick a different platform over Carina even though the caller explicitly setHEARTWOOD_PLATFORM=carinaorSLURM_CLUSTER_NAME=carina. This undermines the PR's stated goal of "Deterministic Carina detection" and can route a session to the wrong platform adapter/policy profile.🐛 Proposed fix: make the explicit override authoritative
explicit_platform = env.get("HEARTWOOD_PLATFORM", "").strip().lower() slurm_cluster = env.get("SLURM_CLUSTER_NAME", "").strip().lower() if explicit_platform == Platform.CARINA.value or slurm_cluster in {"carina", "carina2"}: - carina_evidence: list[str] = [] + carina_evidence: list[str] = [] if explicit_platform == Platform.CARINA.value: carina_evidence.append("HEARTWOOD_PLATFORM=carina") if slurm_cluster in {"carina", "carina2"}: carina_evidence.append(f"SLURM_CLUSTER_NAME={slurm_cluster}") - matches[Platform.CARINA] = carina_evidence + # An explicit override is authoritative and must not be diluted by + # incidental markers for other platforms present in the environment. + evidence = tuple(f"found environment marker {marker}" for marker in carina_evidence) + return PlatformDetection(platform=Platform.CARINA, confidence=0.99, evidence=evidence)🤖 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/detector/src/heartwood/detector/_platforms.py` around lines 87 - 118, Make the Carina override authoritative in the platform-detection flow: when HEARTWOOD_PLATFORM is set to carina or SLURM_CLUSTER_NAME is carina/carina2, return a Carina PlatformDetection immediately using the collected carina_evidence, before the generic no-match or ambiguity handling. Preserve the existing evidence formatting and confidence behavior for this override, and leave unrelated platform detection unchanged.
🧹 Nitpick comments (7)
packages/cli/src/heartwood/cli/_interactive.py (1)
60-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCommands with wrong argument count report "Unknown command" instead of a usage hint.
When a known command like
/allowis submitted without its required argument (or/pausewith extra args), it falls through toUnknown command: /allow. This is misleading — the command is recognized, just mis-argmented. Consider returning a usage message for arity mismatches on known directives.♻️ Proposed refactor
directive = parts[0] - if directive in {"/quit", "/exit"} and len(parts) == 1: + if directive in {"/quit", "/exit"}: + if len(parts) != 1: + return InteractionResult(message="Usage: /quit") return InteractionResult(exit_requested=True) - if directive in {"/allow", "/reject"} and len(parts) == 2: + if directive in {"/allow", "/reject"}: + if len(parts) != 2: + return InteractionResult(message=f"Usage: {directive} <id>") kind = CommandKind.APPROVE if directive == "/allow" else CommandKind.DENY return InteractionResult( events=self._handle( kind, {"target_type": "tool-call", "target_id": parts[1]}, ) ) - if directive == "/pause" and len(parts) == 1: + if directive == "/pause": + if len(parts) != 1: + return InteractionResult(message="Usage: /pause") return InteractionResult(events=self._handle(CommandKind.PAUSE)) - if directive == "/resume" and len(parts) == 1: + if directive == "/resume": + if len(parts) != 1: + return InteractionResult(message="Usage: /resume") return InteractionResult(events=self._handle(CommandKind.RESUME)) - if directive == "/replay" and len(parts) == 1: + if directive == "/replay": + if len(parts) != 1: + return InteractionResult(message="Usage: /replay") return InteractionResult(events=self.replay(), replace_transcript=True) - if directive == "/audit-export" and len(parts) == 1: + if directive == "/audit-export": + if len(parts) != 1: + return InteractionResult(message="Usage: /audit-export") return InteractionResult(events=self._handle(CommandKind.AUDIT_EXPORT)) - if directive == "/status" and len(parts) == 1: + if directive == "/status": + if len(parts) != 1: + return InteractionResult(message="Usage: /status") try: return InteractionResult(message=format_model_status(self.gateway)) except ModelSettingsError as error: return InteractionResult(message=str(error)) - if directive == "/help" and len(parts) == 1: + if directive == "/help": + if len(parts) != 1: + return InteractionResult(message="Usage: /help") return InteractionResult(message=command_help()) return InteractionResult(message=f"Unknown command: {directive}")Note:
test_interactive.pyline 37 assertsinvalid.message == "Unknown command: /allow"— update it to"Usage: /allow <id>"if this refactor is applied.🤖 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 60 - 85, Update the directive handling in the interactive command parser so recognized commands with incorrect argument counts return usage hints instead of the generic unknown-command message. Cover commands such as /allow and /reject with their required identifier, and argumentless commands such as /pause, /resume, /replay, /audit-export, /status, /help, /quit, and /exit with their expected usage; preserve normal dispatch for valid arity and update the related test expectation.packages/adapters/tests/test_generic_adapters.py (1)
29-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding a negative-case detection test.
The test verifies positive detection (
HEARTWOOD_PLATFORM=carina→ confidence > 0.0) but doesn't cover the fallback path where Carina evidence is absent (confidence should be 0.0). Adding this would guard against regressions in thedetect()fallback branch.♻️ Optional: add negative-case assertion
def test_carina_platform_adapter_conforms_and_defaults_to_local_only() -> None: adapter = CarinaPlatformAdapter() assert_platform_adapter_conforms(adapter) detection = adapter.detect({"HEARTWOOD_PLATFORM": "carina"}) policy = adapter.default_policy_profile() assert detection.adapter_id == "carina" assert detection.confidence > 0.0 assert adapter.data_mounts() == () assert policy.platform_id == "carina" assert policy.allowed_action_confirmation_modes == ("always-confirm",) assert policy.credential_allowlist == () + + +def test_carina_platform_adapter_returns_zero_confidence_when_not_carina() -> None: + adapter = CarinaPlatformAdapter() + detection = adapter.detect({"HEARTWOOD_PLATFORM": "generic"}) + assert detection.adapter_id == "carina" + assert detection.confidence == 0.0🤖 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/adapters/tests/test_generic_adapters.py` around lines 29 - 40, Add a negative-case assertion to test_carina_platform_adapter_conforms_and_defaults_to_local_only by calling detect() without Carina evidence and verifying the returned confidence is 0.0, while preserving the existing positive-detection assertions.design/09-implementation-plan.md (1)
86-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSplit this bullet into one deliverable per line.
This checkbox merges three independently-trackable deliverables (
heartwood doctor,heartwood setup, and bareheartwoodrouting) into a single item, unlike every neighboring bullet which tracks one deliverable. As per coding guidelines, "Keep implementation plans compact, directive, and actionable" — splitting lets each piece be checked off independently.✏️ Suggested split
-- [ ] Implement `heartwood doctor` as a read-only environment, storage, accelerator, model-route, credential-reference, and policy diagnostic. Implement `heartwood setup` as the shared resumable first-run flow over the same gateway settings used by the CLI, web UI, and notebook bridge. Platform adapters may contribute detected defaults and validation steps, but setup must show its evidence, require confirmation before mutation or download, and support explicit non-interactive configuration for automation. Make bare `heartwood` state-aware in an interactive terminal: open setup when required configuration is absent, present concise recovery choices when diagnostics fail, and open the conversation when setup is valid; retain help-only behavior for non-interactive invocation and never mutate state merely because no subcommand was supplied. +- [ ] Implement `heartwood doctor` as a read-only environment, storage, accelerator, model-route, credential-reference, and policy diagnostic. +- [ ] Implement `heartwood setup` as the shared resumable first-run flow over the same gateway settings used by the CLI, web UI, and notebook bridge. Platform adapters may contribute detected defaults and validation steps, but setup must show its evidence, require confirmation before mutation or download, and support explicit non-interactive configuration for automation. +- [ ] Make bare `heartwood` state-aware in an interactive terminal: open setup when required configuration is absent, present concise recovery choices when diagnostics fail, and open the conversation when setup is valid; retain help-only behavior for non-interactive invocation and never mutate state merely because no subcommand was supplied.🤖 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 `@design/09-implementation-plan.md` at line 86, Split the implementation-plan checkbox into three independently checkable deliverables: one for the read-only `heartwood doctor` diagnostics, one for the resumable `heartwood setup` flow and its confirmation/non-interactive behavior, and one for state-aware bare `heartwood` routing with preserved non-interactive help behavior.Source: Coding guidelines
images/platform/Dockerfile (1)
120-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffDuplicated GPU-runtime provisioning logic across two Dockerfiles.
This exact conditional block (create
/opt/heartwood-vllmvenv,uv pip sync --require-hashesagainstimages/gpu/vllm-requirements.txt, else abort on unsupported value) is duplicated verbatim inimages/generic/Dockerfile. Consider extracting it into a shared build script (e.g.,images/gpu/install_vllm_runtime.sh) invoked with a--python-binargument from both Dockerfiles to avoid the two copies drifting.🤖 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/platform/Dockerfile` around lines 120 - 127, Extract the duplicated GPU-runtime conditional from the Dockerfiles into a shared script, such as images/gpu/install_vllm_runtime.sh, accepting a --python-bin argument. Move the vllm virtual-environment creation, hashed dependency sync, and unsupported-runtime failure into that script, then update both images/platform/Dockerfile and images/generic/Dockerfile to invoke it while preserving their existing Python target and runtime behavior.images/platforms.toml (1)
49-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
gpu_runtimeduplicates the vLLM version pin without a consistency check.
gpu_runtime = "vllm==0.25.0"re-declares the same version already pinned inimages/gpu/vllm-requirements.txt(confirmedvllm==0.25.0in the lock file per the test suite). Nothing cross-validates these two sources, so bumping the lock file without updating this field (or vice versa) silently produces stale/misleading platform metadata.♻️ Suggested consistency guard
Either derive
gpu_runtimefrom the lock file at build/test time, or add an assertion intest_container_assets.pythat parses the pinnedvllm==line fromimages/gpu/vllm-requirements.txtand compares it againstterra["gpu_runtime"].🤖 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/platforms.toml` around lines 49 - 57, Keep the vLLM version in platforms.toml and images/gpu/vllm-requirements.txt consistent by adding a test assertion in test_container_assets.py that parses the pinned vllm== version from the requirements file and compares it with terra["gpu_runtime"]; fail when they differ.packages/cli/src/heartwood/cli/_tui.py (1)
60-60: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
_render_events/format_eventfailures during replay would crash the app before any UI is usable.
on_mountcallsself._render_events(self.session.replay())unguarded (Line 60), andformat_eventis an externally supplied callback. An exception from formatting a single persistedSessionEvent(e.g. malformed/older-schema audit data) would propagate out ofon_mountand abort the whole terminal session before the user can do anything, including recover via/replay.Consider wrapping the per-event formatting in a try/except so one bad event degrades gracefully instead of taking down the app.
Also applies to: 90-94
🤖 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/_tui.py` at line 60, Guard per-event formatting during replay so a failure from the externally supplied format_event callback or malformed SessionEvent does not abort on_mount. Update _render_events (used by self.session.replay()) to catch formatting errors for individual events, degrade gracefully, and continue rendering subsequent events while preserving normal behavior for valid events.docker-bake.hcl (1)
35-47: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winScope the GHA cache per target to avoid cache thrashing.
Neither new target sets a unique
scopefor itstype=ghacache entries, so they share the defaultbuildkitscope withruntimeandterra-runtime. Per Docker's docs, building multiple images with the same unscoped GHA cache means "each build will overwrite the cache of the previous, leaving only the final cache." With four targets now sharing this scope, most builds will effectively rebuild from scratch instead of hitting cache.♻️ Proposed fix: scope caches per target
target "runtime-gpu-nvidia" { inherits = ["runtime"] platforms = ["linux/amd64"] + cache-from = ["type=gha,scope=runtime-gpu-nvidia"] + cache-to = ["type=gha,mode=min,scope=runtime-gpu-nvidia"] args = {target "terra-runtime-gpu-nvidia" { inherits = ["_terra_common"] - cache-from = ["type=gha"] - cache-to = ["type=gha,mode=min"] + cache-from = ["type=gha,scope=terra-runtime-gpu-nvidia"] + cache-to = ["type=gha,mode=min,scope=terra-runtime-gpu-nvidia"] output = ["type=registry,oci-mediatypes=false"]Also applies to: 95-109
🤖 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 `@docker-bake.hcl` around lines 35 - 47, Assign unique GitHub Actions cache scopes to the runtime-gpu-nvidia target and the other newly added target referenced in the comment. Update each target’s type=gha cache configuration to use a target-specific scope instead of the shared default buildkit scope, while preserving the existing cache settings and target behavior.
🤖 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 `@deploy/carina/bootstrap.sh`:
- Around line 19-29: Make the micromamba bootstrap step in the environment
creation block idempotent: detect whether the "${root}/bootstrap" prefix already
exists, then reuse it when valid or remove and recreate it when invalid before
invoking micromamba create. Preserve the existing repository-root and micromamba
prerequisite checks.
In `@deploy/carina/interactive.sbatch`:
- Around line 21-24: Update the launch command in the interactive sbatch script
to resolve launch-interactive.sh relative to the sbatch script’s own location
using BASH_SOURCE, rather than the Slurm submit directory. Preserve the existing
argument forwarding and exec behavior so submissions work from any directory.
In `@deploy/carina/launch-interactive.sh`:
- Line 37: Derive the repository root from the launch script’s BASH_SOURCE
location, then use that root to construct absolute paths for both
verify_model_snapshot.py and start_vllm.sh. Update the call sites around the
existing Python verification invocation and start_vllm invocation so they work
regardless of the Slurm submit directory.
- Around line 42-49: Update cleanup() to wait for runtime_pid only within a
bounded timeout after sending SIGTERM, then force-kill the process if it remains
alive and perform a final reaping wait before removing staged_model. Preserve
the existing cleanup behavior for missing or already-exited processes.
In `@deploy/carina/verify_model_snapshot.py`:
- Around line 42-69: Update the digest-reading loop in the snapshot verification
function to open each file with the platform’s O_NOFOLLOW flag, following the
existing pattern in heartwood.audit._log.append. Preserve the prior symlink
scan, but make each hashing open reject a path replaced by a symlink between
passes and propagate the resulting failure instead of reading the linked file.
In `@design/03-architecture.md`:
- Line 52: Rewrite the third-column description in the “Terminal presentation”
row to state Heartwood’s responsibilities relative to Textual, rather than
listing Textual’s rendering and testing capabilities. Preserve the ownership
boundary that Textual supplies the framework while Heartwood owns the
integration, presentation configuration, and interaction translation
responsibilities, consistent with the surrounding architecture prose.
In `@images/platform/Dockerfile`:
- Around line 120-127: Update the ownership fix-up after the vLLM setup to
include /opt/heartwood-vllm for the final non-root user. Guard the chown with a
directory-existence check, or ensure the directory is created for all GPU
runtime values so the none configuration does not fail the build.
In `@packages/cli/src/heartwood/cli/__init__.py`:
- Around line 364-394: Handle EOFError from the interactive input calls in
_handle_setup for source selection and action confirmation, converting closed or
empty stdin into a clean setup error or cancellation message with the existing
nonzero return behavior. Keep normal interactive responses and the explicit
--non-interactive validation unchanged, and ensure no traceback escapes before
configuration is persisted.
---
Outside diff comments:
In `@packages/detector/src/heartwood/detector/_platforms.py`:
- Around line 87-118: Make the Carina override authoritative in the
platform-detection flow: when HEARTWOOD_PLATFORM is set to carina or
SLURM_CLUSTER_NAME is carina/carina2, return a Carina PlatformDetection
immediately using the collected carina_evidence, before the generic no-match or
ambiguity handling. Preserve the existing evidence formatting and confidence
behavior for this override, and leave unrelated platform detection unchanged.
---
Nitpick comments:
In `@design/09-implementation-plan.md`:
- Line 86: Split the implementation-plan checkbox into three independently
checkable deliverables: one for the read-only `heartwood doctor` diagnostics,
one for the resumable `heartwood setup` flow and its
confirmation/non-interactive behavior, and one for state-aware bare `heartwood`
routing with preserved non-interactive help behavior.
In `@docker-bake.hcl`:
- Around line 35-47: Assign unique GitHub Actions cache scopes to the
runtime-gpu-nvidia target and the other newly added target referenced in the
comment. Update each target’s type=gha cache configuration to use a
target-specific scope instead of the shared default buildkit scope, while
preserving the existing cache settings and target behavior.
In `@images/platform/Dockerfile`:
- Around line 120-127: Extract the duplicated GPU-runtime conditional from the
Dockerfiles into a shared script, such as images/gpu/install_vllm_runtime.sh,
accepting a --python-bin argument. Move the vllm virtual-environment creation,
hashed dependency sync, and unsupported-runtime failure into that script, then
update both images/platform/Dockerfile and images/generic/Dockerfile to invoke
it while preserving their existing Python target and runtime behavior.
In `@images/platforms.toml`:
- Around line 49-57: Keep the vLLM version in platforms.toml and
images/gpu/vllm-requirements.txt consistent by adding a test assertion in
test_container_assets.py that parses the pinned vllm== version from the
requirements file and compares it with terra["gpu_runtime"]; fail when they
differ.
In `@packages/adapters/tests/test_generic_adapters.py`:
- Around line 29-40: Add a negative-case assertion to
test_carina_platform_adapter_conforms_and_defaults_to_local_only by calling
detect() without Carina evidence and verifying the returned confidence is 0.0,
while preserving the existing positive-detection assertions.
In `@packages/cli/src/heartwood/cli/_interactive.py`:
- Around line 60-85: Update the directive handling in the interactive command
parser so recognized commands with incorrect argument counts return usage hints
instead of the generic unknown-command message. Cover commands such as /allow
and /reject with their required identifier, and argumentless commands such as
/pause, /resume, /replay, /audit-export, /status, /help, /quit, and /exit with
their expected usage; preserve normal dispatch for valid arity and update the
related test expectation.
In `@packages/cli/src/heartwood/cli/_tui.py`:
- Line 60: Guard per-event formatting during replay so a failure from the
externally supplied format_event callback or malformed SessionEvent does not
abort on_mount. Update _render_events (used by self.session.replay()) to catch
formatting errors for individual events, degrade gracefully, and continue
rendering subsequent events while preserving normal behavior for valid events.
🪄 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: 68cf1c4b-059b-47ea-b7ed-aab5243f28c2
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (45)
.github/workflows/container-smoke.yml.github/workflows/dependency-review.yml.github/workflows/gpu-container-image.ymlREADME.mdREUSE.tomldeploy/carina/bootstrap.shdeploy/carina/environment.ymldeploy/carina/interactive.sbatchdeploy/carina/launch-interactive.shdeploy/carina/verify_model_snapshot.pydesign/02-platforms.mddesign/03-architecture.mddesign/09-implementation-plan.mddocker-bake.hcldocs/README.mddocs/carina-cli.mddocs/container-images.mddocs/platform-support.mdimages/generic/Dockerfileimages/generic/image-flavors.tomlimages/generic/scripts/capable_model_e2e.shimages/generic/scripts/offline_stack_smoke.shimages/gpu/start_vllm.shimages/gpu/vllm-requirements.txtimages/gpu/vllm.inimages/platform/Dockerfileimages/platforms.tomlpackages/adapters/src/heartwood/adapters/platform/__init__.pypackages/adapters/src/heartwood/adapters/platform/carina.pypackages/adapters/tests/test_generic_adapters.pypackages/cli/README.mdpackages/cli/pyproject.tomlpackages/cli/src/heartwood/cli/__init__.pypackages/cli/src/heartwood/cli/_interactive.pypackages/cli/src/heartwood/cli/_tui.pypackages/cli/tests/test_cli.pypackages/cli/tests/test_interactive.pypackages/compliance/tests/test_container_assets.pypackages/core-adapter/src/heartwood/core_adapter/_service.pypackages/detector/src/heartwood/detector/_platforms.pypackages/detector/tests/test_platform_detection.pypackages/gateway/src/heartwood/gateway/__init__.pypackages/gateway/src/heartwood/gateway/_gateway.pypackages/gateway/src/heartwood/gateway/_readiness.pypackages/gateway/tests/test_readiness.py
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)
docs/carina-cli.md (1)
74-76: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAvoid placing the API key in shell history.
Replacing
<runtime-secret>with a real value inexportcommonly records the credential in shell history, conflicting with the documented “never stores its value” boundary. Use secret injection or silent input instead.Proposed safer example
-export STANFORD_AI_API_KEY="<runtime-secret>" +read -r -s -p "Stanford AI API key: " STANFORD_AI_API_KEY +printf '\n' +export STANFORD_AI_API_KEY🤖 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 `@docs/carina-cli.md` around lines 74 - 76, Update the Stanford AI setup example around the STANFORD_AI_API_KEY environment variable to avoid embedding the credential in a shell command that can enter shell history. Show a secret-injection or silent-input mechanism while preserving the existing heartwood setup invocation and credential configuration behavior.
🤖 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 `@docs/carina-cli.md`:
- Around line 74-76: Update the Stanford AI setup example around the
STANFORD_AI_API_KEY environment variable to avoid embedding the credential in a
shell command that can enter shell history. Show a secret-injection or
silent-input mechanism while preserving the existing heartwood setup invocation
and credential configuration behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b3fc538c-90ac-44ab-a552-69f055dc82f1
📒 Files selected for processing (4)
deploy/carina/interactive.sbatchdocs/carina-cli.mdimages/platform/Dockerfilepackages/compliance/tests/test_container_assets.py
🚧 Files skipped from review as they are similar to previous changes (3)
- deploy/carina/interactive.sbatch
- images/platform/Dockerfile
- packages/compliance/tests/test_container_assets.py
9358d0b to
354da9b
Compare
354da9b to
1506b68
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@design/03-architecture.md`:
- Around line 123-126: Update the sequence diagram text around the “streamed
result and activity” reference to describe delivery of a completed event batch
instead. Keep the diagram consistent with the gateway’s blocking command
contract and avoid implying token streaming or mid-turn cancellation.
In `@docs/platform-support.md`:
- Line 21: Update the Stanford Carina CLI row in the platform-support table to
explicitly state that no Carina-specific image is published. Retain that
deployment uses native Micromamba environments and clarify that edge-gpu-nvidia
is only the equivalent generic container target, not a Carina-specific
container.
🪄 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: 85d77cbc-ae11-4c1c-b933-055a9878c864
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (45)
.github/workflows/container-smoke.yml.github/workflows/dependency-review.yml.github/workflows/gpu-container-image.ymlREADME.mdREUSE.tomldeploy/carina/bootstrap.shdeploy/carina/environment.ymldeploy/carina/interactive.sbatchdeploy/carina/launch-interactive.shdeploy/carina/verify_model_snapshot.pydesign/02-platforms.mddesign/03-architecture.mddesign/09-implementation-plan.mddocker-bake.hcldocs/README.mddocs/carina-cli.mddocs/container-images.mddocs/platform-support.mdimages/generic/Dockerfileimages/generic/image-flavors.tomlimages/generic/scripts/capable_model_e2e.shimages/generic/scripts/offline_stack_smoke.shimages/gpu/start_vllm.shimages/gpu/vllm-requirements.txtimages/gpu/vllm.inimages/platform/Dockerfileimages/platforms.tomlpackages/adapters/src/heartwood/adapters/platform/__init__.pypackages/adapters/src/heartwood/adapters/platform/carina.pypackages/adapters/tests/test_generic_adapters.pypackages/cli/README.mdpackages/cli/pyproject.tomlpackages/cli/src/heartwood/cli/__init__.pypackages/cli/src/heartwood/cli/_interactive.pypackages/cli/src/heartwood/cli/_tui.pypackages/cli/tests/test_cli.pypackages/cli/tests/test_interactive.pypackages/compliance/tests/test_container_assets.pypackages/core-adapter/src/heartwood/core_adapter/_service.pypackages/detector/src/heartwood/detector/_platforms.pypackages/detector/tests/test_platform_detection.pypackages/gateway/src/heartwood/gateway/__init__.pypackages/gateway/src/heartwood/gateway/_gateway.pypackages/gateway/src/heartwood/gateway/_readiness.pypackages/gateway/tests/test_readiness.py
🚧 Files skipped from review as they are similar to previous changes (39)
- images/gpu/vllm.in
- .github/workflows/dependency-review.yml
- packages/cli/pyproject.toml
- REUSE.toml
- deploy/carina/interactive.sbatch
- docs/README.md
- .github/workflows/container-smoke.yml
- images/generic/scripts/capable_model_e2e.sh
- README.md
- packages/cli/README.md
- deploy/carina/environment.yml
- packages/cli/tests/test_interactive.py
- packages/adapters/src/heartwood/adapters/platform/init.py
- deploy/carina/verify_model_snapshot.py
- images/generic/image-flavors.toml
- packages/core-adapter/src/heartwood/core_adapter/_service.py
- docs/carina-cli.md
- images/generic/scripts/offline_stack_smoke.sh
- deploy/carina/launch-interactive.sh
- images/gpu/vllm-requirements.txt
- packages/gateway/src/heartwood/gateway/init.py
- docker-bake.hcl
- packages/detector/src/heartwood/detector/_platforms.py
- images/platforms.toml
- packages/detector/tests/test_platform_detection.py
- images/platform/Dockerfile
- packages/adapters/src/heartwood/adapters/platform/carina.py
- docs/container-images.md
- design/02-platforms.md
- .github/workflows/gpu-container-image.yml
- packages/cli/src/heartwood/cli/_tui.py
- images/gpu/start_vllm.sh
- images/generic/Dockerfile
- packages/cli/src/heartwood/cli/_interactive.py
- design/09-implementation-plan.md
- deploy/carina/bootstrap.sh
- packages/gateway/src/heartwood/gateway/_readiness.py
- packages/gateway/src/heartwood/gateway/_gateway.py
- packages/adapters/tests/test_generic_adapters.py
### ♻️ Current Situation & Problem Heartwood needs a state-aware first-run path and a reproducible Carina deployment contract before the interactive CLI can be evaluated in an isolated Stanford compute allocation. The portable images also need explicit NVIDIA variants without changing the default multi-architecture, no-weight distribution. This advances #25 and the shared setup work tracked in #22. Live Carina, native GPU, and Stanford AI API Gateway validation remain explicit acceptance evidence rather than repository support claims. ### ⚙️ Release Notes - Add read-only `heartwood doctor`, resumable `heartwood setup`, and state-aware bare `heartwood` routing. - Add deterministic Carina detection, a conservative platform adapter, native Micromamba bootstrap, verified model staging, and an allocation-scoped vLLM launcher. - Add a Stanford AI API Gateway connection through the existing OpenAI-compatible catalog and model-settings path without persisting credentials. - Add explicit AMD64 `edge-gpu-nvidia` and `edge-terra-gpu-nvidia` publication targets with isolated vLLM `0.25.0`, external model weights, immutable commit tags, and Docker schema-2 Terra publication. - Validate setup as one coherent platform, model, connection, policy, credential-reference, and action-mode contract, and restore the prior settings if reconfiguration fails. - Require exact hash-manifest coverage for Carina model snapshots and artifact hashes for every vLLM dependency. - Build both GPU targets on pull requests and coordinate immutable-tag verification and channel promotion on `main`. - Keep portable AMD64/ARM64 images as the default distribution. ### 📚 Documentation - Add the synthetic-only [Carina CLI pilot](docs/carina-cli.md). - Update the platform support matrix, container image reference, CLI guide, README index, and delivery roadmap. - Distinguish implemented and CI-testable contracts from the remaining live Carina, native NVIDIA, Stanford service, and complete synthetic acceptance evidence. ### ✅ Testing - `uv run pytest`: 392 tests passed; 90.07% total coverage. - `uv run ruff check .`: passed. - `uv run mypy packages`: passed with strict typing across 84 source files. - Docker build checks passed for generic and Terra GPU definitions. - A real local AMD64 GPU image built successfully, reported vLLM `0.25.0`, and contained no large bundled model artifacts. - Shell syntax, bake graph, launcher loopback enforcement, setup persistence, credential scrubbing, platform detection, and publication contracts are covered. - Native GPU execution and live platform/service validation require approved external infrastructure and remain outstanding evidence. ### Code of Conduct & Contributing Guidelines By creating and submitting this pull request, you agree to follow our [Code of Conduct](https://github.com/SchmiedmayerLab/.github/blob/main/CODE_OF_CONDUCT.md) and [Contributing Guidelines](https://github.com/SchmiedmayerLab/.github/blob/main/CONTRIBUTING.md): - [x] I agree to follow the [Code of Conduct](https://github.com/SchmiedmayerLab/.github/blob/main/CODE_OF_CONDUCT.md) and [Contributing Guidelines](https://github.com/SchmiedmayerLab/.github/blob/main/CONTRIBUTING.md).
♻️ Current Situation & Problem
Heartwood needs a state-aware first-run path and a reproducible Carina deployment contract before the interactive CLI can be evaluated in an isolated Stanford compute allocation. The portable images also need explicit NVIDIA variants without changing the default multi-architecture, no-weight distribution.
This advances #25 and the shared setup work tracked in #22. Live Carina, native GPU, and Stanford AI API Gateway validation remain explicit acceptance evidence rather than repository support claims.
⚙️ Release Notes
heartwood doctor, resumableheartwood setup, and state-aware bareheartwoodrouting.edge-gpu-nvidiaandedge-terra-gpu-nvidiapublication targets with isolated vLLM0.25.0, external model weights, immutable commit tags, and Docker schema-2 Terra publication.main.📚 Documentation
✅ Testing
uv run pytest: 392 tests passed; 90.07% total coverage.uv run ruff check .: passed.uv run mypy packages: passed with strict typing across 84 source files.0.25.0, and contained no large bundled model artifacts.Code of Conduct & Contributing Guidelines
By creating and submitting this pull request, you agree to follow our Code of Conduct and Contributing Guidelines: