Skip to content

VC3D line annotation: adjacent link candidates - #1829

Merged
hendrikschilling merged 8 commits into
mainfrom
line-annotation-adjacent-links
Sep 19, 2026
Merged

hendrikschilling merged 8 commits into
mainfrom
line-annotation-adjacent-links

Conversation

@skyward7187

@skyward7187 skyward7187 commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Motivation and behavior

Adjacent fiber links connect a vertical fiber to the horizontal fiber one winding outside it (W_V = W_H − 1). Keeping them in branches would let existing same-winding consumers fuse the two windings. This PR stores them exclusively in a separate top-level adjacent_branches array, using exactly the existing branch-entry schema. There is no per-entry adjacent key.

Current VC3D always writes adjacent_branches, even when empty. A missing array identifies a legacy writer; a present empty array expresses a deliberate absence of adjacent links. Both reciprocal refs must have the same kind. Before cross-file validation, VC3D restores missing adjacent reciprocals into peers whose array was absent, leaving peers with a present array untouched. The existing concurrent-save guard remains in place.

fiber_merge merges, reanchors and refreshes both arrays through the same branch machinery. A kind change moves an entry between arrays and retains the new link's review state. A base with adjacent entries and a side lacking the array is a conflict, including through whole-document shortcuts and plain vc_sync uploads.

The input verification digest now includes adjacentExplicit (Hendrik’s P2). Changing whether a reciprocal states its kind can change disagreement handling and solver output; those inputs now have different digests, preventing a false cache-mismatch report.

The GUI designates adjacent candidates and draws adjacent links as triangles. The fiber map applies the H/V-derived one-turn constraint, excludes adjacent links from kollesis seam evidence, and flags invalid H/V pairs and kind disagreements. The existing candidate re-designation fix is also retained.

The format proposal includes the schema, compatibility rules and verification commands.

Compatibility

  • Existing spiral-fitting, vesuvius and HF-publishing readers inspect branches and never see adjacent links.
  • Old VC3D ignores adjacent_branches on load and drops it on save. A surviving reciprocal or synced base lets current software restore or detect that loss.
  • Old fiber_merge carries the array from whichever side it copies and cannot merge the other side's changes. A format-version fence remains a team decision; this PR does not bump the version. If both endpoints lose their arrays and no surviving reciprocal or synced base remains, those files alone cannot recover the links.

Verification

  • Ubuntu amd64, RelWithDebInfo: VC3D builds; line_annotation_adjacent_links, fiber_network_layout, fiber_winding_solver and fiber_global_layout pass.
  • P2 regression: changing only adjacentExplicit produces different solver outputs and now different input digests. The test failed before the hash fix and passes afterward; the full global-layout suite passes. A digest check using the real PHercParis4 pair dj_20260717T165249423_000001.json / lt_20260715T081231472_000515.json (36 control points, 4,791 line points) likewise failed before and passed after, using in-memory copies.
  • 278 Python merge/sync tests pass, covering both arrays, missing versus empty, strips, re-links, approvals, geometry rebinding, opaque entries, peer discovery and same-kind reciprocal refresh.
  • Ten existing reciprocal pairs from the local PHercParis4 v3 annotations passed merge, adjacent restoration, ordinary re-link and strip-conflict checks on in-memory copies. Control-point and line geometry remained identical; source annotations were not written.

Commands (using the existing Python environment with pytest):

AGENTS_AGENT_MODE=1 python -m pytest -q volume-cartographer/scripts/tests
AGENTS_AGENT_MODE=1 cmake --build volume-cartographer/build --target VC3D test_line_annotation_adjacent_links test_fiber_global_layout test_fiber_network_layout test_fiber_winding_solver --parallel 2
AGENTS_AGENT_MODE=1 ctest --test-dir volume-cartographer/build -R '^(line_annotation_adjacent_links|fiber_global_layout|fiber_network_layout|fiber_winding_solver)$' --output-on-failure

macOS and arm64 were not run in this environment. No map, solver or GUI behavior changed in the separate-array revision.

@vercel

vercel Bot commented Sep 18, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
scrollprize-org Ignored Ignored Preview Sep 19, 2026 12:54am UTC

Request Review

@skyward7187
skyward7187 force-pushed the line-annotation-adjacent-links branch from 12cf3c0 to 2ba0cf5 Compare September 18, 2026 15:58
skyward7187 and others added 3 commits September 18, 2026 11:58
A second kind of link: an ADJACENT link ties a V fiber's control point to
an H fiber's control point one winding outside it (horizontals lie on the
front of the sheet, verticals on the back, so a V fiber showing through to
the next wrap out is one sheet thickness from it): W_V = W_H - 1.

- "Designate as adjacent link candidate" on a control point; the
  candidate draws as a green triangle. On another fiber the menu reads
  "Link with adjacent candidate (...)", and a new line annotation can start
  linked to it. Any pair may be linked: a fiber's type can change and a new
  fiber has none yet, so the tags are not checked here.
- FiberBranchRef::adjacent, written as `"adjacent": true` on both
  reciprocal entries (key held in one constant), absent otherwise; the
  loader reads it, the controller's dedup keeps it, and the cross-file
  validation does not compare it (a kind mismatch is the sync merge's
  business).
- Adjacent-linked control points draw as triangles in the same link-state
  colours (blue pending, purple approved) in the cut views, generated
  views, overview strip and the fiber map; the cut view's X glyphs and
  connectors are unchanged.
- The fiber map hands the winding solver a one-turn offset
  (LinkInput::windingOffset) instead of an equality and measures the link's
  turn error against it. An adjacent link whose fibers are not one H and
  one V has no inside: it is flagged as an error (PlacedLink::
  adjacentUnpaired, always suspect, named in the errors list) and carries
  no constraint. Adjacent links are never kollesis seam evidence. The
  per-network unroll has no winding dimension and treats every link as an
  angular tie, as documented.
- Tests: the V fiber lands one winding inside; an unpaired adjacent link
  is an error that constrains nothing; an adjacent link cannot certify a
  kollesis; the output digest is sensitive to both new flags.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YKBWyFShke85mC653ujChD
`adjacent` is a link's kind, never changed on an existing link, so the
refresh mirrors A's value onto B's reciprocal (like the review state) and
restores it on a rebuilt reciprocal; a changed kind counts as a
modification (a pair re-linked as adjacent survives the other side's
deletion of the original); and a three-way merge that meets the same
anchor pair as an adjacent link on one side and an ordinary one on the
other reports a conflict instead of picking. Five tests.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YKBWyFShke85mC653ujChD
Designating a control point as a link candidate (ordinary or adjacent)
after it was a split candidate, or the reverse, left both designations in
place; the split marker takes precedence, so the point stayed red however
it was re-designated. Each designation now clears the other kind on the
same point. Candidates on different points still coexist, as "split from
candidate and link" requires.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YKBWyFShke85mC653ujChD
@skyward7187
skyward7187 force-pushed the line-annotation-adjacent-links branch from 2ba0cf5 to c3ee637 Compare September 18, 2026 16:00
@hendrikschilling

Copy link
Copy Markdown
Contributor
1. [P1] New adjacent links are silently misinterpreted by spiral-fitting.
  spiral-fitting/spiral_helpers.py:279 (https://github.com/ScrollPrize/villa/blob/c3ee63785f81ba9b70073a39b1b96b5bc6d78b4a/spiral-fitting/spiral_helpers.py#L279)

  The loader drops adjacent, and resolve_fiber_links() treats the resulting link as same-winding. Once approved, a link explicitly saying “different windings” therefore joins the fibers as
  though they belong to the same winding.

  I reproduced this through the actual loader and resolver. The PR acknowledges it as out of scope, but it remains a compatibility problem even with all tools built from this branch, not  
  merely with older installations.

  Minimum fix: preserve the flag and explicitly reject or skip adjacent links in unsupported consumers until they support winding offsets.

  2. [P2] The input digest omits adjacentExplicit, although it changes solver output.
  volume-cartographer/apps/VC3D/FiberNetworkLayout.cpp:2006
  (https://github.com/ScrollPrize/villa/blob/c3ee63785f81ba9b70073a39b1b96b5bc6d78b4a/volume-cartographer/apps/VC3D/FiberNetworkLayout.cpp#L2006)

  An absent ordinary flag permits the reciprocal adjacent constraint; explicit false makes it a disagreement and disables that constraint. However, both inputs hash identically.

  Confirmed locally: same input digest, different output digest and disagreement status. The full-rebuild verification can consequently report a false CACHE MISMATCH and disable
  memoization. This is a verification/performance bug, not evidence of stale rendered geometry.

  Fix: include adjacentExplicit in the input digest.


@hendrikschilling

Copy link
Copy Markdown
Contributor
  [P2] An ordinary-link refresh can prevent recovery of unrelated adjacent links.
  volume-cartographer/scripts/fiber_merge.py:1288 (https://github.com/ScrollPrize/villa/blob/8c84e82462c80fee98e446488d9b9a0f04580e3b/volume-cartographer/scripts/fiber_merge.py#L1288)     

  Every modified peer gets adjacent_branches: [] if its array was absent. That changes “legacy writer, adjacent links unknown” into “explicitly no adjacent links.”

  Reproduced:

  1. B has an ordinary link to A and has lost its adjacent reciprocal to C through an older writer.
  2. Refreshing an approval on A–B inserts the empty array into B.
  3. The B–C sync check now reports success instead of the previous missing-array conflict.
  4. VC3D’s loader will not restore C’s reciprocal into B because B’s array is now explicitly present.

  Fix: preserve an absent array during unrelated refreshes. The same normalization in merge shortcuts needs consideration too.

@skyward7187
skyward7187 requested a review from pmh47 as a code owner September 18, 2026 18:32
@hendrikschilling

Copy link
Copy Markdown
Contributor
  [P2] Full three-way merges still manufacture an empty adjacent array.
  volume-cartographer/scripts/fiber_merge.py:1221 (https://github.com/ScrollPrize/villa/blob/f4fe86757bf8baf36b6a1dbfce01cabdb035e362/volume-cartographer/scripts/fiber_merge.py#L1221)     

  The refresh and shortcut paths are fixed. But the full merge still unconditionally writes both arrays.

  Reproduced with all three inputs lacking adjacent_branches and independent tag edits on local/remote:

  - Merge succeeds and inserts adjacent_branches: [].
  - The surviving peer’s missing-array conflict disappears.
  - The result still has a missing reciprocal, and VC3D’s automatic recovery is disabled.

  Minimal fix: preserve absence in the full-merge path too, at least when every input lacks the array and no adjacent entries are produced. Add that case to the tests.

The shortcut and refresh paths already preserve absence; the full
three-way merge still wrote both arrays unconditionally, so a merge whose
saved sides are legacy inserted `adjacent_branches: []`. That turns "a
legacy writer saved this, the kind is unknown" into "deliberately no
adjacent links", which hides a peer's missing reciprocal from the sync
check and stops VC3D's loader restoring it.

The array is now left out when the merge produced no entries for it and
neither content side carried the key. The base is not consulted: it only
arbitrates deletions and contributes no entries, so its own array says
nothing about what the two saved files know, and a base with real
adjacent entries facing a stripped side is already a hard conflict. An
input that knows the kind still yields an explicit empty array.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YKBWyFShke85mC653ujChD
@skyward7187

Copy link
Copy Markdown
Contributor Author

Fixed in 8b89042, minimally: the final for kind in BRANCH_ARRAYS loop in merge_fibers now skips writing an array when the merge produced no entries for it and neither content side carried the key. Nothing else in that path changed.

for kind in BRANCH_ARRAYS:
    entries = merged_branches[kind] + opaque_branches[kind]
    if not entries and all(kind not in doc for doc in (local, remote)):
        continue
    merged[kind] = entries

Your case is now test_full_merge_preserves_an_absent_adjacent_array: three inputs without the array plus independent tag edits on local and remote, so no whole-document shortcut applies and the full merge runs. It asserts the array stays absent and branches is still written, and it fails without the fix.

One thing while checking it: the condition on all three inputs was not quite enough. If the base carries an empty adjacent_branches and both saved sides were stripped by an older VC3D, the base alone forced the array back in, manufacturing "deliberately none" from two writers that did not know the kind. The base only arbitrates deletions and contributes no entries, so it is no longer consulted. That stays safe because a base with real adjacent entries facing a stripped side is already a hard conflict before this point, which I verified rather than assumed. The extra case is asserted in the same test.

Also verified on the real code paths:

  • A base that does carry adjacent entries with both sides stripped is still a conflict, so this does not mask a real loss.
  • With the merged document as the next base: absent lineage raises nothing, a knowing base with an entry still raises the strip conflict.
  • is_fiber_doc accepts the merged document, and nothing reads the merged document's arrays by direct indexing; links_to_any, peer_files and the rest go through _branches_of, which tolerates a missing key. The only direct indexing is on the merge's internal merged_branches dict, which always has both keys.
  • branches is in BRANCH_ARRAYS too, but it can only be skipped for a document that never had one, and VC3D's writer always emits it.

Worth stating plainly, since it is a user-visible consequence rather than a pure bug fix: auto-merge output can now genuinely lack adjacent_branches, so the missing-array guard can fire on merge output and demote a fiber to a manual conflict where it previously auto-merged and quietly left the peer's adjacent reciprocal one-way. That is the point of the fix, but it does mean more manual conflicts on fibers an older VC3D has touched.

@hendrikschilling
hendrikschilling enabled auto-merge (squash) September 18, 2026 22:08
@hendrikschilling
hendrikschilling merged commit f07d33b into main Sep 19, 2026
30 checks passed
@hendrikschilling
hendrikschilling deleted the line-annotation-adjacent-links branch September 19, 2026 01:05
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