feat(runtime): Phase D negative test — admission denial fails closed, no Ollama fallback by hardcoreerik · Pull Request #78 · hardcoreerik/TheOrc

hardcoreerik · 2026-07-19T06:55:06Z

Summary

Lands the first piece of Native Runtime v2.0 Phase D (docs/NATIVE_RUNTIME_V2_SPEC.md §3.4/Phase D): the explicit negative requirement that a native-routed job with admission denied must fail closed with no silent Ollama substitution.
Wires the real objects together (RuntimeOrchestrator + a real OrcScheduler denying against a zero-byte budget + NativeWithFallbackRuntime) instead of the existing synthetic delegate-based FallbackCoordinator tests, which only proved the coordinator's own branching logic.
Runs in every CI build, not just the opt-in hardware-gated lane: a real capacity denial happens in EnsureAdmitted strictly before any model file is opened, so no GGUF/hardware gate is needed for this specific invariant.
Corrects two stale spots in the spec's implementation-status banner and Phase B addendum status section (PR feat(runtime): context-aware VRAM cost estimate from GGUF headers #76's cost-estimate landing and PR feat(runtime): real load-time VRAM measurement from llama.cpp log lines #77's in-flight load-measurement work weren't reflected).

Test plan

dotnet build OrchestratorIDE.slnx — clean, 0 errors
dotnet test — 614 passed, 0 failed, 5 skipped (pre-existing hardware-gated lanes, unaffected by this change)
New test NativeRuntimePhaseDTests.RealAdmissionDenial_PropagatesThroughFallbackWrapper_WithoutInvokingFallback passes in isolation and in the full suite

🤖 Generated with Claude Code

Summary by CodeRabbit

Documentation
- Updated native runtime documentation to reflect current Phase B implementation and validation status.
- Clarified ongoing work for live VRAM measurement and documented successful GGUF lane validation.
- Added coverage details for native admission-denial behavior in Phase D.
Tests
- Added automated coverage confirming denied native runtime requests fail safely.
- Verified denied requests do not invoke the fallback runtime or callback.

… no Ollama fallback Native Runtime v2.0 Phase D (docs/NATIVE_RUNTIME_V2_SPEC.md §3.4) names an explicit requirement: a native-routed job with native prerequisites unavailable must fail closed with no silent Ollama substitution. Existing coverage only proved this via a synthetic FallbackCoordinator delegate that throws a manufactured exception — it verified the coordinator's own branching logic, not that a REAL admission denial from the REAL OrcScheduler actually surfaces through the REAL NativeWithFallbackRuntime wrapper without ever touching the fallback. Wires the real objects together instead: RuntimeOrchestrator + a real OrcScheduler making an actual TryAdmit denial against a zero-byte budget (not the simpler "no scheduler configured" case Phase A's own tests already cover) + NativeWithFallbackRuntime, and asserts RuntimeAdmissionDeniedException propagates with the fallback runtime never invoked. Needs no GGUF or hardware gate: a real capacity denial happens in EnsureAdmitted strictly before SessionManager ever opens a model file, so this specific Phase D requirement can run in every CI build rather than only the opt-in hardware-gated lane — landed early and stronger than the DoD strictly asked for. The rest of Phase D's full E2E lane (discovery through cancellation/telemetry on a real loaded model) remains open. Updated the spec's implementation-status banner and Phase B addendum status to reflect PR #76 (cost-estimate landed) and PR #77 (load-measurement, in flight) — both were stale before this edit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

coderabbitai · 2026-07-19T06:55:16Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: bc281fa4-e831-47d9-b96f-8b4f6557e749

📥 Commits

Reviewing files that changed from the base of the PR and between 0a31aa0 and d11c83f.

📒 Files selected for processing (2)

OrchestratorIDE.UnitTests/NativeRuntimePhaseDTests.cs
docs/NATIVE_RUNTIME_V2_SPEC.md

📝 Walkthrough

Walkthrough

Adds an always-on NUnit test verifying that real native admission denial propagates through NativeWithFallbackRuntime without invoking fallback execution, and updates the native runtime specification with Phase B and Phase D implementation status.

Changes

Native admission denial

Layer / File(s)	Summary
Admission denial propagation test `OrchestratorIDE.UnitTests/NativeRuntimePhaseDTests.cs`	Constructs a native runtime with a zero-budget scheduler and nonexistent model asset, then verifies denied admission, no fallback invocation, and no fallback callback.
Runtime specification status `docs/NATIVE_RUNTIME_V2_SPEC.md`	Updates Phase B status for cost estimation and measurement work, and documents the always-on Phase D admission-denial test.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

hardcoreerik/TheOrc#67: Established the native runtime v2.0 production-readiness specification that this PR updates.
hardcoreerik/TheOrc#70: Changed the admission path to throw RuntimeAdmissionDeniedException, which this PR tests through the fallback wrapper.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: a Phase D negative runtime test that fails closed without invoking fallback.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/native-runtime-phase-d-nofallback-proof

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

hardcoreerik · 2026-07-19T15:30:24Z

@coderabbitai full review

coderabbitai · 2026-07-19T15:30:32Z

✅ Action performed

Full review finished.

Your plan includes PR reviews subject to rate limits. More reviews will be available in 14 minutes.

hardcoreerik · 2026-07-19T23:02:58Z

@coderabbitai full review

coderabbitai · 2026-07-19T23:03:18Z

✅ Action performed

Full review finished.

…ifact (#81) * feat(runtime): Phase D real-model E2E lane with retained evidence artifact Native Runtime v2.0 Phase D's remaining DoD item: "the lane runs green on a real reference box and emits a retained evidence artifact." The negative fail-closed test (#78) and the cancellation leg (#79) both landed already; this closes the happy-path piece. NativeRuntimeE2ELaneTests drives discovery (ModelDepot.Scan) through a REAL admission decision -- a real OrcScheduler against a real live-queried NativeVramProbe budget, deliberately not the allowUnbudgetedExecution opt-out every other gated lane in this workstream uses -- through adapter lifecycle, real inference, and telemetry reads at three stages (before/mid-flight/after), writing a retained JSON evidence artifact regardless of pass or fail. NativeE2ELaneEvidenceStore follows the same schema_version/timestamp_utc/ app_version convention as the existing NativeRuntimeFallbackEvidenceStore and NativeRuntimeComparisonReportStore, under .orc/native-e2e-lane/ -- a sibling store, not a fork of the pattern, since this is the successful-run case those two don't cover (the fallback store only persists non-success outcomes). Verified on real hardware, 3 repeated runs: 25.7 tok/s, 284ms TTFT, 2.5GB measured VRAM. The captured telemetry surfaced a genuinely interesting, correct distinction: residency (ActiveCount) drops to 0 once the conversation disposes, but the VRAM reservation deliberately stays live with the loaded model rather than releasing -- confirming those two lifecycles are intentionally decoupled, not a leak. Full 637-test suite (including every gated real-model lane) stays green. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(runtime): address CodeRabbit findings on the E2E lane PR Three real, valid findings from the first genuine CodeRabbit pass under the new .coderabbit.yaml config: 1. No bound on the real streaming call -- a hardware stall would hang the test indefinitely instead of failing with retained evidence. Added a 2-minute CancellationTokenSource. 2. Evidence was written to %AppData% instead of the repo's .orc/native-e2e-lane/ convention every other evidence store in this workstream uses, because WriteAsync was never given a workspaceRoot. Added a FindRepoRoot() helper (walks up from the test binary looking for OrchestratorIDE.slnx) and passed it through. Verified: evidence now lands under the actual repo, confirmed on a real run. 3. The evidence filename used DateTime.Now while the record's own timestamp_utc field used UtcNow -- inconsistent, and could sort wrong across machine timezones. Now captures one DateTimeOffset.UtcNow and threads it through both. Also fixed a minor unit-spacing nitpick in the spec doc (284ms -> 284 ms, 2.5GB -> 2.5 GB). Skipped one suggestion: CodeRabbit's prompt asked the spec's Phase D banner to say the E2E lane "has landed" -- this PR isn't merged yet, so that would be the overclaiming this whole workstream has deliberately avoided. Left as "in flight (PR pending)," accurate as of this commit. Re-verified on real hardware: full 637-test suite green, E2E lane test passes with evidence now correctly retained in the repo. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(runtime): address ultrareview findings on the E2E lane PR Three real nits from the cloud multi-agent review, all verified against current code before fixing: 1. NativeE2ELaneRunResult dropped the exception TYPE on failure, only keeping ex.Message -- forking the sibling NativeRuntimeFallbackEvidenceStore pattern this store's own doc claims to follow (that one captures both ErrorType and ErrorMessage). A failed run's evidence couldn't distinguish an admission denial from a native load crash from a timeout. Added ErrorType, populated as ex.GetType().Name, threaded through to the JSON record. 2. Assert.Multiple unconditionally indexed snapshots[1]/[2] assuming 3 entries, but the mid-flight snapshot is only appended once a token is actually yielded -- a failure BEFORE the first token (an admission denial or native load failure, exactly the case this lane's real, non-opted-out admission is designed to exercise) leaves Count at 2, and the unconditional index would throw ArgumentOutOfRangeException, burying the real failure already surfaced via `success`/`errorMessage` under confusing noise. Guarded the count-dependent asserts behind an explicit count check. 3. Two doc-only contradictions in NATIVE_RUNTIME_V2_SPEC.md, both introduced across this PR and earlier commits: the Phase B addendum still said "PR #77, in flight" after #77 had actually merged; and this PR's own Phase D section said the E2E lane "landed" while the banner correctly said "in flight (PR pending)" -- fixed the stale #77 reference to "landed", and reworded the Phase D section to match the banner's honest "PR pending" framing rather than claim this PR landed before it's merged. Re-verified on real hardware: full 637-test suite green, E2E lane test passes with error_type now present (null on the successful run) in the retained evidence artifact. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

…endum + D (#82) * docs: sync ROADMAP/CURRENT_STATE with Native Runtime v2.0 Phase B addendum + D Both files still described Phase B's cost-estimate half as deliberately deferred and Phase D as untouched -- stale since PR #76 (cost estimate), #77 (load-time measurement), #78 (negative fail-closed test), and #79 (cancellation-corruption fix) all landed since the last sync. Updates the Native Runtime v2.0 direction table in ROADMAP.md and the native_runtime note in CURRENT_STATE.yaml to reflect current reality: Phase B fully landed (estimate + measurement), Phase D essentially complete pending the E2E evidence-lane PR (#81, open). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: mark E2E evidence lane landed (PR #81 merged), not pending CodeRabbit finding: ROADMAP.md's banner still said the E2E lane was "built, verified on real hardware, and pending merge" while CURRENT_STATE.yaml's parallel note omitted that qualifier entirely -- inconsistent, and now stale either way since PR #81 actually merged. Updated both to say landed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

hardcoreerik mentioned this pull request Jul 19, 2026

chore: disable CodeRabbit auto-incremental-review to reduce credit burn #80

Merged

hardcoreerik merged commit e671de0 into master Jul 20, 2026
2 checks passed

hardcoreerik deleted the feat/native-runtime-phase-d-nofallback-proof branch July 20, 2026 01:14

This was referenced Jul 20, 2026

feat(runtime): Phase D real-model E2E lane with retained evidence artifact #81

Merged

docs: sync ROADMAP/CURRENT_STATE with Native Runtime v2.0 Phase B addendum + D #82

Merged

coderabbitai Bot mentioned this pull request Jul 21, 2026

feat: HV-1 native campaign driver (Tools/Hv1NativeCampaignRunner) #87

Merged

3 tasks

coderabbitai Bot mentioned this pull request Aug 1, 2026

Native studio tools for CaseForge, Art Forge, and KeyHound #96

Merged

Navigation Menu

feat(runtime): Phase D negative test — admission denial fails closed, no Ollama fallback - #78

feat(runtime): Phase D negative test — admission denial fails closed, no Ollama fallback#78
hardcoreerik merged 1 commit into
masterfrom
feat/native-runtime-phase-d-nofallback-proof

hardcoreerik commented Jul 19, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

hardcoreerik commented Jul 19, 2026

Uh oh!

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

Uh oh!

hardcoreerik commented Jul 19, 2026

Uh oh!

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

hardcoreerik commented Jul 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

hardcoreerik commented Jul 19, 2026

Uh oh!

coderabbitai Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hardcoreerik commented Jul 19, 2026

Uh oh!

coderabbitai Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

hardcoreerik commented Jul 19, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading

coderabbitai Bot commented Jul 19, 2026 •

edited

Loading