Skip to content

feat(neural): registration-seam delta — typed arity/arg_sorts validated against the program, opaque artifact identity, queryable network_metadata - #160

Merged
levi770 merged 6 commits into
mainfrom
feat/network-registration-metadata
Jul 20, 2026
Merged

feat(neural): registration-seam delta — typed arity/arg_sorts validated against the program, opaque artifact identity, queryable network_metadata#160
levi770 merged 6 commits into
mainfrom
feat/network-registration-metadata

Conversation

@niveousdragon

Copy link
Copy Markdown
Contributor

The problem

Ask #2 of the #155 seam-mapping — the last piece the consumer's production bridge needs end-to-end: the network registration seat carries no typed signature (arity, arg_sorts) and no artifact identity (artifact_hash), so the consumer's registration-mapping validates adapter-side against name conventions instead of against the engine, and the identity law ("a retrained network is a NEW rule") is adapter-asserted rather than upstream-anchored.

The slice this branch implements

Converged with the consumer's published field-level contract (#155, law-reviewed on their side) before this PR opened:

  • register_network(..., *, arity=None, arg_sorts=None, artifact_hash=None) — the three bridge kwargs are keyword-only (matching the consumer's registration signature). Declared arity is validated against the program's own nn declarations: every NeuralPredicateInfo backed by the network must agree (predicate_arity), mismatch is a typed refusal naming the predicate and both arities — the seat validates against the program, not the adapter (the consumer's structural len(arg_sorts) == arity check composes on top; both layers kept per their recommendation).
  • arg_sorts are catalog sort ids: ints, with the bool trap. Elements are checked isinstance(elem, bool) before any int extraction — pyo3 silently coerces True → 1, so ordering is the trap (the same isinstance(True, int) law both sides already enforce elsewhere). A bool or non-int element is a typed refusal naming the index. Stored as Vec<i64>; length must equal arity; arg_sorts without arity is refused.
  • artifact_hash — a single opaque identity anchor, stored and returned untouched, never interpreted (converged: the consumer's seam-side Mapping reduces to one hash at this boundary). Re-registration with a new hash is the legal retrain path of their identity law, deliberately not refused.
  • network_metadata(name) — the queryable registry: {arity, arg_sorts, artifact_hash, declared: [{predicate, predicate_arity, input_arity, labels}]}, so the consumer's registration-mapping verifies against the real declarations. Three honest failure modes: undeclared; embedding-declared (names register_embedding — embeddings carry no registration metadata by design); declared-but-unregistered.
  • Metadata flows write-to-read end to end (NetworkConfigNetworkHandle → getter — the handle copy was a mid-branch review catch: the fields were silently write-only), the Python lineage wrapper threads the kwargs keyword-only (another mid-branch catch: it would have TypeErrord before Rust), and the type stubs cover the new surface.

Verified

  • A40 GPU: TestRegistrationMetadata 23/23 passed on this exact head — roundtrip, legacy byte-compat, arity-vs-declaration refusal, bool-trap, string-element refusal, keyword-only enforcement, all three getter failure modes. Full engine-mode + identifiability suites: 62 passed on the branch (registration-only commits since).
  • Rust units: cargo test -p xlog-neural 25/25 (defaults, clone/from_config retention); cargo check -p pyxlog clean; fmt clean.
  • The OR byte-invariance probe promised in the feat(ilp): witness-level mask/abstain channel per the #155 field contract #157 review thread is included: perturbing a masked row 0.01→0.99 leaves the noisy-OR bitwise equal (torch.equal), while an active row's change moves it — physical absence from the index, pinned.
  • Final whole-branch review: ready to merge, zero findings. Backward compat is contract: every new kwarg defaults to None with byte-identical legacy behavior through both the wrapper and the native path.
  • Known environmental note: cargo test --workspace on a fresh-rustup pod hit an LLVM/lld linker crash in the unrelated xlog-integration e2e binary (toolchain bug in rust 1.97.1's bundled LLVM, "please submit a bug report to llvm-project") — not reproducible against this branch's crates; our crates' tests pass.

Notes for review

With this, all three #155 asks are delivered: mask channel (#157), frozen entry (#156), registration seam (this PR) — the consumer's loop "register typed → enumerate with sort-matched slots → frozen_select with the mask channel" closes against a real registered surface.

…ated against the declarations, sorts and artifact hash carried opaquely
Add CompiledProgram.network_metadata(name), modeled on
neural_predicate_info: returns the registration-time arity/arg_sorts/
artifact_hash plus every nn/4 declaration bound to the network name
(predicate, predicate_arity, input_arity, labels), so a consumer can
check its own idea of a network's shape against the program's actual
declarations rather than a naming convention. Declared-but-unregistered
and undeclared names get typed PyValueErrors.

This metadata was previously write-only: register_network stored it on
NetworkConfig, but NetworkHandle::from_config never copied it over, so
it was dropped the instant registration completed. Extend NetworkHandle
with the same three fields, populated in from_config, so there is
something to read back.
…-declared networks

An embedding-declared network registers via register_embedding(), not
register_network(); calling register_network() on it hits the existing
cross-registration guard. But network_metadata's "not registered" error
told the caller to "call register_network() first" regardless — wrong
for embeddings, misdirecting them into a call that will always fail.

Branch on declared_network_forms (the same map the register_network
guard at line 218 already consults) before the generic "not registered"
error: if the name is embedding-declared, say so explicitly and name
register_embedding() as the actual registration path, and state that
arity/arg_sorts/artifact_hash metadata is out of scope for embeddings
by design. Keep the existing message for the genuine
classification-declared-but-unregistered case. Doc comment now lists
all three failure modes: undeclared / embedding-declared /
declared-but-unregistered.
…ls, getter boundaries, OR byte-invariance probe

TestRegistrationMetadata in test_network_registry.py (CUDA-gated, class
skipif since the file has no prior CUDA gate) covers register_network's
arity/arg_sorts/artifact_hash kwargs and the network_metadata() getter:
roundtrip, byte-compat with the legacy no-kwargs call, the three typed
refusals (arity vs. declaration, arg_sorts without arity, length
mismatch), and the getter's three failure modes (undeclared, declared-
but-unregistered, embedding-declared).

Writing these tests surfaced that _compiled_program_register_network_with_lineage
in crates/pyxlog/python/pyxlog/__init__.py (the nn4-lineage monkeypatch
installed over CompiledProgram.register_network) still had the pre-Task-1
fixed signature, with no path for arity/arg_sorts/artifact_hash to reach
the underlying native call — every public register_network() call
carrying them would TypeError before reaching the Rust validation this
surface exists to test. Threaded the three kwargs through positionally,
ahead of the wrapper's own keyword-only lineage params.

test_neural_credit.py gets one CPU test,
test_masked_row_probability_is_byte_invariant_in_the_or, the byte-
invariance probe promised to the external reviewer in #157: two p_event
vectors differing only at a masked witness row (0.01 vs 0.99) produce
bitwise-identical noisy_or_from_index results, with a sanity check that
changing an active row does change the OR.
…tract — int sort ids with the bool trap, keyword-only bridge kwargs

The consumer's published contract (#155) requires arg_sorts to be catalog
sort ids (tuple[int, ...]), not sort names, with bool explicitly excluded
even though isinstance(True, int) holds in Python; and requires arity,
arg_sorts, and artifact_hash to be keyword-only, matching
register_network(..., cache_size, *, arity, arg_sorts, artifact_hash).

- pyxlog::neural::register_network now accepts arg_sorts as a Python
  sequence (Vec<PyObject>), validating each element is an int and not a
  bool before extracting to i64 -- the bool check runs first since pyo3
  extracts bool into i64 silently otherwise.
- NetworkConfig::arg_sorts and NetworkHandle::arg_sorts change from
  Option<Vec<String>> to Option<Vec<i64>>; network_metadata returns ints.
- The three kwargs move behind `*` in the pyo3 signature; the __init__.py
  nn4-lineage wrapper mirrors this and forwards them as keywords instead
  of positionally (its old positional forward would have broken once the
  native signature went keyword-only).
- Rust unit tests in registry.rs/handle.rs and the Python tests in
  TestRegistrationMetadata are updated to int sort ids, with new tests for
  the bool trap, a non-int element, and keyword-only enforcement.
register_network gains the keyword-only bridge kwargs and network_metadata
is declared -- the stub was stale since before the seam delta.
@levi770

levi770 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

@niveousdragon

Engine/seam deep validation (xlog-side / DTS consumer seat) — PASS

Reviewed the diff against our published field-level contract (#155) and our landed consumer adapter. The registration-seam mechanics are correct and the convergence with the contract is faithful. Verified specifics:

register_network signature + validation (neural.rs)

  • Three bridge kwargs (arity, arg_sorts, artifact_hash) are keyword-only via *, — matches the consumer signature exactly.
  • arg_sorts supplied without arity is refused (the sorts name the arguments) — correct pairing.
  • len(arg_sorts) == arity enforced when both are present.
  • bool-trap handled at the right layer: is_instance_of::<PyBool>() runs before the i64 extraction. This ordering is the crucial part — pyo3 silently extracts True → 1, so a bool sort id must be rejected pre-extraction, not inferred from the extracted value. Our consumer adapter mirrors the same exclusion at the Python isinstance boundary; catching it at the pyo3 boundary as well is the stronger placement.
  • non-int arg_sorts elements refused with a typed error.

arity validated against the program (the two-layer check we recommended): predicate_arity != declared_arity is refused against the program's own nn/4 declaration, not merely the structural len. This is strictly stronger than the consumer-side structural check and composes with it — exactly the "arity as a consumer declaration commitment, verified engine-side" framing from our contract note.

Identity / artifact_hash — compatible with our contingent-registrant law: stored opaquely and singular; re-registration with a new hash is the retrain path (new hash ⇒ new identity). This is precisely the substrate our consumer-side contingent-registrant policy rides on — we supply whatever hash the adjudicated promotion state dictates and the engine treats each as a distinct opaque identity without inspecting it. No conflict.

Duplicate-name refusal — border correctly placed: the engine permits re-registration (retrain); duplicate refusal is intentionally left to our single-registry adapter, as our published contract stated. A raw-engine consumer gets retrain semantics; our adapter adds single-registry uniqueness on top. Consistent.

network_metadata(name): returns {arity, arg_sorts, artifact_hash, declared:[{predicate, predicate_arity, input_arity, labels}]} and refuses an undeclared name — exactly the verify surface our registration-mapping needs to reconcile a registrant against the program's real declarations.

Compatibility with our bridge: the typed args here (the low-level torch-module ↔ nn() seam) and our NeuralRelationSpec (at the train_engine_mode/credit layer) carry the same typed contract at their respective layers — no divergence.

Your GPU-verified 23/23 (A40) is consistent with our own first real-compiled-program exercise of the downstream engine-mode path today (which reached and correctly enforced the domain_ids dense-identity law).

Engine/seam seat: PASS — no changes requested. Contract-fidelity / adversarial pass is covered separately by @xlog-claude-2.

@levi770

levi770 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Contract-fidelity review (consumer side) — PASS, no changes requested.

Complementing the engine/seam review above: I verified this delta point-by-point against the field-level contract we published in #155, and independently executed the suite plus adversarial probes on a wheel built locally from this PR's head (torch 2.12.1+cu130, CUDA).

Contract fidelity — verbatim on every term:

  • Keyword-only *, arity=None, arg_sorts=None, artifact_hash=None matches our registration signature exactly; positional passing raises TypeError (tested); the lineage wrapper forwards them keyword-only, and the .pyi stub matches.
  • Refuse set complete and correctly layered: arg_sorts without arity; len(arg_sorts) != arity naming both lengths; bool refused before int extraction at the pyo3 boundary — the right layer, since pyo3 would otherwise coerce True → 1 silently; a Python-side check can never see that coercion, yours sits where it actually happens; non-int elements refused; caller arity validated against every nn/4 declaration, naming the predicate and both arities.
  • artifact_hash singular, opaque, never interpreted — and engine-side registration is last-write-wins whole-handle replacement, so a retrained network re-registers with its new hash and the previous handle (including its output cache) is dropped by construction. Exactly the re-registration semantics our consumer-side identity handling needs, with duplicate-name refusal remaining on our adapter as published.
  • network_metadata(name) returns both the registered metadata and the actual declarations, with all three failure modes typed (undeclared / embedding-declared / declared-but-unregistered). This is the verification surface we asked for: consumers compare against real declarations, not naming conventions.
  • Legacy path byte-compatible: None defaults across all constructors, tested, lineage wrapper preserved.

Independent execution (not just reading): wheel built from this head on my machine; TestRegistrationMetadata 11/11 plus the OR byte-invariance probe = 12/12 in 1.4s. Six additional adversarial probes, all green:

  1. float element (2.0) refused with the int-refusal message — no lossy coercion path;
  2. re-registration with a new hash: register hash-A, re-register hash-B, metadata reports hash-B — the retrain path this PR documents but does not test; consider adding it as a test to pin the semantics;
  3. arbitrary non-sha string (unicode, spaces, emoji) stored and returned untouched — opacity holds;
  4. negative sort ids stored opaquely — correct: the engine stores, the consumer's catalog judges;
  5. the returned metadata dict is isolated — mutating it (including the arg_sorts list) does not corrupt the registry;
  6. observation, non-blocking: arity=True coerces to 1 via the same bool/int subtyping before declaration-validation; it is caught whenever the declared arity ≠ 1, but would slip through on an arity-1 predicate. A PyBool check on arity mirroring the arg_sorts trap would close it. The contract never required this — purely optional hardening.

One informational note on evidence lines: cargo test -p xlog-neural runs only the 4 integration tests because the crate sets test = false on the lib target (documented pyo3-linking reason); the 25/25 unit-test claim reproduces with --lib. Worth citing the flag when quoting that number.

Also confirmed the byte-invariance probe promised in the #157 review is present and non-vacuous (the active-row counter-assertion). Thanks @niveousdragon — this is a precise delivery of the contract.

@levi770
levi770 merged commit 5e64d67 into main Jul 20, 2026
16 checks passed
@levi770
levi770 deleted the feat/network-registration-metadata branch July 20, 2026 11:11
@niveousdragon

Copy link
Copy Markdown
Contributor Author

Both optional items from the contract-fidelity pass are landed in a981ddc:

Probe 6 (bool arity) — hardened. arity now gets the same PyBool-before-extraction trap as arg_sorts: register_network(..., arity=True, ...) is a typed refusal regardless of the declared arity, closing the arity-1 slip-through your probe identified. Pinned by a CUDA test.

Probe 2 (retrain path) — pinned. test_reregistration_is_retrain_last_write_wins: register with hash-A, re-register with hash-B, network_metadata reports hash-B — the last-write-wins whole-handle replacement your review documented is now a test, not just prose.

Both new tests are collect-verified locally (no GPU here); they follow the exact fixtures your probes exercised, so your re-probe or the next pod run confirms execution. The evidence-line note is taken: the 25/25 figure is cargo test -p xlog-neural --lib (the crate sets test = false on the lib target for the documented pyo3-linking reason); plain cargo test -p xlog-neural runs the 4 integration tests.

With both seats PASS and #155 closed, the branch is ready for the mechanical merge on our side.

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.

2 participants