Graph improvements - #67
Merged
Merged
Conversation
The two graph pages showed the same radial tree but built it two different ways. /recipes/graph/ used the shortest-path spanning tree, while /recipes/graph/<id>/ still attached each node to its closest neighbour one ring up. That rule lets a path chain A -> B -> C with dist(B,C) = 2 even when dist(root,C) = 2, so the edge distances along a path sum to more than the node's real distance from the root and the displayed numbers lie. Measured over 12 root recipes in the current collection, 630 of 937 nodes (67%) had path sums that overshot their ring radius. - Generalise build_film_sim_tree into build_recipe_tree, taking a `candidates` sequence so each page decides which recipes qualify: recipes sharing a film simulation, or recipes within a maximum distance. - Add recipes_within_distance for the per-recipe candidate set, and drop build_recipe_graph along with RecipeNode/RecipeEdge/RecipeGraphData. - Rename the result types to RecipeTreeNode/Edge/Data. - Add RecipeTreeEdge.is_exact, set to False when no parent satisfies the shortest-path constraint and the node falls back to its nearest in-tree neighbour. Path sums through those edges genuinely overshoot, so callers can render them differently instead of the graph lying silently. - Find the constrained parent and the fallback parent in a single pass, removing the fallback branch's redundant distance recomputation. - Scan candidate parents in tree-insertion order rather than set order, so ties resolve identically on every request and the graph is stable. - Add build_recipe_neighbourhood so the per-recipe view goes through the application layer like the film-sim view already did, and extract _cyto_elements so both views serialise nodes and edges identically. - Delete build_all_recipe_graph, AllRecipeGraphData and AllRecipeNode, which no view called, plus their tests and the src/domain/images/graph.py re-export shim that had no importers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The info panel floated over the top-right of the canvas. Because the canvas
still spanned the full width underneath, the radial tree centred itself
partly behind the panel and the right-hand portion of the graph was unusable.
- Make the canvas and the panel flex siblings inside .graph-body, so the
canvas owns a known width and the tree centres in space it actually has.
- Keep the panel mounted at a fixed 390px at all times, toggling only its
inner selected-recipe section, so clicking a node never resizes the canvas
and re-centres the graph under the cursor.
- Move the film-simulation dropdown into the sidebar, leaving nothing
overlaying the canvas.
Both templates were about 800 lines each with the layout, node styling,
label placement, path finding, panel rendering, tooltip and the whole
compare overlay duplicated verbatim. They are now shells over shared assets:
- static/js/recipe-graph.js exposing RecipeGraph.init and replaceGraph,
which unifies the two existing "swap the whole graph" paths (the film-sim
filter and re-rooting onto a selected recipe).
- static/js/recipe-compare-overlay.js exposing RecipeCompareOverlay.open.
- static/css/recipe-graph.css for the shell, docked panel and tooltip.
- includes/graph_panel.html and includes/graph_tooltip.html for the markup.
Also fixed along the way:
- Normalise the root id to a string once at init. One template passed it as
a JSON number and the other as a string, so root comparisons were written
inconsistently between the two files.
- Render fallback edges (is_exact false) dashed, so a solid edge is a
promise that the path sums are truthful.
- Render the panel's leading template comment with {% comment %}. Django's
{# #} cannot span multiple lines, so it leaked into the page as literal
text, and as a bare text node in a flex row it collapsed the canvas to
zero width and the graph stopped drawing entirely. Covered by tests that
assert .graph-body holds no stray text and no comment reaches the page.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- Update ADR 002 to record that both views now share build_recipe_tree, including the measured scale of the old rule's error, describe the dashed fallback edge, and explain why the panel is docked rather than floating. - Correct the max distance from 6 to 7, matching the RECIPE_GRAPH_MAX_DISTANCE default in settings. - Note the dashed connections and the sidebar dropdown in the user-facing recipe graph guide. investigation/recipe_graph.md was rewritten to match, but investigation/ is gitignored so it is not part of this commit. The screenshots in docs/recipe_graphs.md still show the floating panel and the corner dropdown, so they need retaking. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The toggle appeared in four templates, with the same CSS copied into three places: inline in the explorer and recipe detail pages, and again in recipe-graph.css. Restyle it once as the segmented control from the design handoff and reuse it everywhere. - Add a .segmented-toggle component to brand.css, which already hosts .button--primary and is already linked by all four templates, so no new stylesheet links are needed. It also serves the two other segmented controls the redesign introduces. - Add recipes/includes/section_toggle.html taking active="explorer" or active="graph", and use it from the explorer, recipe detail and both graph pages. - Delete the three duplicated .sidebar-nav rule sets. - Keep the existing behaviour that the half representing the current page renders without an href, so it is not a link to where you already are. It is now a <span> rather than an <a> without href, which says the same thing in markup instead of only in CSS. Colours come from the existing brand tokens rather than the handoff's palette, so the accent stays consistent with the rest of the app. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Only 42 of 207 recipes have a name, so most nodes render as "#124" and the graph is hard to read. Add a filter that keeps just the named ones. The filter has to run server side. The graph is a spanning tree, so hiding nodes in Cytoscape would orphan their children and break it. Instead the tree is rebuilt from a smaller candidate set, which build_recipe_tree already supports, so it stays connected and its distances stay true. - Add named_recipes() to the domain, mirroring recipes_within_distance(). - Always keep the root, named or not. Eterna's most-used recipe is #124 with an empty name, so dropping unnamed recipes wholesale would leave that graph rootless. Keeping it also means toggling the filter never changes which recipe the others are compared against. - Add RecipeTreeNode.is_named and serialise it, so callers have the flag instead of sniffing the label for a leading hash. The label still falls back to "#<pk>" for display. - Both use cases take named_only and apply the filter to their candidates. - Both views read it from ?named=1, so the state is bookmarkable. Off by default: nothing is hidden unless asked for. Verified against the real collection that each filtered graph is still spanning (n nodes, n-1 edges) and that Eterna keeps its unnamed root. The UI control for this lands with the sidebar rebuild. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The sidebar held only the Explorer / Graph toggle and, on one page, the film-simulation dropdown. Fill it out per the design handoff. - Make it a column so the recipe list takes the leftover height and scrolls on its own rather than scrolling the whole sidebar. Width and padding stay in step with the explorer and recipe detail sidebars, so the column does not jump when moving between the Recipes sections. The handoff asked for a narrower 244px, which did jump. - Add a legend whose middle row swaps between "Selected to compare" and a muted "Pick one to compare" depending on whether a comparison is active. - Add the recipe list, one row per graph node, ordered most-used first with a label tie-break so the order is stable when counts collide, which they do for every unused recipe. - Surface the named-only filter from the previous commit as a "Named recipes only" switch. A new .toggle-switch component in brand.css carries it, modelled on the gallery's "Sort by rating" switch but slightly larger; the gallery and image detail pages predate it and still hold their own copies of those rules, so they can adopt it separately. - Restyle the film-simulation select and add section labels. Named and unnamed recipes look alike in the list. Showing every recipe means treating them as equally important, so the only rows that stand out are the reference and the compared one. Clicking a sidebar row now does exactly what clicking the node does: both call one selectRecipe() extracted from the Cytoscape click handler, so the two cannot drift apart. Row clicks are delegated from a wrapper element so the handlers survive the list being replaced. The list belongs to whichever graph is loaded, so the film-sim selector and the named filter share one loadGraph() path, and the JSON response carries the sidebar re-rendered from the same include the page uses. That keeps one source for the markup rather than rebuilding rows in JavaScript. Ordering lives in the use cases as recipes_by_usage so it is covered by application tests, and GraphRecipeRow formats the counts in the interfaces layer, since django.contrib.humanize is not installed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The comparison panel needs every property of both recipes with a changed flag, grouped into categories. get_path_deltas reports only what differs, so it cannot drive a panel that shows the whole recipe and filters down to the changes on demand. - Add PROPERTY_GROUPS, arranging all 18 recipe fields into the categories from the design handoff, plus a Monochrome group the handoff omits. Its sample was a colour recipe, but Acros and the other black and white simulations do carry those two fields. - Add get_recipe_properties for a single recipe and get_recipe_property_comparison for a pair, both returning grouped rows. - Rows carry the model field name alongside the display label, so the interfaces layer can map an icon to each without the domain knowing anything about icons. - Drop a field when neither recipe has a value for it, and drop a group left with no rows. That keeps the Monochrome group out of a colour recipe's panel with no special casing. - Extract the existing "value missing" placeholder into EMPTY_VALUE so the new rows and the existing diff rows agree. A test asserts every RECIPE_FIELDS entry belongs to exactly one group, so a field added later cannot be silently left out of the panel. Verified against the real collection: Classic Chrome yields 7 groups and Acros STD yields 8. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The panel made you diff a "reference" block against a separate "differences" block by eye. It is now one grouped, icon-led property list where a changed property shows both values inline, plus a connected-dot timeline for the hops between the two recipes. - Render the panel server side from a partial with two states: the reference recipe on its own, or the comparison. The page embeds it on load and re-fetches it on selection, so both states have one source and roughly 120 lines of DOM building leave recipe-graph.js. - Add an inline SVG sprite of the 15 property glyphs. Inlined rather than referenced as an external file because an external <use> target does not inherit the host document's CSS in every browser, and the rows need to drive stroke colour and per-state opacity. - Map recipe fields to glyphs in a template filter, keeping icon names out of the domain. The two white balance axes share the shift arrow, and the monochrome fields reuse the nearest glyphs since the design has none. - Add the All properties / Only changes filter, which hides unchanged rows and any group left empty. It uses a new .segmented-toggle--subtle variant: the design gives this filter a quieter treatment than the Explorer / Graph toggle, since it filters a view rather than navigating. Each group carries its own changed count so an emptied group can be hidden without relying on :has(). - Move "Compare images" into the header and give each card its own "View recipe" link, so which recipe a link points at is unambiguous. - Skip the delta breakdown for a single hop, where the property list above already says everything that changed. Replace RecipePathDeltas with RecipeComparisonPanel at recipes/graph/comparison/. Nothing consumes the old JSON shape now that the panel is server rendered, so its view, URL and functional tests go with it. The domain get_path_deltas still drives the timeline and is untouched, along with its integration tests. _root_fields_json and the old graph_panel.html include are likewise no longer reachable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- Describe the named-only filter, why it runs server side, and why the root survives it, in the investigation notes. - Describe the panel's two states, the property grouping and when the delta timeline is skipped. - Refresh the implementation map and test coverage tables, and note that the sidebar and panel are server-rendered fragments swapped in as HTML. - Cover the recipe list, the named-only switch and the reworked panel in the user-facing guide. The screenshots in docs/recipe_graphs.md predate both this redesign and the earlier docking change, so they still need retaking. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Node size was meant to show how much a recipe is used, but it did not: a
recipe with 97 images rendered the same as one with 1.
Two reasons, both fixed here.
The scale ran against the largest count in the graph, which is nearly
always the root and nearly always an outlier. A Provia root carries 2,444
images against 283 for the next recipe, so the root consumed the range and
the other 52 recipes were squeezed into 7.6px of it. The root is now left
out of the scale, which widens that band to about 62px. It needs no help
being found: it is red, centred, and floored at a minimum size.
Diameter also scaled linearly with the count, but the eye compares circles
by area, so that overstated the busy recipes and flattened the rest. The
ratio is now square rooted: four times the images gives twice the diameter.
For Provia, where the root has 2,444 images:
images before after
283 21.6px 80.0px
97 16.6px 52.6px
10 14.3px 26.4px
1 14.0px 17.9px
One consequence: the busiest non-root recipe now renders at the same size
as the root. Colour and position still distinguish them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Recipe graph: one correct algorithm, plus the sidebar and panel redesign
Why
The two graph pages drew the same radial tree but built it two different ways.
/recipes/graph/used a shortest-path spanning tree, while/recipes/graph/<id>/attached each node to its closest neighbour one ring up. That second rule lets a
path chain
A -> B -> Cwithdist(B,C) = 2even whendist(root,C) = 2, so thed=Nedge labels along a path sum to more than the node's real distance from theroot and the displayed numbers lie.
Measured over 12 root recipes on the current collection, 630 of 937 nodes (67%)
had path sums that overshot their ring radius. It is now zero.
The rest of the branch is the sidebar and comparison panel redesign from the
design handoff, which lands on top of that.
What changed
One tree algorithm.
build_recipe_treetakes acandidateslist, so each pagejust decides which recipes qualify (film simulation match, or within
RECIPE_GRAPH_MAX_DISTANCE). The old builder is gone, along withbuild_all_recipe_graphand thesrc/domain/images/graph.pyshim, which nothingcalled.
Where the shortest-path constraint cannot be met, the node attaches to its nearest
in-tree neighbour and the edge is flagged
is_exact=Falseand drawn dashed. A solidedge is now a promise that the numbers add up.
Named recipes only. Only 42 of 207 recipes have a name, so most nodes render as
#124. A sidebar switch filters to the named ones (Provia goes from 53 nodes to 6).It runs server side, rebuilding the tree from a smaller candidate set, because hiding
nodes client side would orphan their children. The root is always kept even when it
is itself unnamed, which Eterna's
#124root requires.Sidebar. Now carries the film simulation selector, a legend, and the recipes in
the graph ordered by image count. Clicking a row does exactly what clicking the node
does: both call one
selectRecipe().Comparison panel. One grouped, icon-led property list where a changed property
shows both values inline, an All properties / Only changes filter, and a
connected-dot timeline for multi-hop paths. Rendered server side from a partial, so
the markup lives in Django templates rather than being rebuilt in JavaScript.
Node sizing. Node area (not diameter) is now proportional to image count, scaled
against the busiest non-root recipe. Previously the root outlier consumed the whole
range and every other Provia recipe sat within 7.6px of its neighbours, so 97 images
looked the same as 1.