Skip to content

642 Storage SL5b allholonNodes fix - #657

Closed
nphias wants to merge 3 commits into
mainfrom
642-storage-sl5b
Closed

642 Storage SL5b allholonNodes fix#657
nphias wants to merge 3 commits into
mainfrom
642-storage-sl5b

Conversation

@nphias

@nphias nphias commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

Whole-space discovery becomes ordinary graph traversal. Publishing a lineage writes
Holon ─OwnedBy→ HolonSpace and its HolonSpace ─Owns→ Holon inverse; get_all_holons_internal
reads that Owns collection instead of the AllHolonNodes path index. This is the
coordinator-owned replacement #631 §3 and §9 named as SL5b's precondition.

GetAllHolons, the legacy dance, the wire binding, and the TS SDK keep their contracts.

One semantic change: the LocalHolonSpace anchor is no longer in GetAllHolons results — it is
not a member of its own Owns collection (#642 §1, §9), so every DB count drops by one. Callers
that want the anchor use TransactionContext::get_space_holon().

AllHolonNodes is still written and all of its tests still pass. Nothing reads it, which makes this
a reversible checkpoint rather than a cutover.

Changes

commit_functions.rs — new persist_space_membership, called from commit_holon's ForCreate
arm immediately after persist_holon returns. It writes the OwnedBy and Owns SmartLinks
directly through the existing persist_smartlink, in the node pass.

Membership is deliberately not staged for the relationship pass. See the first note below — this
is the load-bearing decision in the PR.

staged.rsrelationship_collections_for_commit excludes OwnedBy from every scope. Storage
authors membership; the coordinator never replays it. A staged OwnedBy can still arrive (cloning a
saved holon carries its persisted membership into the staged map) and is dropped rather than
re-anchored to the clone or to a new version.

guest_holon_service.rsget_all_holons_internal resolves the space and delegates to the
existing fetch_related_holons_internal with Owns; no new traversal code. delete_holon_internal
gains retract_space_membership, deleting the Owns link on the space and the OwnedBy link on the
holon before the node delete. create_local_space_holon gains a comment recording that its staging
bypass is now load-bearing.

Reading through fetch_related_holons_internal is a small win: the old code built its collection
with add_references and no keys, whereas the Owns link's canonical key is the owned holon's
key — so the collection is now keyed from tag bytes with no hydration, which is what every
get_by_key consumer behind this call already wanted.

Contract docsholon_service_api.rs, holon_dance_adapter.rs, get_all_holons_dance.rs,
transaction_command.rs, transaction_wire.rs, map-sdk/.../transaction.ts, and
schema-src/commands/schema.tdl (with the generated JSON regenerated — a one-line diff). All said
"all persisted holons".

Fixturescount_saved() drops the + 1 for the anchor, carrying eight fixtures at once;
load_holons_internal_fixture's five hardcoded MapInteger(1) baselines become MapInteger(0);
ENSURE_DB_EMPTY and the inline "+1 for the space holon" comments corrected. New
partial_commit_membership_fixture — see Testing.

Notable decisions

  • Membership is authored in the node pass, beside persist_holon. This is where the design
    landed after two earlier attempts, and the reason matters. Membership is what whole-space
    discovery reads, so a holon that publishes without it is persisted and permanently invisible —
    something AllHolonNodes could not produce, because it indexed inside PublishRoot.

    Staging the edge for the relationship pass reintroduces exactly that split. The relationship pass
    is skipped outright whenever any staged holon fails to publish, so a commit of two holons where
    one is rejected leaves the other published and unowned, with nothing to repair it later. Ordering
    OwnedBy first within the pass — which an earlier revision of this PR did — narrows the window
    but does not close it. Writing membership beside the node write restores the coupling
    unconditionally.

  • The coordinator never replays membership. Since storage authors it, replaying a staged copy
    can only do harm: a version-producing commit replays the full scope against the new version's
    id, which would make the space own the lineage root and every version after it separately (Storage SL5b — Replace AllHolonNodes with HolonSpace ownership #642
    §8 rejects re-anchoring per version). Excluding OwnedBy from every scope is one filter and
    covers the version case, the clone case, and any future path that stages it.

-resolve_inverse_relationship_name never sees OwnedBy, so the descriptor
short-circuit added for it is gone and the descriptor system is back to having no exceptions.

  • Deletion retracts membership both ways. delete_holon_node deletes only the path link and the
    entry; SmartLinks survive it, which would leave deleted holons discoverable (Storage SL5b — Replace AllHolonNodes with HolonSpace ownership #642 §9). The
    retraction lives in delete_holon_internal, not the persistence layer, which is deliberately
    descriptor-unaware.

  • A latent harness bug surfaced. FixtureHolons::counts() charged a deleted holon saved -= 1
    in addition to its head no longer counting as Saved. The old anchor allowance cancelled it
    exactly, which is why it went unnoticed and why delete_holon_fixture's post-delete assertion was
    commented out with a TODO. Deleted now contributes nothing and that assertion is enabled — it is
    how the retraction above is tested.

  • The saved-content comparator ignores OwnedBy. assert_saved_content_eq refused to compare an
    undescribed actual holon carrying any relationship content, and every saved holon now carries
    membership. Infrastructure-supplied edges never appear in fixture snapshots — the same reasoning
    that already tolerates commit-materialized inverse SmartLinks there.

  • all_holon_nodes.rs and its mod.rs wiring; index_under_all_holon_nodes and its PublishRoot
    call; ALL_HOLON_NODES_PATH; LinkTypes::AllHolonNodes and its four dispatch arms; both validators,
    the AllHolonNodesDelete rejection, and their re-exports; the all_holon_nodes_delete_for_test
    probe and the RootIndexLinkType::AllHolonNodes variant; both coordinator-surface.toml
    [[export]] blocks; and the AllHolonNodes tests and table legs. mock_conductor.rs's liveness
    probe calls the get_all_holon_nodes extern purely as a callability check and needs repointing.

  • validate_root_index_create and the shared rejection variants stay — LocalHolonSpace still uses
    them (Storage SL5 — Retire Obsolete Persistence Indexes #631 §9). Removing the enum variant renumbers LocalHolonSpace 1→0 and SmartLink 2→1 and so
    changes the DNA hash: the same accepted consequence as SL5a, inert by construction since the enum is
    the single source of truth for both the write and the read of every link.

Testing

The write and retract paths are both directly exercised, which matters because discovery now
depends entirely on them:

  • Create. Every ensure_database_count step pins membership exactly, now that the anchor
    allowance is gone: pre-commit steps assert the anchor is excluded, post-commit steps assert new and
    cloned lineages appear, and stage_new_version_fixture asserts a version adds no member.
  • Partial commit. partial_commit_membership_fixture (new) stages two holons, one carrying more
    properties than PVL permits, so its PublishRoot is rejected and the commit reports Incomplete
    before the relationship pass runs. It then asserts the other holon is still an owned member. This
    is the case the earlier staged-membership design got wrong; the fixture was confirmed to fail
    against that design and pass against this one.
  • Delete. delete_holon_fixture's post-delete count assertion, re-enabled here, asserts the
    deleted holon is gone from GetAllHolons — the retraction's only test, and the reason the harness
    counting bug above had to be fixed.

Not measured: the Owns fan-in cost. Every inverse Owns link is based on the space holon, and
put_smartlink scans all live links on the base for conflict detection — so a 191-holon core-schema
load is ~O(n²) tag decodes against one base. Numbers on this branch: dance_tests 144.9s,
holon_storage_tests 68.8s, pvl_validation_tests 47.9s, smartlink_tests 19.1s. A before/after
comparison still needs a baseline run on the parent commit. This is the open SL3 candidate-scan cost,
now on a hot path; conflict-detection semantics were deliberately left alone.

Environment note. The three workspaces share one target/; running cargo outside
nix develop mixes rustc versions and the next nix-side build fails with E0514 plus misleading
cannot find type X errors in untouched files. Also, npm run sweet:test skips the WASM rebuild and
the audit.

Scope

Unchanged: QueryExpression, QueryDance, query planning (#642 §5); any new public whole-space
query API; key or type indexes; the core-schema JSON inputs beyond one regenerated command
description; persistence_layer::smartlink and the Tag v1 codec; LocalHolonSpace bootstrap
behavior; exact-version reads and Storage SL2 record behavior.

OwnedBy and Owns still set neither IsDefinitional nor AllowsDuplicates in the TDL. Nothing
reads them — storage authors membership directly and the coordinator never resolves it through
descriptor policy — so it is not blocking, but it is a trap for anyone who later routes ownership
through that policy.

Still open, unaffected: the put_smartlink candidate-scan cost (SL3), now more load-bearing than
before, and the reference-layer/storage disagreement over what "the same link" means (SL4).

@nphias nphias linked an issue Aug 23, 2026 that may be closed by this pull request
11 tasks
@nphias nphias changed the title allholonNodes fix 642 Storage SL5b allholonNodes fix Aug 24, 2026
@nphias
nphias marked this pull request as ready for review August 24, 2026 15:17
@nphias nphias linked an issue Aug 26, 2026 that may be closed by this pull request
11 tasks
@nphias
nphias marked this pull request as draft August 26, 2026 10:34
@evomimic

Copy link
Copy Markdown
Owner

Comments on Draft PR

I know you have not submitted this PR for review yet, but I wanted to provide feedback now because I think the implementation is drifting into unnecessary complexity.

On the asserted need to populate OwnedBy / Owns directly during pass 1

Status: Disagree.

I do not think the asserted gap exists under the current transaction lifecycle.

A pass-1 failure produces an Incomplete commit response. It does not complete the transaction or clear its nursery. A holon successfully persisted in pass 1 is retained as a staged holon in Committed(saved_id) state, so its persisted identity remains available through the transaction’s staged references.

The relevant sequence is:

1. Pass 1 saves holon A and records its LocalId on the staged holon.
2. Pass 1 fails for holon B.
3. The commit response is Incomplete; pass 2 is skipped.
4. The transaction remains open and its nursery remains intact.
5. The caller repairs or abandons B, then commits again.
6. The subsequent complete commit processes ordinary relationships for A using its retained committed LocalId.
7. Normal relationship persistence writes A --OwnedBy--> Space and materializes Space --Owns--> A.

Therefore, A is not permanently invisible and does not need a special pass-1 persistence path. Before transaction completion, it is discoverable through the nursery/staged-reference surface, which is the appropriate recovery surface for an incomplete transaction. Whole-space discovery is a finalized persisted-membership view, not the recovery mechanism for partially completed transactions.

Proposed ownership implementation boundary

I think the required change is deliberately small: implement context-derived space membership entirely in the nursery’s HolonStagingBehavior, and keep commit relationship persistence unchanged.

stage_new_holon

  • Require the current HolonSpace from the transaction’s space manager.
  • Construct a bound HolonReference for that space.
  • Add OwnedBy → current HolonSpace to the new staged lineage.
  • If the transaction cannot provide its current space, return an explicit error rather than silently creating an unowned holon.

LocalHolonSpace bootstrap remains the explicit exception: it is created through its existing bootstrap path and is not staged as a member of itself.

stage_new_from_clone

  • Clone the ordinary relationship state.
  • Inspect the inherited OwnedBy relationship.
  • If it names a different space, replace it with OwnedBy → current HolonSpace.
  • If it already names the current space, retain one canonical membership.

An independent clone is a new lineage, so its ownership must be anchored to the current transaction space rather than the source lineage’s space.

stage_new_version

  • Clone the ordinary relationship state.
  • Remove OwnedBy from the cloned relationship map.

OwnedBy belongs only to the lineage root. A version’s owner is resolved by tracing its lineage to the root and following that root’s OwnedBy; copying the relationship onto the version would be misleading and risks re-persisting it against the new version ID.

Commit consequence

Pass 2 remains ordinary relationship persistence:

  • New lineages and independently cloned lineages already have staged OwnedBy, so normal relationship commit persists it and materializes Owns.
  • New versions have no staged OwnedBy, so there is nothing to suppress or specially filter.
  • No direct persist_space_membership path, hand-authored inverse SmartLinks, or global OwnedBy commit filter is needed.

An incomplete commit remains an open transaction with a retained nursery. Successfully persisted staged holons and their pending relationship work remain available until a later complete commit or rollback; this is not a reason to make membership a special pass-1 persistence action.

The partial-commit regression test should be changed accordingly: assert that the successful holon remains addressable through its staged reference after Incomplete, then assert membership appears after the follow-up complete commit.

Issue #642 contract departures in PR #657

The retrieval direction is correct: GetAllHolons traversing the active HolonSpace’s Owns relationship, excluding the space anchor, is the intended replacement path. The following points are departures from the issue’s specified design or Definition of Done.

  1. Ownership is authored directly at commit rather than remaining staged for ordinary relationship persistence

    Status: Disagree.

    The PR replaces the prior staged-relationship approach with persist_space_membership after PublishRoot succeeds.

    This changes ownership from staged lifecycle state into a special persistence side effect. Restore the prior approach: infer and add OwnedBy to the staged holon, then let ordinary relationship commit materialize Owns.

  2. OwnedBy is excluded from every ordinary relationship commit scope

    Status: Disagree.

    The PR filters OwnedBy out of both full and touched relationship commit scopes. It states that any staged OwnedBy, including one inherited during clone, is dropped.

    Handle version semantics in stage_new_version: remove OwnedBy from the cloned relationship map. New lineages and independent clones retain staged ownership; versions never carry it into pass 2. No global commit-time filter is needed.

  3. The inverse pair is hand-authored instead of using ordinary relationship commit behavior

    Status: Disagree.

    persist_space_membership directly constructs and persists both OwnedBy and Owns SmartLinks.

    Issue Storage SL5b — Replace AllHolonNodes with HolonSpace ownership #642 calls for ordinary relationship commit behavior to persist OwnedBy and materialize Owns. The direct pair bypasses the normal relationship-commit and inverse-materialization path, hard-coding relationship names, direction, key selection, properties, and duplicate behavior.

  4. The partial-commit fixture asserts a new, unsupported intermediate-state contract

    Status: Disagree.

    The new fixture requires a holon saved in pass 1 to appear in GetAllHolons immediately when another staged holon causes the commit response to be Incomplete.

    On Incomplete, the transaction remains open; saved staged holons retain their IDs and can be discovered through the nursery. A later complete commit can run ordinary relationship persistence for those committed staged holons. The fixture should instead assert retained recoverability after the incomplete response, then assert OwnedBy/Owns persistence after repair or abandonment of the failed sibling and a complete commit.

  5. AllHolonNodes retirement has not been implemented

    Status: Blocker.

    The PR description states that AllHolonNodes is still written and its tests still pass. The changed-file set does not remove the module, write, reader surface, link type, integrity validation, probes, externs, constants, or index-specific tests required by Issue Storage SL5b — Replace AllHolonNodes with HolonSpace ownership #642.

    Removing the complete AllHolonNodes surface is required by Issue Storage SL5b — Replace AllHolonNodes with HolonSpace ownership #642. It must remain in this PR’s scope before the PR can claim to deliver or close the issue.

  6. Deletion cleanup is implemented through another hand-managed relationship path

    Status: Partially agree.

    The required outcome is correct: deleted holons must not remain discoverable through Owns.

    However, retract_space_membership repeats the direct ownership-persistence exception and assumes all discovered OwnedBy links are space membership managed by this service. Prefer the corresponding lifecycle-aware relationship cleanup, with explicit ownership-scope semantics, rather than a second special direct-link protocol.

  7. Coverage does not demonstrate the intended lifecycle semantics

    Status: Partially agree.

    The PR adds useful coverage for count changes, version non-duplication, deletion, and its proposed partial-commit behavior. It does not demonstrate the intended staged ownership lifecycle, independent-clone re-anchoring, or ordinary inverse materialization.

    Replace or supplement the partial-commit test with tests that pin the intended lifecycle:

    • A new holon receives context-derived OwnedBy in staged state.
    • An independent clone is re-anchored to the current transaction space.
    • A version omits OwnedBy from its cloned staged relationship map.
    • An incomplete commit retains saved staged identity without requiring finalized Owns visibility.
    • A subsequent complete commit materializes the normal forward and inverse relationship pair.

Changes consistent with Issue #642

  • GetAllHolons now resolves the current space and traverses Owns.
  • The public command, wire, host dispatch, and SDK surface remain in place.
  • The LocalHolonSpace anchor is excluded from GetAllHolons.
  • Version-producing updates do not add membership for each historical version.
  • Deleted holons are intended not to remain discoverable.
  • No QueryExpression, QueryDance, or new public query API is introduced.

Recommended revision

  • Retain the retrieval migration and anchor contract.
  • Revert persist_space_membership and restore staged ownership lifecycle behavior in the nursery.
  • Handle clone re-anchoring and version ownership removal in their respective staging methods.
  • Treat incomplete commit as an open transaction with recoverable nursery state, not as a completed whole-space-discovery state.
  • Complete the AllHolonNodes retirement surface before claiming Issue Storage SL5b — Replace AllHolonNodes with HolonSpace ownership #642.

@nphias

nphias commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

moved work to sub issues

@nphias nphias closed this Aug 26, 2026
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.

Storage SL5b — Replace AllHolonNodes with HolonSpace ownership

2 participants