Skip to content

review(2/3): runtime contracts, diagnostics and workflow qualification - #46

Draft
MrScripty wants to merge 25 commits into
review/quality-part1-2026-10-04from
review/quality-part2-2026-10-04
Draft

MrScripty wants to merge 25 commits into
review/quality-part1-2026-10-04from
review/quality-part2-2026-10-04

Conversation

@MrScripty

@MrScripty MrScripty commented Oct 4, 2026 •

Copy link
Copy Markdown
Owner

Review-only slice 2 of 3 for combined integration #44. Do not merge this PR; immutable aliases preserve existing source history without code changes. Integration and combined CI remain owned by #44.

Exact range: 8a08b6a -> 2a4d5c8. Changed tracked paths: 88. The three consecutive ranges (84/88/77 paths) have union exactly all 225 changed paths of main4938e405 -> combined99b8fb38; no source paths are omitted. Part3 includes the ancestry-preserving PR21/22 composition. These review surfaces are necessary because CodeRabbit refused the 223 eligible-file combined PR under its100-file limit.

Review the complete source changes in this bounded range, including interactions visible in the final tree, relevant error/lifecycle/contract/CI behavior, and tests. Reviews are supporting evidence and do not independently authorize merge. Findings will be reconciled against the combined candidate and repaired there, with affected external coverage refreshed as necessary.

CodeRabbit defaults exclude Cargo.lock and package-lock.json; record actual ignored files honestly. The integration owner separately examines generated lockfile deltas with dependency resolver/audit/build evidence. No new exclusions or relaxed gates are introduced.

Combined local qualification passes 547 frontend tests, production audit zero, full lint/typecheck/build and main-relative lint:no-new. Hosted combined qualification runs at #44 head99b8fb389c1a59ae63d42c633ca099424664e267, treecc71bdd23ce923b58207fdaf88e477090e6e7e45; intermediate slices may retain known later-repaired CI failures. Real-model GUI/release acceptance remains unqualified.

Summary by CodeRabbit

  • API Updates
    • Introduced named request types for PyTorch text generation and media conversion, and updated inference settings and scheduler APIs to use grouped inputs.
    • Updated several Rust API types and error results to reduce inline data size; serialized formats and existing behavior remain unchanged.
  • Bug Fixes
    • Missing model references no longer produce an invalid-reference diagnostic.
  • Tests
    • Expanded regression coverage for inference, diagnostics, serialization, workflow execution, and runtime lifecycle behavior.
  • Documentation
    • Added technical reports and changelog notes describing API updates and compatibility considerations.

@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 8f7db976-7888-416a-8879-f40b8af73e56
📥 Commits

Reviewing files that changed from the base of the PR and between 8a08b6a and 2a4d5c8.

📒 Files selected for processing (88)
  • .github/workflows/quality-gates.yml
  • CHANGELOG.md
  • crates/inference/src/backend/compatibility.rs
  • crates/inference/src/backend/llamacpp.rs
  • crates/inference/src/backend/mod.rs
  • crates/inference/src/backend/pytorch.rs
  • crates/inference/src/backend/pytorch_tests.rs
  • crates/inference/src/backend/pytorch_worker_image_contract_tests.rs
  • crates/inference/src/execution_telemetry.rs
  • crates/inference/src/gateway.rs
  • crates/inference/src/gateway_tests.rs
  • crates/inference/src/image_generation_planner.rs
  • crates/inference/src/image_generation_planner_tests.rs
  • crates/inference/src/lib.rs
  • crates/inference/src/managed_binaries.rs
  • crates/inference/src/managed_runtime/contracts.rs
  • crates/inference/src/model_contracts.rs
  • crates/inference/src/server.rs
  • crates/inference/src/server_tests.rs
  • crates/inference/src/types.rs
  • crates/inference/tests/model_contracts.rs
  • crates/node-engine/src/core_executor.rs
  • crates/node-engine/src/core_executor/kv_cache_test_support.rs
  • crates/pantograph-diagnostics-ledger/src/event.rs
  • crates/pantograph-diagnostics-ledger/src/sqlite/event_sqlite.rs
  • crates/pantograph-diagnostics-ledger/src/tests.rs
  • crates/pantograph-embedded-runtime/src/node_execution_ledger.rs
  • crates/pantograph-embedded-runtime/src/runtime_dispatch_candidate_provider.rs
  • crates/pantograph-embedded-runtime/src/runtime_host_execution_port.rs
  • crates/pantograph-media-conversion/src/lib.rs
  • crates/pantograph-runtime-host-contracts/src/reservation_lifecycle.rs
  • crates/pantograph-runtime-host-contracts/src/runtime_host_execution.rs
  • crates/pantograph-runtime-host-contracts/src/runtime_session_load.rs
  • crates/pantograph-runtime-host-contracts/tests/as_ref_compatibility.rs
  • crates/pantograph-runtime-registry/src/runtime_selection_policy.rs
  • crates/pantograph-runtime-registry/src/technical_fit_tests.rs
  • crates/pantograph-scheduler/src/queue.rs
  • crates/pantograph-scheduler/tests/queue_state.rs
  • crates/pantograph-workflow-service/src/graph/connection_intent.rs
  • crates/pantograph-workflow-service/src/graph/inference_interface_request.rs
  • crates/pantograph-workflow-service/src/graph/inference_validation_state.rs
  • crates/pantograph-workflow-service/src/graph/inference_validation_task_owner.rs
  • crates/pantograph-workflow-service/src/graph/session_inference_validation_api.rs
  • crates/pantograph-workflow-service/src/scheduler/store_task_results.rs
  • crates/pantograph-workflow-service/src/scheduler/store_tests.rs
  • crates/pantograph-workflow-service/src/scheduler/task_orchestrator.rs
  • crates/pantograph-workflow-service/src/scheduler/task_orchestrator_tests.rs
  • crates/pantograph-workflow-service/src/workflow/attribution_api.rs
  • crates/pantograph-workflow-service/src/workflow/contracts.rs
  • crates/pantograph-workflow-service/src/workflow/diagnostics_api.rs
  • crates/pantograph-workflow-service/src/workflow/executable_validation_snapshot.rs
  • crates/pantograph-workflow-service/src/workflow/runtime_branch_batch_execution.rs
  • crates/pantograph-workflow-service/src/workflow/runtime_branch_run_finalization.rs
  • crates/pantograph-workflow-service/src/workflow/session_execution_api.rs
  • crates/pantograph-workflow-service/src/workflow/session_queue_api.rs
  • crates/pantograph-workflow-service/src/workflow/session_runtime.rs
  • crates/pantograph-workflow-service/src/workflow/session_scheduler_runner.rs
  • crates/pantograph-workflow-service/src/workflow/task_execution_owner.rs
  • crates/pantograph-workflow-service/src/workflow/task_execution_worker.rs
  • crates/pantograph-workflow-service/src/workflow/task_graph.rs
  • crates/pantograph-workflow-service/src/workflow/task_run_summary.rs
  • crates/pantograph-workflow-service/src/workflow/tests/diagnostics.rs
  • crates/pantograph-workflow-service/src/workflow/tests/task_binding_resolution.rs
  • crates/pantograph-workflow-service/src/workflow/tests/task_graph.rs
  • crates/pantograph-workflow-service/src/workflow/tests/task_state_read_model.rs
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-gateway-private-contexts.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-inference-error-boundaries.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-inference-lint-basics.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-inference-payload-layout.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-ledger-artifact-payload-layout.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-ledger-inference-payload-layout.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-ledger-private-projection-input.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-llama-settings-input.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-media-result-input.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-node-validation-context.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-orchestrator-selection-error-layout.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-pinned-rustfmt-correction.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-pytorch-text-generation-request.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-pytorch-trust-policy-docs.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-ready-inference-projection-layout.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-runtime-host-asref.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-runtime-intent-layout.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-runtime-registry-diagnostic-errors.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-session-run-context.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-terminal-diagnostic-input.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-workflow-lint-basics.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-workflow-private-payload-layout.md
  • docs/plans/domain-architecture-and-multimodal/reports/2026-10-03-workflow-private-type-names.md
💤 Files with no reviewable changes (3)
  • crates/inference/src/execution_telemetry.rs
  • crates/pantograph-workflow-service/src/workflow/session_queue_api.rs
  • crates/inference/src/types.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

This pull request updates Rust APIs and payload layouts across inference, media conversion, diagnostics, runtime hosting, and workflow execution. It groups several multi-argument calls into structs, boxes selected payloads, adjusts workflow graph handling, and adds targeted tests and CI discovery checks.

Changes

Rust API and inference inputs

Layer / File(s) Summary
Inference and media input APIs
crates/inference/src/backend/*, crates/inference/src/server*, crates/pantograph-media-conversion/src/lib.rs, CHANGELOG.md, docs/plans/domain-architecture-and-multimodal/reports/*
PyTorch text generation accepts PyTorchTextGenerationRequest; Llama server methods accept LlamaCppRuntimeSettings; and MediaConversionResult::try_new accepts MediaConversionResultInput. The associated callers and tests use these input types.
Inference runtime and lifecycle handling
crates/inference/src/gateway*, crates/inference/src/image_generation_planner*, crates/inference/src/managed_binaries.rs, crates/inference/src/managed_runtime/contracts.rs, crates/inference/src/model_contracts.rs, crates/inference/src/types.rs, crates/inference/src/execution_telemetry.rs
Gateway helpers receive grouped context values. Selected error and plan payloads are boxed, managed-binary and task-registry errors return boxed values, and lifecycle builder methods no longer carry #[must_use]. Tests cover contexts, serialized errors, and plan layout.
Diagnostic payload and error layouts
crates/pantograph-diagnostics-ledger/*, crates/pantograph-embedded-runtime/src/node_execution_ledger.rs, crates/pantograph-runtime-registry/src/*, docs/plans/domain-architecture-and-multimodal/reports/*
Ledger event payloads and selected runtime-registry diagnostic errors are boxed. Projection failure writing uses the shared state-writing path, and timeline details filter out zero counts. Tests check serialized payloads, persisted failure state, and diagnostic values.
Scheduler intent and projection layouts
crates/pantograph-scheduler/*, crates/pantograph-workflow-service/src/graph/inference_validation_state.rs, crates/pantograph-workflow-service/src/workflow/{task_graph.rs,executable_validation_snapshot.rs,task_execution_worker.rs}, crates/pantograph-workflow-service/src/scheduler/task_orchestrator*
Runtime task intents use a boxed payload and a runtime constructor. Ready projections, environment requests, completed worker outcomes, and no-selection decisions also use boxed payloads. Related callers and layout, serialization, and decision tests are updated.
Workflow execution contexts and terminal events
crates/node-engine/src/core_executor.rs, crates/pantograph-workflow-service/src/workflow/{runtime_branch_*,session_execution_api.rs,session_scheduler_runner.rs,task_execution_owner.rs}, docs/plans/domain-architecture-and-multimodal/reports/*
Workflow execution and terminal diagnostic recording pass grouped context/input structs. Existing transition details and execution paths are retained in the updated call sites.
Workflow graph and contract updates
crates/pantograph-workflow-service/src/graph/*, crates/pantograph-workflow-service/src/workflow/{contracts.rs,task_graph.rs,task_run_summary.rs}, docs/plans/domain-architecture-and-multimodal/reports/*
Graph handling updates default implementations, model-reference parsing, fingerprint arguments, finished-task filtering, and dependency ID ordering. Task-summary error variants are renamed while their messages and detection conditions remain unchanged.
Runtime-host accessors and test discovery
crates/pantograph-runtime-host-contracts/*, .github/workflows/quality-gates.yml, CHANGELOG.md, docs/plans/domain-architecture-and-multimodal/reports/*
Validated wrappers provide standard AsRef implementations, with compile-time call-form tests. The Rust quality-gates job adds focused test commands and checks that selected tests are discovered before execution.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~60 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 2a4d5

This is a review-only slice that groups Rust API inputs and boxes selected payloads while keeping serialized formats and runtime decisions the same. No concrete merge-blocking issue was found. The PR author states it is not intended to be merged, and combined CI is owned by the integration PR.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 2a4d5

The examined changes primarily reshape calling conventions and payload storage. Existing validation, execution ownership, and identity checks appear preserved; no introduced security regression was established. Incomplete downstream and recovery coverage prevents a complete security clearance.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The examined changes affect existing inference execution, scheduler task/reservation handling, and persisted diagnostics. Their shared contracts warrant downstream compatibility review, but supplied deployment and authentication context does not establish the maximum independently attackable tenant, service, or environment scope.

Trust Boundaries and Controls

  • observed — Boxing a runtime intent does not itself grant execution authority. The inspected queue consumer still validates the contained intent and correlates workflow, run, node, and task identifiers before accepting it for a task.
  • observed — Runtime batch execution retains owned-running-event and batch-claim checks before dispatch and validates response membership before publication. Terminal recording continues to derive task and attempt identity from the started batch member, preserving the existing ownership handoff in the examined change.

Resilience and Maintainability Implications

  • inferred — Terminal-input grouping preserves completed, failed, and cancelled transition metadata and the existing cleanup ordering. Inspected tests encode rejection of invalid response members before publication and rejection of duplicate publication. These controls do not establish whole-batch atomic rollback or comprehensive crash recovery; the sequential mutation limitation predates the examined refactor and is not retained as an introduced concern.
🚥 Pre-merge checks | ✅ 4 | ❓ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ❓ Inconclusive Docstring coverage is 45.15% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 206 functions across 50 files. (35 skippe… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies this review slice’s focus on runtime contracts, diagnostics, and workflow qualification, which matches the changes.
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 45.15% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 206 functions across 50 files. (35 skipped: 25 unsupported, 10 over the file limit.)

✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Copy link
Copy Markdown
Owner Author

@coderabbitai review

Review the full bounded source range 8a08b6a -> 2a4d5c8 for combined candidate #44. This is review slice 2/3; all three ranges together cover all 225 combined changed paths. Please report substantive issues with lifecycle, error propagation, contracts, security and CI. Keep any ignored/generated-file coverage limitations visible. Do not alter code or merge.

@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Owner Author

@coderabbitai review

Retry after the reported included-review reset, using the included/free OSS allowance only. Exact unchanged range: 8a08b6a -> 2a4d5c8, 88 tracked files. This range has not received an external review yet. Review the complete bounded source changes for combined PR44, retaining visible coverage limitations. This PR is review-only; do not modify source or merge.

@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant