Conversation
Contributor
Author
eb273c2 to
daca576
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds user-selectable graph layouts to the shared viewer and wires the feature through the full stack: viewer state, persistence, query/view metadata, Cytoscape layout engines, schema validation, tests, and docs.
Closes #39.
The feature now supports these layout IDs:
fcoseelkcoladagreconcentricThe control is exposed in the viewer toolbar, relayouts the current graph in-session, and persists the user's explicit choice per repo.
What Changed
1. Added an explicit layout model in PureScript
A new
Layoutmodule defines the canonical layout IDs, labels, parsing, default selection, and the source of the current effective layout (SavedExplicit,ViewDefault,Fallback).This keeps layout precedence explicit in PureScript instead of burying it in the Cytoscape FFI.
2. Extended viewer state and behavior
The viewer now tracks:
Behavior changes:
3. Added dedicated layout persistence
This PR deliberately does not change the existing title-keyed persistence for selected node, depth, and tutorial progress.
Instead it adds a separate localStorage entry:
graph-browser:layout:<identity>The identity is derived to stay repo-scoped in app mode:
owner/repofrom raw GitHubbaseUrlwhen possiblebaseUrlconfig.sourceUrlconfig.titleThat keeps explicit layout choices repo-specific without broadening the scope of the older persistence contract.
4. Added authored layout defaults to both view mechanisms
Both view entry points now accept an optional
layoutfield:data/queries.jsondata/views/*.jsonInvalid layout strings are ignored at decode time rather than failing the whole file.
5. Added Cytoscape layout engines and runtime switching
The bundle now registers:
cytoscape-fcosecytoscape-elkcytoscape-colacytoscape-dagreThe FFI keeps one active layout concept and uses it for both fresh renders and direct relayout requests. Each layout gets conservative default options tuned for this viewer so switching is immediate and doesn't require data reload.
6. Updated schemas, prompt-builder context, and README
The JSON schemas now validate the optional
layoutfield for both query-backed and legacy views.The prompt-builder guidance and README now document:
7. Added and extended tests
The PR adds decode coverage for both query-backed and legacy view layout metadata, while preserving existing integration tests over the self-graph.
Design Choices
Precedence rule
The effective layout order implemented here is:
fcosefallbackThis matches the spec and keeps authored defaults useful for first-time visitors without overriding a deliberate user choice.
Separate persistence instead of mutating
PersistedStateI kept layout preference storage separate from the older selected-node/depth/tutorial restore path. Re-keying the older persistence model would have expanded the risk of this change well beyond the layout feature.
Canonical repo identity in app mode
The spec work initially described
baseUrldirectly as the primary identity input. During implementation I tightened that to canonicalowner/repowhenbaseUrlis a raw GitHub URL, so branch previews share the same repo-scoped layout preference instead of fragmenting by branch ref.The spec artifacts in
specs/012-layout-picker/were updated to reflect that implementation decision.Files to Review
Core implementation:
src/Layout.purssrc/Viewer.purssrc/Viewer/Controls.purssrc/Persist.purssrc/FFI/Cytoscape.jssrc/bootstrap.jsData/model/schema:
src/Graph/Query.purssrc/Graph/Views.pursschema/query-catalog.schema.jsonschema/view.schema.jsonTests and docs:
test/Test/QueryCatalog.purstest/Test/ViewDecode.pursREADME.mdspecs/012-layout-picker/*Verification
Executed locally:
nix develop -c just testnix develop -c just bundle-appnix develop -c just bundle-libResults:
30/30)Follow-up
I did not deploy a preview in this PR because the repo does not define a concrete
surgetask or checked-in preview workflow. If we want a Surge preview here, that should be added as an explicit, repeatable command rather than inferred ad hoc.