Evaluate package-declared typed application results - #701
Conversation
|
@coderabbitai review |
|
@codex please review |
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughAdds compiler-owned result projections to executable operations, including contract admission, lowerer/verifier binding, scheduler evaluation, canonical typed results, receipt/WAL retention, recovery validation, and durable runner reporting. Provider manifests, schemas, digests, fixtures, tests, and documentation are updated accordingly. ChangesResult projection execution corridor
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20642dbf3c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@CHANGELOG.md`:
- Around line 21-23: Distinguish declared result limits from produced result
bytes across the three documentation sites: in CHANGELOG.md lines 21-23, replace
“compiler-declared result bytes” with “compiler-declared result ceiling”; in
README.md lines 290-291, state that the declared maximum result size is capped
before evaluation; and in
docs/adr/0025-scheduler-owned-executable-operation-actions.md lines 125-128,
clarify that the bound applies to the declared maximum while produced bytes are
checked during evaluation.
In `@crates/echo-edict-provider-lowerer/src/executable_operation.rs`:
- Around line 610-678: Carry the validated maxOutputBytes value through lowering
instead of discarding it. Store it as max_output_bytes on ResultProjectionPlan
in validate_result_projection, then emit that value as a dedicated field in
encode_package’s application_result_projection map so scheduler consumers can
enforce the bound without decoding artifact_bytes.
In `@crates/echo-edict-provider-verifier/src/executable_operation.rs`:
- Around line 684-689: Update the projection validation in the verifier and
lowerer around the required_u64 checks to reject maxOutputBytes values above
MAX_APPLICATION_RESULT_BYTES, matching validate_edict_result_projection. Also
reject configurations where nodeKeyField and replacementField resolve to the
same path, consistent with
EchoOperationApplicationResultProjectionV1::from_value, before accepting or
lowering the projection.
In `@crates/warp-core/src/echo_operation.rs`:
- Around line 6468-6480: Update hash_optional_application_result to emit the
same explicit absence/presence tag as the other optional hash helpers: write the
absence byte for None and retain the existing tagged identity and
canonical_bytes hashing for Some. Regenerate affected receipt and
terminal-outcome digest fixtures and retained WAL test evidence.
- Around line 1450-1472: Update same_projection_shape and the authored
EchoOperationResultExpressionShapeV1 representation to preserve source kind and
path instead of collapsing all sources to Source. Match ApplicationInputPath
only when its runtime path equals the authored applicationInput path, and
require CapabilityResult to match the lowered application_input_node_key_path
produced by the lowerer. Keep mismatched source kinds or paths invalid so the
runtime plan cannot bind to an unrelated projection identity.
In `@docs/topics/WAL.md`:
- Around line 340-341: Update the recovery wording in the WAL documentation to
say it “revalidates the retained evidence against the exact installed package
projection” rather than requiring evidence to reproduce under it. Preserve the
callback-free, evidence-based recovery contract and make clear that recovery
does not reevaluate the projection.
In `@tests/edict-provider-host-v1/tests/package_contract.rs`:
- Line 71: Add a direct identity assertion in the package contract test
alongside EXPECTED_SCHEMA_BINDING_COUNT, verifying the
edict.result-projection.artifact/v1 domain/root pair through the existing
schema-binding representation. Keep the count assertion unchanged so the test
validates both the total number of bindings and the presence of this specific
new binding.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7622eb9a-1734-4796-8f13-2a91ac7875c7
⛔ Files ignored due to path filters (17)
Cargo.lockis excluded by!**/*.lockcrates/echo-wesley-gen/assets/v1/edict-provider/package/v1/components/lowerer.echo-dpo.component.wasmis excluded by!**/*.wasmcrates/echo-wesley-gen/assets/v1/edict-provider/package/v1/components/verifier.echo-dpo.component.wasmis excluded by!**/*.wasmcrates/echo-wesley-gen/assets/v1/edict-provider/package/v1/generated/evidence/provenance.provider-generation.jsonis excluded by!**/generated/**crates/echo-wesley-gen/assets/v1/edict-provider/package/v1/generated/evidence/review.provider-generation.jsonis excluded by!**/generated/**crates/echo-wesley-gen/assets/v1/edict-provider/package/v1/generated/primary/schema.echo-provider-artifacts.cddlis excluded by!**/generated/**schemas/edict-provider/components/v1/lowerer.echo-dpo.component.wasmis excluded by!**/*.wasmschemas/edict-provider/components/v1/verifier.echo-dpo.component.wasmis excluded by!**/*.wasmschemas/edict-provider/generated/v1/evidence/provenance.provider-generation.jsonis excluded by!**/generated/**schemas/edict-provider/generated/v1/evidence/review.provider-generation.jsonis excluded by!**/generated/**schemas/edict-provider/generated/v1/primary/schema.echo-provider-artifacts.cddlis excluded by!**/generated/**schemas/edict-provider/package/v1/components/lowerer.echo-dpo.component.wasmis excluded by!**/*.wasmschemas/edict-provider/package/v1/components/verifier.echo-dpo.component.wasmis excluded by!**/*.wasmschemas/edict-provider/package/v1/generated/evidence/provenance.provider-generation.jsonis excluded by!**/generated/**schemas/edict-provider/package/v1/generated/evidence/review.provider-generation.jsonis excluded by!**/generated/**schemas/edict-provider/package/v1/generated/primary/schema.echo-provider-artifacts.cddlis excluded by!**/generated/**tests/edict-provider-host-v1/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (48)
CHANGELOG.mdREADME.mdcrates/echo-edict-provider-lowerer/README.mdcrates/echo-edict-provider-lowerer/src/executable_operation.rscrates/echo-edict-provider-lowerer/src/lib.rscrates/echo-edict-provider-lowerer/tests/executable_operation_package.rscrates/echo-edict-provider-lowerer/tests/fixtures/generated_echo_dpo.rscrates/echo-edict-provider-lowerer/tests/lowerer_contract.rscrates/echo-edict-provider-verifier/README.mdcrates/echo-edict-provider-verifier/src/executable_operation.rscrates/echo-edict-provider-verifier/tests/executable_operation_package.rscrates/echo-wesley-gen/README.mdcrates/echo-wesley-gen/assets/v1/edict-provider/contracts/v1/edict-provider-contracts.cddlcrates/echo-wesley-gen/assets/v1/edict-provider/contracts/v1/manifest.jsoncrates/echo-wesley-gen/assets/v1/edict-provider/echo-provider-semantics-v1.jsoncrates/echo-wesley-gen/assets/v1/edict-provider/package/v1/provider-manifest.echo.jsoncrates/echo-wesley-gen/assets/v1/repository/Cargo.lock.sourcecrates/echo-wesley-gen/src/provider_artifacts.rscrates/echo-wesley-gen/src/provider_contract_pack.rscrates/echo-wesley-gen/src/provider_package.rscrates/echo-wesley-gen/src/provider_semantics.rscrates/echo-wesley-gen/tests/provider_contract_pack.rscrates/echo-wesley-gen/tests/provider_package.rscrates/echo-wesley-gen/tests/provider_package_corpus.rscrates/echo-wesley-gen/tests/provider_semantic_source.rscrates/warp-core/Cargo.tomlcrates/warp-core/src/echo_operation.rscrates/warp-core/src/lib.rsdocs/adr/0025-scheduler-owned-executable-operation-actions.mddocs/architecture/application-contract-hosting.mddocs/spec/scheduler-warp-core.mddocs/topics/GeneratedRules.mddocs/topics/WAL.mdschemas/edict-provider/README.mdschemas/edict-provider/components/v1/README.mdschemas/edict-provider/contracts/v1/README.mdschemas/edict-provider/contracts/v1/edict-provider-contracts.cddlschemas/edict-provider/contracts/v1/manifest.jsonschemas/edict-provider/echo-provider-semantics-v1.jsonschemas/edict-provider/package/README.mdschemas/edict-provider/package/v1/provider-manifest.echo.jsontests/edict-provider-host-v1/tests/package_contract.rsxtask/src/provider_lowerer_component.rsxtask/src/run_edict_operation.rsxtask/tests/fixtures/edict-operation/ORIGIN.tomlxtask/tests/fixtures/edict-operation/executable-operation-package.cborxtask/tests/fixtures/edict-operation/verification-report.cborxtask/tests/run_edict_operation.rs
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 `@crates/echo-edict-provider-lowerer/src/executable_operation.rs`:
- Around line 540-550: Apply the existing projection-path text-byte bound to
both configuration-derived fields before constructing ProgramConfiguration:
reject oversized nodeKeyField and replacementField values in lowerer
executable_operation.rs#L540-L550, and apply the identical validation before
reconstructing the expected package in verifier
executable_operation.rs#L577-L587. Add boundary coverage for accepted and
rejected field-name lengths, preserving the existing empty-field and equality
checks.
In `@crates/warp-core/src/echo_operation.rs`:
- Around line 7266-7307: Update
runtime_projection_cannot_rebind_authored_source_kind_or_path to capture the
projection_test_package error and assert its refusal detail explicitly
identifies the authored/runtime source shape mismatch, matching the established
detail assertion used by the adjacent preflight test. Keep both cases and their
existing error expectation intact while preventing unrelated validation failures
from satisfying the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9b57b396-190d-4a67-959b-0aaae1e4cc87
⛔ Files ignored due to path filters (6)
crates/echo-wesley-gen/assets/v1/edict-provider/package/v1/components/lowerer.echo-dpo.component.wasmis excluded by!**/*.wasmcrates/echo-wesley-gen/assets/v1/edict-provider/package/v1/components/verifier.echo-dpo.component.wasmis excluded by!**/*.wasmschemas/edict-provider/components/v1/lowerer.echo-dpo.component.wasmis excluded by!**/*.wasmschemas/edict-provider/components/v1/verifier.echo-dpo.component.wasmis excluded by!**/*.wasmschemas/edict-provider/package/v1/components/lowerer.echo-dpo.component.wasmis excluded by!**/*.wasmschemas/edict-provider/package/v1/components/verifier.echo-dpo.component.wasmis excluded by!**/*.wasm
📒 Files selected for processing (20)
CHANGELOG.mdREADME.mdcrates/echo-edict-provider-lowerer/README.mdcrates/echo-edict-provider-lowerer/src/executable_operation.rscrates/echo-edict-provider-lowerer/tests/executable_operation_package.rscrates/echo-edict-provider-verifier/README.mdcrates/echo-edict-provider-verifier/src/executable_operation.rscrates/echo-edict-provider-verifier/tests/executable_operation_package.rscrates/echo-wesley-gen/assets/v1/edict-provider/package/v1/provider-manifest.echo.jsoncrates/echo-wesley-gen/tests/provider_package.rscrates/echo-wesley-gen/tests/provider_package_corpus.rscrates/warp-core/src/echo_operation.rscrates/warp-core/src/trusted_runtime_host.rsdocs/adr/0025-scheduler-owned-executable-operation-actions.mddocs/topics/WAL.mdschemas/edict-provider/README.mdschemas/edict-provider/components/v1/README.mdschemas/edict-provider/package/v1/provider-manifest.echo.jsontests/edict-provider-host-v1/tests/package_contract.rsxtask/src/provider_lowerer_component.rs
|
Merge gate open at 671124e: all current-head checks pass, zero bot-authored review threads remain unresolved, and the local affected suites, generators, frozen-host contract, linters, and pre-push gate are clean. The repository is solo-maintained by flyingrobots and main is not branch-protected, so criteria 1–4 substitute for a separate human approval. The standalone Hello Echo build and direct generic runner are green; hello-echo#18 owns the intentionally deferred consumer artifact-pin update. |
Admits compiler-declared result projections, evaluates them during scheduler-owned Tick construction, and retains exact canonical typed results through outcomes, Receipts, WAL recovery, and the schema-neutral runner report. Refreshes the independently reproduced generic provider components and checked package closure.
Validation: lowerer 12/12; verifier 8/8; warp-core 685/685; executable-operation integration 38/38; runner 9/9; provider corpus 82/82; frozen provider host green; affected Clippy lanes and pre-push hooks green. Standalone Hello Echo build and direct durable runner are green; its published matrix remains RED only on intentionally pinned predecessor artifact identities owned by hello-echo#18.
Closes #698.