Repository navigation
Prioritize displayed relationships with connected-pair fallback - #224
Conversation
🧪 Comprehensive Test Suite
Full-stack smoke gate runs in the CI workflow. |
🧪 Comprehensive Test Suite
Full-stack smoke gate runs in the CI workflow. |
🧪 Comprehensive Test Suite
Full-stack smoke gate runs in the CI workflow. |
1 similar comment
🧪 Comprehensive Test Suite
Full-stack smoke gate runs in the CI workflow. |
🧪 Comprehensive Test Suite
Full-stack smoke gate runs in the CI workflow. |
mvalancy
left a comment
There was a problem hiding this comment.
Independent review of the relationship-priority delta through 5aaf6ec7e488ae074ccfdc3132f83dd59a655f03, against stacked base 38a5e02. I did not write this change, edit the checkout, start a browser/service, push, approve or merge.
Two verified minor interaction findings remain: closing a pair browser after switching to All can target a connected but no-longer-focusable SVG label (thread4404185217), and the small-graph Links control bubbles into the document background handler and dismisses the current node/editor selection (thread4404202776). I reproduced the first DOM transition and the second using the extracted production handler in small JSDOM probes. The original lint blocker is fixed in5aaf6ec; its exact-head quality job now passes.
The core policy is coherent: original unordered endpoint-pair selection precedes group aggregation; higher tiers and fallback preserve ties/directions/IDs, unknown types remain contextual, missing endpoints partition separately, and broader levels monotonically add represented records. Canonical inputs still feed hierarchy/physics/editing. The represented set drives paths, hitboxes, arrows, labels and bundle admission together. I inspected the primary-label explanation, exact pair browser, keyboard handler, persistence, live deletion fallback, separate group counts and bounded label/route ordering.
Verification: 41/41 scoped component/helper cases passed with one worker in9.20s using the existing dependency fixture; I confirmed matching hashes for13 relevant implementation/test files. The10 routing regressions passed serially in0.26s, including the deliberate spaghetti negative and blocker-first work budget. I inspected the new2,000-item/5,000-edge test source: it seeds500 explicit blocker/reverse-dependency pairs, covers all levels in item mode plus All/Essential overview, asserts pair coverage/count conservation/camera/API identity and retains geometry/timing attachments. This is source/test review, not a claim that the extended browser gate has completed. Final runtime artifacts are still required.
Address the two inline interaction findings and review their targeted regressions before acceptance. Keep #220 open for actual detailed obstacle routing/buses, and integrate only after #223 acceptance under the existing dev playbook. CI badges alone are not acceptance evidence.
🧪 Comprehensive Test Suite
Full-stack smoke gate runs in the CI workflow. |
mvalancy
left a comment
There was a problem hiding this comment.
Final independent code review of exact 5dc7686548a040e4854fcc5237bebe0b7dc86265: no remaining code findings in the relationship-priority delta. This supersedes the earlier head-specific summary; final integration/runtime acceptance is still pending.
Both interaction findings are fixed and have verification replies. I extracted the actual corrected callback and independently checked focusable, untabbable, disconnected and unrelated-focus cases. I also checked the actual control/outside-click handlers: state is preserved and the native default is not prevented. The new browser test exercises the previously missing pair→All→Back focus sequence and selected Gateway→pointer/native select→All/Essential inspection continuity at1440/390.
I inspected the focus5 log and passed marker (2/2 in25.0s), confirmed the headed preview runtime files exactly match5dc7686, and compared parsed test syntax to establish that the preview's unformatted executed test and committed formatted test are equivalent. The earlier focus4 invocation is not counted. I did not launch another browser or rerun the workstation matrix. The previous41 helper/component cases and10 serial routing cases remain valid for the unchanged policy/routing code; the new probes target only this correction. The repo lesson records the failures, correction, actual run and limits.
Final exact-source CI and the actual extended2,000-item/5,000-edge runtime artifacts still need inspection before acceptance. Do not infer those results from this source review or badges. Preserve the #223 predecessor/dependency and dev integration playbook; keep #220 open for detailed obstacle routing/buses. I made no checkout edits, commits, pushes, approvals, merges, browser/service changes or graph writes.
🧪 Comprehensive Test Suite
Full-stack smoke gate runs in the CI workflow. |
🧪 Comprehensive Test Suite
Full-stack smoke gate runs in the CI workflow. |
mvalancy
left a comment
There was a problem hiding this comment.
Independent correction review of exact 9b8409ceadae298e2890a619f490bf4dec170ba9: attachment retention is fixed; one newly visible screenshot/assertion gap remains in thread4404525139.
The full change from517742c is only the two test producers plus their lesson/index. Runtime source is unchanged. Both producers now write owned outputPath files and attach by path, preserving the collector contract. I read the actual1440px Playwright JSON: one expected pass, retry0, no skips/flakiness/global errors. Its3193-byte accounting JSON and61545-byte fallback PNG exist. The accounting proves four unchanged items, five→four original edges with only the explicit blocker deletion,4/5/3 level stages and a stable stage camera. I replayed the actual raw report through the real adapter with copies under an owned test-artifacts directory; JSON and image are retained without evidence errors. Executed and committed test syntax matches after ignoring formatting.
I then inspected the PNG itself: the graph is almost empty, with0 labels and only a clipped Store at the top-left. DOM priority attributes prove selection membership but do not prove visible fallback. Add readiness/framing plus actual visible dependency wire/label coverage before capturing; the code currently proceeds after reload without waiting for camera readiness again. I have not classified the empty image as a lasting renderer defect versus a framing race. The earlier passing2k step on517742c remains nonfinal; corrected exact-head CI and retained2k stage/accounting/geometry evidence still need acceptance review after this visual gap is addressed.
No checkout edits, browser/service calls, graph writes, pushes, approvals or merges were made in this review.
mvalancy
left a comment
There was a problem hiding this comment.
Independent correction review of 2679255 (base69d0493). No remaining finding in the reviewed correction. Only the fallback test and incident lesson changed from9b8409c; product/runtime sources remain identical to the reviewed517742c composition. I inspected the complete test delta, raw local report, accounting and geometry JSON, and both actual screenshots. The1440/390 cases passed without retries; the final captures now visibly prove the fallback and useful readable endpoints, with zero geometry violations. Original identities/directions/positions are preserved and the sole write is the intended blocker deletion. The executed test is syntax-equivalent to the committed formatted test. Earlier interaction/lint corrections and file-backed attachment corrections remain intact.
This is not final integration acceptance: fresh exact-source CI40748118917 and Comprehensive40748119284 are pending, including retained small-scene and2,000-node/5,000-edge stage/count/geometry/image evidence, clean source fingerprints, and all required gates. PR remains stacked on223; dev-only integration policy and post-integration CI still apply. No approval or merge performed.
🧪 Comprehensive Test Suite
Full-stack smoke gate runs in the CI workflow. |
mvalancy
left a comment
There was a problem hiding this comment.
Independent correction review of 377f5a3 against predecessor2679255 and stacked base69d0493: no remaining finding in this correction. Product changes are limited to (1) refreshing label placement when the relationship panel changes and (2) awaiting matching GET_EDGES observed-query refetches after the editor explicit Delete action. I inspected the installed Apollo document matching and verified the obsolete canvas-only mutation attempt was reverted.
Actual final typography16 report: both1440/390 journeys passed in25.808s with no retries/flaky/skips/global errors. I inspected both PNGs and geometry/accounting/typography files: readable fallback endpoints and Depends On +1, zero geometry violations, unchanged node positions and exact original records except the one BLOCKS deletion, stable priority-stage camera, and one DeleteEdges mutation only. Header and dock counts now both read4. Named computed samples require visible title14px/600, dock14px/500, Links16px/600; desktop header counts14px/500 are visible. Phone header counts are explicitly hidden and treated only as cache/style evidence; its deliberate12px/400 negative targets the visible dock. The final runtime files and formatted test byte-match the executed preview. The deterministic old-runtime obstacle14 failure and earlier count/cache failures are preserved in indexed repo lessons.
Fresh exact-source CI40751802514 / Comprehensive40751802494 is pending. This review does not claim final2,000-node artifact acceptance, full routing/bus completion, dev integration acceptance or GitHub approval. Final retained small/large evidence, source fingerprints, every required gate and post-integration checks remain required.
🧪 Comprehensive Test Suite
Full-stack smoke gate runs in the CI workflow. |
mvalancy
left a comment
There was a problem hiding this comment.
Final artifact audit of377f5a3 / CI40751802514: not ready to integrate. Two verified nested-LOD regressions are posted inline: incoming trace loses its sole reverse dependency after global priority filtering, and the390px trace loses room for its second group after the Links toolbar is added. I inspected the actual failed PNGs/traces; these are not merely badge failures. Nested reports two failed browser cases plus one process-evidence row. Layout recovery, router navigation, mobile, living effects and performance stages did not execute.
Successful retained evidence is real: item-wires7, large-symbols1, new-user42, schematic-visual4, semantic-lod2 and smoke6 all passed with exit0, no evidence errors or skipped cases. All seven emitted reports have the same clean unchanged source fingerprint4c7927d17440c286994eee661f27cd8384d999e9a93483312a6d5df17492b9a4 at synthetic mergec45e172, whose tree is identical to377f5a3. All90 referenced attachments exist and are nonempty.
The corrected small-scene accounting, PNGs and typography JSON are retained and inspected: exact deletion, unchanged positions, count4, readable fallback, visible14/500 dock and16/600 Links, phone hidden-header limit, and the visible12/400 negative. The actual2,000-node/5,000-edge file retains item levels4496→4998→5000→4496 IDs,4495 original pairs, stable Essential return and subset monotonicity; overview totals reconcile to5000. Overview geometry has8cards and zero clutter violations; item geometry has3visible cards/1label and zero symbol violations. Load15.536s, measured zoom2.52–2.79s, no writes/errors. I viewed overview, zoomed-out, All-detail and held-pan PNGs. Item wires remain straight and crossing in detailed scenes: this evidence does not close full item routing/buses issue220.
Runtime contracts and Node22/24 comprehensive suites passed separately; self-host scan inventories162 existing advisories, so no zero-vulnerability claim. Correct the two nested findings and rerun the final exact-source full gate before acceptance; prior stage success is supporting evidence only.
🧪 Comprehensive Test Suite
Full-stack smoke gate runs in the CI workflow. |
mvalancy
left a comment
There was a problem hiding this comment.
Independent correction review of 588a2c6, now targeted to dev4a6d75d: no remaining finding in the reviewed correction. The diff from377f5a3 is limited to directional priority scope, responsive control spacing/count layout, the component/browser regressions and indexed documentation. I checked that scope projection uses original member pairs before group routing/paging, retains lower-tier fallback for a different pair, accounts for out-of-scope originals, and restores the global selection on exit. No input or persisted level mutation is introduced.
Independent serial validation:3 focused suites,19 tests passed in3.23s using source-matched owned dependencies. Actual headed nested18 evidence:2/2passed in51.507s, no retries/errors; inspected all geometry and desktop/phone directional PNGs. Each trace has2cards/1painted route, zero card intersections/crossings/overlaps/diagonals/small text, and controls no longer cover one another. Incoming remains Essential and routes the original dependency toward Payments. All related runtime files and the final nested test byte-match the executed preview. The superseded green-but-overlapping nested17 scene remains rejected evidence. Both blocking findings have verified correction replies.
This is correction clearance, not final integration acceptance: fresh CI40757794024 and Comprehensive40757794023 still must pass, with retained final small/2k/nested/typography artifacts and clean source fingerprint. The full post-dev gate and repo merge rules still apply; no approval or merge performed. Item-level obstacle routing/buses remains issue220.
mvalancy
left a comment
There was a problem hiding this comment.
Independent final-source review and artifact audit: no remaining blocking finding for this PR's dev integration. This is a comment review, not a GitHub approval or release acceptance.
Reviewed source 588a2c6faf4fedf153b4b20215699f2061bae0b1 against dev 4a6d75d24ee664a0357aa124168f39c410625902. The tested merge 31f4a1c92c44ac025fc3d0a49393be21dc2fe4b0 has exactly those parents and the same tree as the source, f2b29082be5884c76cb797230232b4abd52322a7. Re-read current PR metadata and all eight review threads: all resolved. The prior correction review and verified inline replies remain applicable.
I inspected the completed CI run 40757794024, Comprehensive run 40757794023, downloaded final smoke artifact 12849760448, and checked its contents rather than treating a green stage as sufficient evidence:
- All 11 strict reports passed: 110 cases, no failed/warned/skipped cases, exit 0, no timeout/evidence errors. Every report starts/ends clean at the tested merge with unchanged fingerprint
456b2cc89f434b79c78021aca339a34af90c8642f2cecba53a2a7e098a76a4f5. All 93 referenced attachments resolve to nonempty files inside the artifact; no dropped body-only attachment or remaining source-path reference. Actual living-graph execution separately passed 3 cases in 51.3s. Recovery, routes, mobile, new-user and performance stages all executed on this final source. - Actual 2,000-node/5,000-edge evidence retains 4,496 → 4,998 → 5,000 → 4,496 unique IDs across Essential/Dependencies/All/Essential, with 4,495 connected endpoint pairs at each level, monotonic subsets and exact restored selection. Overview partitions total 5,000 (All: 760 drawn + 101 deferred + 4,139 internal; Essential: 758 drawn + 2 summarized + 101 deferred + 4,139 internal). No display-only writes or captured errors. Load 21.672s; measured zoom settling 2.65–4.15s, with wall times retained separately. I viewed final overview, zoomed-out, All-detail and held-pan PNGs: useful readable cards are actually present.
- Final desktop/phone priority JSON and PNGs show the painted Depends On +1 fallback and readable endpoints after deleting exactly the blocker. Source items/positions and other edge IDs remain unchanged; only one intended DeleteEdges call is recorded. Header/dock counts become four. Computed title 14px/600, dock 14px/500 and Links 16px/600 are retained, together with the rejected visible 12px/400 negative. The phone header count remains explicitly hidden; visible phone counter evidence comes from the dock.
- Final nested desktop/phone geometry and screenshots show two useful cards, clear reserved controls, and the original incoming direction into Payments while Links stays Essential. Outgoing accounts for 50 drawn + 132 internal + 1 priority summary; incoming for 1 drawn + 132 internal + 50 external-in-list = 183. All six retained scenes have one painted route and zero measured card intersections, unrelated crossings/overlaps, diagonals or undersized text. This closes both final blocking findings; the failed predecessor CI and rejected intermediate control-overlap screenshots remain relevant historical evidence.
I also checked the final runtime identity/readback/replacement contracts and scanner inventory against the tested image IDs. Scanner success is not a zero-advisory claim: self-host retains 162 advisories, including 43 HIGH, with no fixed HIGH/CRITICAL observed; api/generic had zero findings. Comprehensive unit logs include 26 existing server skips, despite zero skips in the strict browser reports. My independent focused component/helper run was 19/19 passed with one worker.
Limits remain explicit: detail-level item wires still cross, so #220 stays open; this does not prove 10,000-node browser capacity, production parity or release readiness. #225 owns the separate inspector typography exceptions. The main/nightly-only production stack job was intentionally skipped. A merge must still follow the repo's dev integration rules and receive post-dev acceptance on the actual merged commit. Review scratch and strict audit are at /tmp/core-pr224-artifact-review-588a2c6/; durable evidence is the linked GitHub run/artifact.
|
Independent post-dev acceptance audit: no new blocker found for this relationship-priority checkpoint. This is independent agent evidence under the repository's dev integration playbook, not a GitHub non-author approval or release clearance. I did not change source, merge, close issues, or run a local browser/database matrix. Provenance independently read back: live Post-dev CI 40764224407 and Comprehensive 40764224527 completed successfully on that exact merge, attempt 1. I inspected actual jobs, logs and downloaded artifacts, rather than relying on the badges. The skipped main/nightly production job and PR-only report job are non-applicable to this dev push.
Limits remain: #220 item routing is unfinished, and the large detail screenshots still show straight wires crossing other wires/cards. This does not establish universal overlay clearance, arbitrary-scale streaming, hosted parity or release readiness. Inspector typography exceptions remain #225. The failed predecessor Audit files, exact metadata, scripts and downloaded evidence are retained at |
Dense graphs should emphasize the relationships that matter most while retaining a connection for pairs with no higher-priority type. Links now offers Essential, Dependencies and All levels, with a persisted choice, stable camera and access to every original relationship.
Continues #220 and #193. Targets accepted dev4a6d75d after readability #223. This PR does not close #220: detailed item obstacle routing and buses remain unfinished.
Accepted source:
588a2c6faf4fedf153b4b20215699f2061bae0b1. Accepted on deva531823443e87c3bee776e5181637e4c43be7fbbafter native administrative PR merge under standing dev-only authority. Parents4a6d75d24ee664a0357aa124168f39c410625902+588a2c6faf4fedf153b4b20215699f2061bae0b1, treef2b29082be5884c76cb797230232b4abd52322a7, matching the tested composition. Post-dev CI40764224407 and Comprehensive40764224527 passed. Independent actual-artifact acceptance: 11 strict reports,110 passes,93 verified attachments,zero strict failures/warnings/skips/timeouts/evidence errors; 819 unit passes on each Node22/24 plus26 explicit DB skips,198 harness,10 geometry,9 real-Neo4j and3 runtime contracts. The162 self-host OS advisories remain explicit. This was an admin merge with independent agent review, not a non-author GitHub approval.Validation includes scoped component/helper and routing tests plus web types. Policy cases conserve 50,000 edges across 10,000 items and cover ties, unknown types, missing endpoints, determinism, monotonicity and member-pair fallback; this is not browser-capacity evidence. The new directional component regression failed on source 377f5a3, then 19 relevant cases passed on the correction. Final focused nested desktop/phone journeys passed 2/2 in 51.51s, zero retries/errors: two useful cards, actual incoming dependency with Essential unchanged, exact partitions, readable text, clear controls and unchanged source data. Types and targeted lint passed.
The desktop/phone priority journey checks all levels, persistence, exact pair inspection/editing, focus, explicit blocker deletion and visible dependency fallback. It retains actual computed typography and rejects a deliberately tiny 12px/400 counter; visible phone-counter proof comes from the Connections dock, because the header count is intentionally hidden. The headless 2,000-node/5,000-edge gate adds 500 blocker/reverse-dependency pairs and checks every pair, monotonicity, grouping partitions, useful visible content, bounded timing/work and no display-only writes.
Retained failures: source377's full CI failed two nested cases (incoming fallback and phone capacity); downstream gates did not run. Its successful small-scene, typography and 2,000-node/5,000-edge stages, with 90 attachments, were independently audited but did not establish merge acceptance. An intermediate local green run was also rejected after screenshot review found a control overlap. Neither failure is replaced by a retry-only claim.
Canonical causes/corrections are in
.agents/inbox/2026-10-28-trace-fallback-needs-directional-scope.md,2026-10-28-priority-controls-must-preserve-context.md,2026-10-28-browser-evidence-needs-files.md,2026-10-28-edge-deletion-must-refresh-subscribed-counts.mdand2026-10-28-preview-styles-need-rendered-verification.md.Limits: finite geometry can defer routes; every original remains inspectable. Detailed item paths still cross. This does not establish 10,000-node browser capacity, hosted parity or release readiness. Inspector-specific typography exceptions are in #225 / active PR #226. No shared CodeGraph library. Accepted branch/worktree can be removed after the verified merge; recover source from this PR. Design
docs/architecture/schematic-connections.md.