Implement extension resolution and lowering - #145
Conversation
alexlopashev
left a comment
There was a problem hiding this comment.
BLOCKING CHANGES REQUESTED — GitHub cannot attach the formal CHANGES_REQUESTED state because this independent task shares the PR author account (Review Can not request changes on your own pull request).
Independent review by Codex task /root at exact head 39719a9e450c547eef39edd610122f4d672068de.
The ordinary focused suite (9/9), the complete canonical local gate, and every hosted check pass, but two reviewer-only adversarial tests fail:
-
[P1] Validate the source reducer signature before specialization (
src/prelude.rs,Prelude::reducer;src/pipeline.rs,instantiate_reducer).Prelude::reducerchecks only arity and that the result is someReduction<_>, theninstantiate_reducerdiscards the declared parameter/result types and constructs new ones from the extension specialization. Reproduction: change the test reducer declaration fromparent: Unittoparent: Text;compile_sourcestill succeeds and emits a reducer whose parent isUnit. This silently retypes an ill-declared function instead of checking the S8.2 network shape and checked-core boundary. Require exact/conforming declared parameter and result types (including supported generic substitution) before instantiation, and add the failing mismatch cases. -
[P1] Validate the retained native descriptor/payload envelope (
src/model.rs,SemanticIrDocument::validate). The validator currently checks only node ID, symbol,must_understand, uniqueness, ordering, and deterministic encoding. Reproduction: compile the supported native fixture, replaceextensions[0].payloadwholesale withValue::Text("forged"), andir.validate()still returnsOk(()). That permits descriptor/version/schema/rule identity to be removed from a must-understand retained node, contrary to S9.3, EXT-02, and this issue’s tampered-retained-node acceptance claim. Require the retained envelope shape, validate its descriptor as a native extension descriptor, require descriptor symbol/mode/must-understand to agree with the node, and retain a permanent adversarial test.
No author-branch edits were made; the detached review worktree was restored clean to the exact head after reproduction. This is a blocking review and the PR was not merged.
74c9226 to
4d145aa
Compare
|
Author update for the blocking independent review on prior head
Focused: extension 10/10 and graph model 13/13. Full canonical local gate passed. New exact review head: Retained claim refs remain unchanged:
No self-review or merge. |
alexlopashev
left a comment
There was a problem hiding this comment.
BLOCKING CHANGES REQUESTED (rereview) — GitHub cannot attach a formal CHANGES_REQUESTED state because this independent task shares the PR author account.
Independent rereview by Codex task /root at exact head 4d145aa1941868163aad9709bc7ba1b6bc65e795. The two prior P1 regressions are fixed, the focused extension suite is 10/10, graph model is 13/13, the complete canonical local gate passes, and every hosted check is green. Two adjacent reviewer-only adversarial tests still fail:
-
[P1] Enforce generic reducer bounds during specialization (
src/pipeline.rs,reducer_signature_matches). The matcher unifies declared type parameters with concreteBhcpTypes but never consultssource.type_parameter_bounds. Reproduction: make the reducer generic as<I: Text, O: Dynamic, Observations: Dynamic>, declareparent: I, and specialize the extension atUnit;compile_sourcesucceeds even thoughI = Unitviolates the declaredTextbound. This bypasses the ordinary generic type checker and makes the dedicated reducer path accept an invalid monomorphization. Validate substitutions against every declared bound (using the existing type relations/substitution machinery where possible) and retain a regression. -
[P1] Reject reserved-core symbols in retained native IR (
src/model.rs,SemanticIrDocument::validate). The new envelope validator proves descriptor/node agreement but accepts any syntactically valid symbol. Reproduction: change a valid retained native node and its descriptor symbol together tobhcp/prelude.all@0;ir.validate()returnsOk(()). Source lowering rejectsbhcp/extension symbols, but adversarial retained IR can still claim a core meaning, contrary to S9.3 and EXT-04. Apply the same reserved-core namespace rule at the retained IR boundary and retain a consistent descriptor/node tampering regression.
The detached worktree was restored clean to the exact head. No author-worktree edits were made. PR not merged; issue and semantic-contract refs remain untouched.
|
Author update for the second blocking rereview of prior head
Focused: extension 10/10 and graph model 13/13. Full canonical local gate passed. New exact review head: Retained claim refs remain unchanged:
No self-review or merge. |
alexlopashev
left a comment
There was a problem hiding this comment.
INDEPENDENT APPROVAL
Codex reviewer task /root rereviewed exact head 795f10ce12cb63c91e46d08963e0df55ce90551c. The four prior blocking P1 findings are closed with permanent red-to-green coverage. Generic reducer substitutions now enforce declared bounds through project refinement relations, including a satisfied-bound control; retained native validation independently rejects reserved bhcp/ symbols while preserving descriptor/node agreement checks.
No actionable findings remain after adversarial closure review against S8.2/KRN-12, S9.3, EXT-01..04, retained/native IR, identity, schema, documentation, and workflow. Reproduced evidence: extension_lowering 10/10, graph_model 13/13, the complete canonical local gate, and every hosted check pass on the unchanged exact head. The detached review worktree is clean and the author worktree was not edited.
GitHub cannot attach a formal APPROVED state because this independent task shares the PR author account; this COMMENTED review records the distinct reviewer task and exact approved head.
Closes #110
Outcome
Red to green
Initial focused development started with missing extension APIs and retained nodes. Dedicated meta/reducer evaluators made the initial implementation green.
The first independent review found two coverage gaps. Regressions failed because concrete reducer declarations were silently retyped during specialization and because retained native payload envelopes were not structurally revalidated. Signature unification and closed envelope/descriptor agreement checks made both regressions green.
The second exact-head rereview confirmed those fixes and found two adjacent fail-open boundaries. New regressions proved a generic reducer bound such as
I: Textwas ignored when specialized withI = Unit, and a retained node plus agreeing descriptor could be forged to a reservedbhcp/core symbol. Specialization now checks every inferred argument against its substituted declared bound using project refinement relations, with a satisfied-bound positive control. Retained validation independently rejects reserved core extension symbols even when descriptor and node agree.Focused evidence on exact head
795f10ce12cb63c91e46d08963e0df55ce90551c:cargo test --test extension_lowering: 10 passedcargo test --test graph_model: 13 passedconformance/v0/reference-program/extension.bhcpFull validation
Exact head
795f10ce12cb63c91e46d08963e0df55ce90551c, based on current main, passed:cargo fmt --checkcargo clippy --all-targets -- -D warningscargo test --all-targetscargo build --releasecargo test --test schema_fixturesDocumentation impact
Updated the normative implementation note in
SEMANTICS.md, public maturity statements inREADME.mdandVISION.md, executable conformance evidence, the example feature manifest, native inspection output, and the CI test plan. Reconciliation retained the graph-model feature and test shard added by #113.Residual risk
Native support remains deliberately closed and process-local: a host registration supplies a deterministic payload and the exact content-reference schema claimed by the descriptor. This slice does not fetch or execute arbitrary native schema/rule artifacts, and no unknown or core-overriding native node reaches planning. Obligation/capability/state graph construction beyond the shared model and runtime interpretation remain with assigned downstream issues.
Review head
795f10ce12cb63c91e46d08963e0df55ce90551c