Skip to content

Keep dense graph neighborhoods readable and organize connected items - #234

Merged
mvalancy merged 37 commits into
devfrom
feat/topology-organization-20261028
Nov 8, 2026
Merged

mvalancy merged 37 commits into
devfrom
feat/topology-organization-20261028

Conversation

@mvalancy

@mvalancy mvalancy commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

When Items mode focuses a high-degree item, it now shows readable original neighbors, named overflow groups and bounded schematic buses. EPIC2215 used to paint 107 wires while every neighbor was offscreen. The local display cut keeps useful endpoints on screen without changing original positions. Paging and filtering, directed original-edge inspection, original editing, Back and reload all keep their context. Small direct graphs keep their native view.

Organize now uses connectivity. It places central hubs, chains and components with measured card clearance and bounded refinement. Short chains snake to fit the viewport, and the camera frames each component from its hub, the middle of a chain or the top of a cycle. Overview buses keep 64px card gutters, 20px clearance from unrelated cards and 16px from unrelated wires, and report honest coverage of the original links.

On phones, the status/navigation bar spans the bottom edge with equal app tabs and 44/48px touch controls. Measured three-line item titles stay at 18px.

A freshly organized graph is framed before its first paint. Healthy save notes are passive: no disabled buttons, taps pass through, and they never reframe the camera. So the first phone tap opens the editor.

Closes #233. General arbitrary detailed item routing remains #220.

Status (2026-11-07, after the history rewrite)

All SHAs before today's rewrite are historical. The current head is the rewritten branch plus two fixes:

The previous final CI (40826842888) failed the new-user gate: single-tap editing at 320×568 and 390×844. The earlier "review clear" statement no longer applies. Independent review 5813599782 confirmed that the blocker was still present on the rewritten head.

Local validation: serial, one worker, disposable Neo4j, headed under xvfb.

  • Lint has 0 errors; types pass.
  • Focused web units: 95/95.
  • responsive-real-workflows: 7/7 widths, plus 3 repeats each at 320 and 390. The 320 case had failed every run before the fix.
  • topology-organization + layout-recovery + local-item-lod: 17/17.
  • The new first-tap regression passes 4/4 repeats at both widths. On the previous source it fails at both widths (Δk 0.480 at 320, 0.0376 at 390).

Heavy matrices (new-user, 2k zoom, LOD and mobile sequences) run in hosted CI on the new head.

Hosted CI follow-ups, independently reviewed

Each hosted red was fixed rather than retried. Every fix was reviewed by an independent reviewer: 5814078018, 5814131350, 5814430376.

  • cf58c97 + c43a6db: 2k zoom budget. It failed at 6.6 s against a 5 s budget because presentation ran in 4 passes instead of 3. During a programmatic zoom, leaving the overview now defers to the single end-of-zoom pass. User gestures still re-render at the threshold.
  • ae47b26: desktop overview grid. The overview again keeps its regular grid whenever that grid holds every card.
    • 8ee27ea had lowered the floor from 13 to 11 pairs. The reviewer showed that this hid a regression from b72a87e, which spread the grid over 4 rows: 11/38 pairs and 60/361 relationships instead of 13 and 101. That change is reverted, the floor is back at 13, and a floor of 101 desktop / 45 phone drawn relationships is added.
    • The group browser now uses z-30, so the status dock no longer covers it.
  • 337fca4 + 9defd8b: viewport-aware packing. Loose components pack to the viewport's aspect when that fits at a strictly larger scale.

After 9defd8b, three more changes:

  • 7300420: merge of dev. Dev's dd87b87 moved camera fitting into lib/graphFit. The conflict was resolved by fitting through computeGraphFit while keeping this branch's organization floor and component focus framing, both measured at the final scale. An independent review of the merge (5814850645) found no issues. Locally, 31/31 focused Playwright specs passed on the merge, including dev's fit-focus.
  • 7f91b57 + a116e79: local cards after reload. Hosted semantic-lod failed at 390px after reload with every local card at x=3844. The same failure had appeared once on an earlier head. With 4x CPU throttling it reproduced 4/4. Cause: a deferred rebase re-read the camera prop before it caught up with the restored camera, which put the layer back on the identity camera. The layer's latest-camera ref now takes the settled camera directly. Results:
    • The throttled journey passes 3/3.
    • A new unit test fails on the old code.
    • Review 5815087455 found no issues.
  • Later merges of dev: 5e76858 and 86192c1. Both were clean and touched docs, CI, release and the public-content guard. Toolchain contracts pass.

CI 42403937015 on 9defd8b passed in full. Comprehensive passed too. Local runs (serial):

  • Unit tests: web units 536/536.
  • Specs: semantic-lod + local-item-lod + nested + topology + layout-recovery + item-wires 23/23. new-user-modes + admin-dialog 18/18 over 3 repeats.
  • Sequence: new-user 41/43. The two failures were an admin-form timeout and the run-level evidence check that fails with it. The timeout happened while the shared browser cache was being deleted, and the same test passed 3/3 when re-run. All review threads are answered and resolved.
Earlier PR body (historical, cites pre-rewrite SHAs)

Items mode around a high-degree focus now shows readable original neighbors and named overflow groups with bounded schematic buses. EPIC2215 previously painted107 wires while every neighbor was offscreen; the local display cut keeps useful endpoints on screen without changing original positions. Paging/filtering, directed original-edge inspection, original editing, Back and reload retain context. Small direct graphs keep their native view.

Connectivity-aware Organize places central hubs, chains and components with measured card clearance and bounded refinement. Overview buses retain64px card gutters,20px unrelated-card/16px unrelated-wire clearance and honest original-link coverage. Phone status/navigation spans the bottom edge with equal app tabs and44/48px touch controls; measured three-line item titles remain18px.

Closes #233. General arbitrary detailed item routing remains #220.

Final source: 41c1fa015e16ae9e8cfe54c3336ff68100df29b6, composed with accepted live dev479edf6/PR236. Independent exact-source review is clear; all findings answered/resolved.

Validation: coherent six-case headed organization/local journeys pass90.16s; final-head affected three cases pass51.6s after strengthening native-class isolation negatives, live D3 x/y/fx/fy equality and settled offscreen paint.107-neighbor desktop/phone fixtures check actual paint,64/20/16clearance, exact directed original IDs, hover/keyboard, paging/filter reset, focus/Back, small pan, cold reload, one exact original-title edit and unchanged remaining source data. Phone tabs are130/130/130px across390px,50px high, no overflow; status dock is full-width/flush.44focused model/layout/selection/controller/inspector controls, the compact/expanded storage-label controls, types and scoped lint pass (legacy inspector-any warnings explicit). Earlier failures and rejected screenshots are retained in canonical lessons.

Pending gates: CI40826842888, Comprehensive40826842856. The existing2,000-item/5,000-relationship gate now also exercises a111-neighbor hub (107added), local→overview→local under the unchanged5s budget, and actual canonical conservation; ordinary detail/priority/input budgets remain. Final hosted13-report/runtime/package artifact audit, dev-only exact-head integration and actual post-dev acceptance remain required before closure. No original localhost changes, heavy local matrix, main/deployment or hosted-parity claims.

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown

🧪 Comprehensive Test Suite

  • Unit suites (Node 22.18.0 & 24.x) — core, web, server, mcp-server: ✅ passed
  • Installer & deploy config: ✅ passed

Full-stack smoke gate runs in the CI workflow.

@github-actions

Copy link
Copy Markdown

🧪 Comprehensive Test Suite

  • Unit suites (Node 22.18.0 & 24.x) — core, web, server, mcp-server: ✅ passed
  • Installer & deploy config: ✅ passed

Full-stack smoke gate runs in the CI workflow.

@mvalancy mvalancy left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent source review of 2d81ac0279156ca6ca41761ef9c1a1a79fcbe31b against 4b825b12a752f54531a2263816630501396eabb8: two P2 findings block acceptance of this draft source.

I read the complete 15-path change, issue233, new lesson/architecture guidance and the relevant renderer/save/mode lifecycle. I independently ran the seven new pure topology/overview tests on exact-source copies with one worker: 7 passed, 0 skipped, 643ms. Connectivity deduplication, deterministic component/ring ordering, the 40,000-edge refinement budget, bounded overview swaps, truthful hidden/deferred IDs and the new clearance measurements look consistent with their stated pure contracts. The new agent note/index do not change integration authority. Browser sequence enrollment preserves the existing workers/retry/timeout policy.

The two lifecycle probes execute exact source blocks with the actual pure layout helpers and lightweight dimension/simulation adapters; they are not browser reproductions. An initial delayed-edge hypothesis was withdrawn after examining the full render gate: while either query loads, the SVG is absent and initialization returns. Its isolated-block output is explicitly marked invalid as product-failure evidence in the retained scratch README. No correction is requested for that withdrawn hypothesis.

One existing boundary must remain honest in the new docs: initialization still calls separateCards on saved overlapping positions and persists that repair. That predates this PR, but it prevents an unconditional claim that saved positions remain unchanged until Organize. Preserve the disclosed repair exception or deliberately change and test the policy.

This is source review only. I did not run a browser, database, install, build or full local matrix. The documented phone readability correction, actual failure-before/fix-after lifecycle regressions, the detailed organizer’s scale acceptance, full hosted gates/screenshots and post-dev acceptance remain pending. The existing 2,000/5,000 browser fixture uses saved positions and does not by itself execute the new explicit detailed organizer. Radial placement is not completion of obstacle-aware item routing #220.

Read-only evidence: /tmp/graphdone-resolution-20261027/pr234-review-2d81ac0/ (pure-tests.log, exact-source copies, probe.cjs, lifecycle-probe.log, probe-limits.json, README.md). No source/index edits, pushes, merge or issue closure.

@github-actions

Copy link
Copy Markdown

🧪 Comprehensive Test Suite

  • Unit suites (Node 22.18.0 & 24.x) — core, web, server, mcp-server: ✅ passed
  • Installer & deploy config: ✅ passed

Full-stack smoke gate runs in the CI workflow.

@mvalancy mvalancy left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent correction/composition review of immutable 97763da249c96598416ecb89b3bc38195c2a84b6: both original P2 lifecycle findings are corrected. No additional blocker found in those corrections.

The complete composition has parents 409d28a062426a0c1e84094760e676195565dc05 and accepted PR232 dev 17acc25dc98978c64f87a5b0d6fb36b1833cdfa7, tree 25452870c699072cd75b7e3950aa7d5b9d7651dd. Its diff from accepted dev stays within the 17 topology feature paths; the accepted bus interaction implementation is retained, the shared overview component adds topology placement/clearances, and the index retains both lesson entries. No integration authority changed.

I independently ran exact-source copies of the topology, overview and camera unit files: 9 passed / 0 skipped in 616ms, one worker. A small source-level lifecycle probe confirms complete simulation membership, all five IDs reaching organization, force mode stored and notified, zero applications from an already-pending hierarchical callback, and no hierarchical timer on reload. This probe is not browser execution. The saved-position repair exception is now explicitly documented.

I inspected the author’s retained actual 4-case report: 4 passed / 0 skipped / 0 flaky in 42.860s, including native low-zoom title editing and delayed authoritative edges. Its existing phone visual result remains rejected; a passing report does not override that usability finding. I also inspected the composed 301-item desktop report: 1 passed in 51.224s. That test covers its existing card geometry and bus interaction assertions; it does not call the new inspectSchematicOverview clearance oracle. The 64/20/16 measurements are enrolled in the nested and large-graph suites, so the 301 run alone is not quantitative clearance acceptance.

No final PR acceptance is granted here. This review excludes uncommitted phone changes, which require a new immutable review and actual visual evidence. Final-source hosted gates, large/nested quantitative clearance and scale evidence, and post-dev acceptance remain required. No local browser/database/install/build/full matrix or source/index mutation was performed. Evidence: /tmp/graphdone-resolution-20261027/pr234-review-97763da/ (lifecycle.cjs, lifecycle.log, pure-tests.log, complete composition.raw).

@github-actions

Copy link
Copy Markdown

🧪 Comprehensive Test Suite

  • Unit suites (Node 22.18.0 & 24.x) — core, web, server, mcp-server: ✅ passed
  • Installer & deploy config: ✅ passed

Full-stack smoke gate runs in the CI workflow.

@github-actions

Copy link
Copy Markdown

🧪 Comprehensive Test Suite

  • Unit suites (Node 22.18.0 & 24.x) — core, web, server, mcp-server: ✅ passed
  • Installer & deploy config: ✅ passed

Full-stack smoke gate runs in the CI workflow.

@mvalancy mvalancy left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review of phone/paint/lifecycle source 8516bc59e4c84f36f2cffdf21f005677e4f1bd29 and its four-path geometry correction f5640464d1b85b651863abe9b3b36024cee1ffb7: the new P2 clearance defect is corrected; no additional blocker found in this reviewed chunk. This is not final PR acceptance.

The new finding was verified with actual production layout functions: compact slots used individual measured card sizes, then length-only refinement moved larger cards into smaller slots. A five-item fixture reduced minimum clearance from 64.000027px to 9.000027px. The corrected function rejects swaps that violate any component-card clearance, with separately counted/capped rectangle work. Independent replays now preserve 64.000018px and 64.000027px for the two retained fixtures; 13 exact-source pure tests pass in 666ms. No source mutation or browser run was needed for this proof.

The explicit-organize camera fix now marks the graph framed, which the existing initial-fit callback checks before doing work; it therefore stops that competing camera owner. Scope changes still reset organization framing, and explicit Zoom to fit removes the readable-scale floor. The save-progress attribute excludes only normal progress from permanent organized framing; attention/error states remain obstacles. The bounded phone angle/aspect search and stronger focus-priority scorer match their stated limited scope. They are not an obstacle-free guarantee for every large graph or transient overlay.

I inspected the retained topology-human-painted.json report: 4 passed, 0 skipped/flaky, 45.672s. I independently checked its desktop/phone geometry receipts and viewed all four organized/reloaded PNGs. All nine desktop and five phone cards are painted; complete fixture titles remain readable, and actual recorded screen bounds have zero delta between organized and reloaded captures. Original relations remain 8/4 with zero reported page errors. The new test requires 12px screen/control margins and repeats readability after camera framing, entrance opacity and persisted camera restoration. These are the retained author-run 8516-era visuals, not a new final-head hosted run or proof for heterogeneous cross-linked fixtures; those are covered here by the pure negative/correction.

The earlier 97763da phone clipping and faded/early reload captures remain rejected evidence. The docs correctly retain the existing saved-overlap repair exception, normal-progress/attention distinction, quantitative 301-run evidence correction, and #220 detailed-wire-routing limit. The correction changes only topology source/test and their two documents; no integration authority changed.

The uncommitted overview accounting chunk is outside this review. Final immutable composition review, actual quantitative overview/scale receipts, full hosted gates and post-dev acceptance remain required. No local browser, database, install, build, matrix, source/index edit, merge or issue closure was performed. Evidence: /tmp/graphdone-resolution-20261027/pr234-review-8516bc5/ including original production-function negative inputs/logs, fixed-replay.json, and fixed-pure-tests.log.

@github-actions

Copy link
Copy Markdown

🧪 Comprehensive Test Suite

  • Unit suites (Node 22.18.0 & 24.x) — core, web, server, mcp-server: ✅ passed
  • Installer & deploy config: ✅ passed

Full-stack smoke gate runs in the CI workflow.

@mvalancy mvalancy left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent bounded UI/visual/accounting review of 6ba55f237963af0d5c9b4f6fe7899a1dc01ebc04.

I inspected the actual topology-phone-slot-output at-rest PNG and geometry receipt, read the exact slot-phase/test delta, and independently executed the previous/current pure placement code against the measured phone obstacles. The previous source offers 2 slots; this source offers 3, matching admission capacity, with unchanged inputs, 240×96 cards, 64px row gaps and at least 20px control clearance. Candidate phases are bounded to 32 and placement/capacity share the slot calculation.

The phone image is readable: complete cards at y342/502/662, 18px titles, visible directional arrows, and useful whitespace around controls. Its four drawn directed group pairs represent 45 original external relationships out of 361; the receipt retains 316 deferred plus 9 internal relationships, totaling all 370. All three displayed groups have a drawn connection; this does not mean all eight groups or all 38 eligible pairs are displayed. The visible denominator, “5 more in group list,” and “Browse all 370” make that limitation understandable. The receipt reports zero card intersections/unrelated crossings/overlaps/diagonals, with 20px unrelated-card and 96px unrelated-wire minimum clearance. The actual local report has 1 passing phone journey, no retries/skips/failures, 40.665s total.

I also previously inspected the retained desktop 301 overview and the corrected initial/reloaded desktop-nine/phone-five star screenshots. Those sampled frames are clean and readable; the earlier clipped/faded captures were rejected and are not acceptance evidence. The desktop coverage receipt represents 101/361 external relationships and 13/38 pairs, not complete wiring coverage.

No new blocker in this reviewed UI/accounting slice. This is not whole-PR approval, final-source hosted acceptance, proof of useful coverage for every graph, or proof that mixed detailed routing is spaghetti-free. Current composed-source 2,000-item/5,000-relationship, nested, runtime/full CI and post-dev evidence remain required. Detailed item-routing/bus work remains #220. No repository edits, browser runs or merge performed by this reviewer.

@github-actions

Copy link
Copy Markdown

🧪 Comprehensive Test Suite

  • Unit suites (Node 22.18.0 & 24.x) — core, web, server, mcp-server: ✅ passed
  • Installer & deploy config: ✅ passed

Full-stack smoke gate runs in the CI workflow.

@mvalancy mvalancy left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent exact-source review of 6ba55f2 against accepted dev 17acc25: one new P2 minimap lifecycle finding posted inline; no integration clearance yet.

The 22-path composition retains the previously reviewed organization/lifecycle and compact-ring clearance corrections. No workflow authority or dependency changes were introduced. The new overview accounting partitions the same canonical edge projection into external/internal/pending counts; deferred plus priority-summarized plus actual drawn counts equal the external denominator. The 301 journey uses the separately queried original edge list, retains attempted-write rejection/final source-state checks, and now calls the quantitative clearance helper before inspection.

The slot search considers at most 32 deterministic phases and uses the same slot function for capacity and placement. Card size, row/column spacing and obstacle margins are preserved. I independently ran 29 exact-source semantic projection/slot, topology and camera cases: all passed in 876 ms with one worker. An initial scratch-copy setup omitted graphSymbolLayout and could not collect that suite; it was corrected before this result and is not product failure evidence.

I read the retained 0b desktop and 6ba phone geometry receipts and viewed their actual at-rest PNGs. Recomputing pairs from the actual SVG routes gives desktop 101/361 external links, 13/38 directed pairs, 8/8 displayed groups connected, card/wire clearance 20/24 px; phone 45/361, 4/38, 3/3, clearance 20/96 px. All routes are painted and both receipts report zero card intersections, unrelated crossings/overlaps or diagonals. The remaining originals are honestly available through Browse; these fixture results are not a universal coverage guarantee.

This is bounded source/local-evidence review. Hosted CI 40815091344 is still running, and the minimap correction plus any PR236 composition require renewed exact-source and actual final artifact review. No local browser, service, install or full matrix was started. Detailed item routing/crossings remain #220; no main, hosted or release readiness claim.

@mvalancy mvalancy left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent bounded re-review of exact 8698e92 (four-path delta from reviewed 6ba55f2): the minimap P2 is corrected; no new source finding in this delta.

The event calls only the existing position/type publisher after organization, preserves the closed-map reopen path, and removes its listener on effect cleanup. My exact-callback probe with the real d3-force loop confirms one immediate complete snapshot and no post-cleanup publication. I independently inspected the author-run fail-before/pass-after reports, organized JSON and actual PNG: old source fails the new coordinate equality predicate; corrected desktop passes 1/1 in 19.300 s, all nine coordinates match, and existing save/reload/paint/camera assertions remain intact. The 4.5 s wait deliberately crosses the existing finite 4 s startup polling window; it is not an interaction-timeout relaxation.

Full reviewed application composition remains based on accepted dev 17acc25. This is source-correction clearance only: the previous 6ba hosted run is superseded, fresh exact-head hosted gates/artifacts and the planned PR236 composition still require review. No local browser, service, install or full matrix was started for this review. Detailed routing #220 and previously recorded release/hosted limits remain open.

@github-actions

Copy link
Copy Markdown

🧪 Comprehensive Test Suite

  • Unit suites (Node 22.18.0 & 24.x) — core, web, server, mcp-server: ✅ passed
  • Installer & deploy config: ✅ passed

Full-stack smoke gate runs in the CI workflow.

@mvalancy mvalancy left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of 3abae8a..cf58c97 (8ee27ea and cf58c97). This is a comment review only: no approval and no merge recommendation.

Blocking (1): 8ee27ea relaxes a guard to match a regression this PR introduced. Inline on semantic-lod.spec.ts L184.

I replayed the hosted 1440 px geometry offline. Current code reproduces the hosted run exactly: 11 pairs and 60 of 361 originals drawn. With only overviewSlots swapped back to its pre-b72a87e version, the same viewport, controls and router draw 14 pairs and 141 of 361 originals.

b72a87e's phone-motivated phase search shifts the desktop grid to 4 rows. That adds no capacity for 8 groups but halves the drawn relationships on this fixture. The floor of 13 was carried over from a measurement taken before that change. Please fix the slot selection (or choose by routed coverage) and restore the floor, or record the trade-off explicitly with these numbers if the owner accepts it.

Non-blocking (1): cf58c97 also defers the item pass during user gestures (wheel, pinch, drag), not only programmatic transitions. Inline on useItemPresentation.ts L54. The lifecycle checks out: end fires on interrupt, the flag resets on rebuild, and I found no path to a stuck flag or a missed render. The fix addresses the stated double pass (2 → 4 presentation passes) in the 2k gate.

Checked:

  • useItemPresentation.test.tsx passes 4/4 with one worker.
  • The offline router and layout replay above. The router ceiling is 12 pairs with caps removed, which confirms the author's replay.
  • Hosted run 42394922572 at 8ee27ea: the item-wires sequence passed, including the new first-phone-tap regression. The 2k gate failed and the later sequences did not run, so the new-user gate is still unconfirmed on hosted CI.
  • CI 42397389459 on cf58c97 was still running when I posted.

No browser or Playwright runs were done locally.

Comment thread tests/e2e/graph/semantic-lod.spec.ts Resolved
mvalancy and others added 2 commits November 7, 2026 09:56
Review follow-up: a held pinch or continuous wheel that leaves the
overview must re-render cards when it crosses the threshold, not when
the gesture ends. Only transitions without a source event (minimap,
fit, focus) defer to their single terminal pass.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
b72a87e let the overview shift its slot rows to the phase that yields
the most slots, which fixed a phone viewport but also moved the desktop
grid: with the control bar over the first row the shifted phase spread
eight groups over four rows and drew 11 of 38 group pairs and 60 of 361
relationships instead of 13 and 101. The regular grid is now kept
whenever it already holds every card being placed; the phase search
still runs when it does not (the phone case) and for capacity.

Restores the desktop drawn-pair floor to 13 (reverting 8ee27ea's 11) and
also asserts drawn relationships (101 desktop, 45 phone) so a layout
cannot keep the pair count while dropping heavy buses. Unit test fails
on the previous slot code (24 px row offset). The group browser now sits
on z-30 like the relationship browser in the same place, so the status
dock no longer intercepts taps on its last rows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

🧪 Comprehensive Test Suite

  • Unit suites (Node 22.18.0 & 24.x) — core, web, server, mcp-server: ✅ passed
  • Installer & deploy config: ✅ passed

Full-stack smoke gate runs in the CI workflow.

@mvalancy mvalancy left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of cf58c97..ae47b26 (c43a6db and ae47b26). This is a comment review only: no approval and no merge recommendation.

No blocking findings remain in this delta. Both threads are fixed; there is a reply on each.

  • r4503068132, overview slot grid: fixed. Replaying the hosted 1440 px geometry offline gives 14/38 pairs and 141/361 originals drawn (cf58c97: 11 and 60). Phone 390 px is unchanged at 4 pairs and 45 originals, with card positions identical to the hosted run. A sweep over 2–18 groups on both geometries never shows fewer cards than before. It keeps 3 rows up to the regular grid's 11 slots and shifts exactly as before for 12–14 groups. The one small coverage dip is at 10 groups (14 vs 16 pairs).
  • r4503068139, item-pass deferral: fixed. The deferral now covers only programmatic transitions. User gestures interrupt any running transition and never set the flag, and the 2k gate's minimap-driven zooms keep the single pass.

Checked:

  • semanticLOD.test.ts, useItemPresentation.test.tsx and SemanticGraphOverview.test.tsx pass with one worker (33/33).
  • The group browser's new z-30 matches the relationship browser in the same position.

Pending hosted gates: the new-user gate (first phone tap), semantic-LOD acceptance (the restored 13/101 floors) and performance budgets have not yet run on hosted CI after these fixes. The 2k gate passed at cf58c97. CI 42398756458 and Comprehensive 42398756445 on ae47b26 were running when I posted. I ran no browser or Playwright tests locally.

…ads better

Hosted new-user acceptance failed at 844×390: two unconnected items were
packed with the fixed 3:2 row width, so they stacked vertically in a
landscape phone viewport and the fit settled at 0.48 with compact,
three-line titles. Before 720cbef the test only passed because it caught
the unfitted first frame that the camera then jumped away from (the
first-tap bug); the settled view was already shrunk.

Packing now also tries the viewport's own aspect and keeps it only when
it gives a strictly larger fit scale. At 844×390 both cards sit side by
side at 1.03 with full single-line titles; portrait phones and desktop
keep the existing packing. Unit test fails on the previous packing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@mvalancy mvalancy left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of ae47b26..337fca4 (the viewport-aspect component packing in organizeTopology). This is a comment review only: no approval and no merge recommendation.

No blocking findings. The packing logic is correct. The alternative packing is tried only with two or more components and kept only for a strictly larger fit. At 844×390, two loose cards now sit side by side instead of stacked and shrunk, which matches the author's diagnosis of the new-user-modes failure.

Gates that depend on this layout:

  • The 301-item overview is one connected component, so it is unchanged. Replaying the hosted 1440 px geometry gives 14/38 pairs and 141/361 relationships, which clears the 13/101 floors. The hosted 390 px replay gives 4 pairs and 45 relationships, also unchanged. A connected-group sweep over 2–18 groups shows no layout difference against ae47b26.
  • The first-tap and responsive fixtures use two loose items, which still stack in portrait.

Non-blocking (1), inline on L457:

  • Portrait phones with 3 or more loose components now stack in a single column. This contradicts the commit message, although every case I probed fits better.
  • Multi-component overviews on desktop also change: 29 of 275 synthetic layouts changed, with the same card counts and coverage within about ±7% (worst −1 pair and −27 relationships). No gate covers that case.

topologyOrganization, overviewOrganization and semanticLOD units pass with one worker (32/32). The hosted new-user, semantic-LOD and performance gates on this head have not run yet. I ran no browser or Playwright tests locally.

@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

🧪 Comprehensive Test Suite

  • Unit suites (Node 22.18.0 & 24.x) — core, web, server, mcp-server: ✅ passed
  • Installer & deploy config: ✅ passed

Full-stack smoke gate runs in the CI workflow.

Review follow-up: 337fca4's message said portrait phones keep the old
packing, which holds for two loose cards but not three or more. Those now
form one column (390×844, three 240×140 cards: fit 0.54 -> 1.06). Make
that behaviour explicit in the unit test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

🧪 Comprehensive Test Suite

  • Unit suites (Node 22.18.0 & 24.x) — core, web, server, mcp-server: ✅ passed
  • Installer & deploy config: ✅ passed

Full-stack smoke gate runs in the CI workflow.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

🧪 Comprehensive Test Suite

  • Unit suites (Node 22.18.0 & 24.x) — core, web, server, mcp-server: ✅ passed
  • Installer & deploy config: ✅ passed

Full-stack smoke gate runs in the CI workflow.

Conflicts: dev's dd87b87 moved camera fitting into lib/graphFit
(computeGraphFit, measuring cards at the fitted scale). Resolved by
fitting through computeGraphFit and keeping this branch's organization
floor (stay centred at the readable minimum scale) and component focus
framing, measured with getNodeDimensionsAt at the final scale. Quick-edit
focus return keeps the local-item trigger first, then dev's
focusNodeControl. INDEX.md keeps both lesson rows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@mvalancy mvalancy left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of the conflict resolution in merge 7300420 (dev 7f7c5db into the branch), comparing 852f4a1..7300420 and git show --cc. This is a comment review only: no approval and no merge recommendation.

No findings. The resolution keeps both sides' intent. Only InteractiveGraphVisualization.tsx and .agents/issues/INDEX.md needed manual resolution.

fitViewToNodes:

  • Shared fit. Fitting now goes through dev's computeGraphFit, which measures cards with getNodeDimensionsAt at the scale it returns. The branch's obstacle filter is unchanged: passive save notes are ignored, and in-progress saves are ignored only under the Organize floor.
  • Organize floor. When the zoom floor is above the fitted scale, the code recovers the world point at the fit region's centre (the bounds centre) and re-centres it at the floor. This is the same transform the branch produced before (max(floor, fit) centred on the bounds), just expressed on top of the shared fit.
  • Focus framing. Framing still goes through frameOrganizedNeighborhood. It now measures cards with getNodeDimensionsAt at the final scale, so the floor and the framing see the same compact or full geometry. Empty obstacle rectangles are filtered before framing, as computeGraphFit does internally.

closeQuickEdit: the local-item [data-local-open] trigger is still tried first, then dev's focusNodeControl, which focuses the card itself when it is compact. The dependency list includes focusNodeControl.

The 720cbef pre-paint convergence loop works with the new fit. computeGraphFit already returns a fixed point for scale-dependent card geometry, so the loop now converges on its confirming second pass. I probed 168 cases (6 viewports, 1–40 cards, linked and unlinked, organized by organizeTopology) with dev's layout-key reuse rule: at most 2 fit passes, never an oscillation. The loop is now redundant but harmless, and it still sets the presented scale before the first refresh().

Other merged files:

  • graphSymbolLayout.itemTitleScale is a pure extraction, so layouts are unchanged.
  • unified.config.mjs takes dev's moved schematic config path and keeps the branch's additions to semantic-lod (local-item-lod) and item-wires (topology-organization).

Checked locally: graphFit, topologyOrganization, organizationCamera, LayoutSaveStatus and useItemPresentation units pass with one worker (28/28). I ran no browser or Playwright tests. The author's local 31/31 Playwright run is not independently verified here. CI 42411726205 and Comprehensive 42411726259 on 7300420 were running when I posted; the previous merge head 852f4a1 passed both.

@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

🧪 Comprehensive Test Suite

  • Unit suites (Node 22.18.0 & 24.x) — core, web, server, mcp-server: ✅ passed
  • Installer & deploy config: ✅ passed

Full-stack smoke gate runs in the CI workflow.

mvalancy and others added 2 commits November 7, 2026 13:25
…prop

Hosted semantic-lod failed at 390px after reload with every local card
at x=3844 (also seen once on an earlier head). Reproduced 4/4 locally
with 4x CPU throttling: the layer activated at the initial identity
camera, the saved camera was restored and its settled event correctly
rebased the layer, but a deferred two-frame rebase then fired before
React re-rendered the layer with the restored camera prop and reset the
origin to the stale identity camera. Cards stayed shifted by the whole
restore (3489, 3636 px).

The layer's latest-camera ref now takes the settled camera directly and
only follows the prop when the prop itself changes, so a deferred rebase
cannot read the pre-restore camera. Throttled 390px journey passes 3/3;
the new unit test fails on the previous code (translate -356, -110).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@mvalancy mvalancy left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of 7300420..5e76858: fix 7f91b57, plus the merge of dev 150125c, which only touches docs, CI and the public-content guard. This is a comment review only: no approval and no merge recommendation.

No findings. The fix matches the diagnosis, and I found no new stale-camera path.

Can cameraRef get ahead of the prop and break delta? No, not in a lasting way:

  • delta is still computed from the rendered camera prop and origin.current. cameraRef only feeds origin in three places: the two-frame rebase, board "Explore", and history entries.
  • The IGV zoom handler calls setCurrentTransform with a new object on every zoom event, and end (the settled event) fires after the last zoom event. So a settled camera is never older than the newest prop, and the prop always catches up to the same values. The only way cameraRef can be newer is the gap between settled and that re-render.
  • During that gap the layer can be off for at most one render. This happens when a rebase sets origin to the newer camera while the prop still lags. It corrects itself when the prop arrives. Before the fix, the error persisted: origin was set back to identity, which produced the 3489/3636 px offsets.
  • Manual gestures emit no settled event until they end, so mid-gesture cameraRef follows the prop exactly as before. At gesture end, the manual early-return path only updates the ref, which equals the final prop anyway. The off-screen session reset is unchanged.

Board "Explore" and history: both now read the newest real camera, which is correct and slightly better than before:

  • "Explore" rebases on the camera actually shown.
  • navigate stores { ...session, camera: cameraRef.current }, which onNavigate restores exactly.

A remount on scope change re-seeds both refs from the prop.

Tests:

  • LocalItemNeighborhood.test.tsx passes 7/7 with one worker.
  • Reading the old code through the new test's sequence, the deferred rAF rebase sets origin back to the stale identity, and the re-render with the restored prop then leaves a non-zero translate. So the test is a valid negative control, as the author reports. I did not execute it against the old source.
  • The merged public-content-contract.mjs guard passes on this head.

I ran no browser or throttled Playwright tests. CI 42415792389 and Comprehensive 42415792345 on 5e76858 were running when I posted.

@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

🧪 Comprehensive Test Suite

  • Unit suites (Node 22.18.0 & 24.x) — core, web, server, mcp-server: ✅ passed
  • Installer & deploy config: ✅ passed

Full-stack smoke gate runs in the CI workflow.

CI lint (no-undef) rejected FrameRequestCallback in the new
LocalItemNeighborhood regression. Repo-wide `npm run lint` now reports
0 errors.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

🧪 Comprehensive Test Suite

  • Unit suites (Node 22.18.0 & 24.x) — core, web, server, mcp-server: ✅ passed
  • Installer & deploy config: ✅ passed

Full-stack smoke gate runs in the CI workflow.

1 similar comment
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

🧪 Comprehensive Test Suite

  • Unit suites (Node 22.18.0 & 24.x) — core, web, server, mcp-server: ✅ passed
  • Installer & deploy config: ✅ passed

Full-stack smoke gate runs in the CI workflow.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

🧪 Comprehensive Test Suite

  • Unit suites (Node 22.18.0 & 24.x) — core, web, server, mcp-server: ✅ passed
  • Installer & deploy config: ✅ passed

Full-stack smoke gate runs in the CI workflow.

@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

🧪 Comprehensive Test Suite

  • Unit suites (Node 22.18.0 & 24.x) — core, web, server, mcp-server: ✅ passed
  • Installer & deploy config: ✅ passed

Full-stack smoke gate runs in the CI workflow.

@mvalancy

mvalancy commented Nov 8, 2026

Copy link
Copy Markdown
Member Author

Maintainer verification of b9cf59f (owner-delegated merge approval)

  • CI on this head, dev with Installer installs main (or GRAPHDONE_REF), never a deleted branch #247 and Release version 0.4.0-alpha #248 merged in: 18 pass, including the 4 required checks and the full Smoke Gate. The prod job is main-only. 0 unresolved threads; 0 commits behind dev.
  • The new browser specs really run: compared with the latest green dev push, the Smoke Gate's item-wires sequence goes from 7 to 13 passing (topology-organization.spec.ts, wired in unified.config.mjs) and semantic LOD from 2 to 4 (local-item-lod.spec.ts). Together they hold 160+ expects.
  • Local, fresh clone merged with dev, Node 22:
    • web Vitest 543/543 (73 files);
    • web tsc --noEmit clean;
    • npm run lint clean;
    • test:unified:lib 289/289;
    • toolchain contracts pass, including the public-content and version contracts.
  • Scope and safety: about 7.1k added lines, roughly half of them tests. All web front end; no server, auth or data-path changes. The added lines contain no innerHTML/dangerouslySetInnerHTML, eval, new fetch, storage or postMessage use.
  • Review: the independent review's findings are each fixed with a regression test (first phone tap, drawn-pair and link floors, 2k-zoom budget, loose-item packing, delayed rebase after reload), and the threads are resolved.
  • Known follow-up, accepted and recorded in Organize graphs around their topology with readable card and wire clearances #233 for LOD-WIRE-2: carry clean ports and buses into detailed item wiring #220: disconnected-cluster desktop overviews draw slightly fewer links (worst sampled 379 vs 406).

Merging. #233 closes after the post-merge dev CI is green.

@mvalancy
mvalancy merged commit dcde00a into dev Nov 8, 2026
19 checks passed
@mvalancy
mvalancy deleted the feat/topology-organization-20261028 branch November 8, 2026 00:16
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.

1 participant