Add spaday-based model registry browser - #249
Conversation
Test Results 1 files ± 0 1 suites ±0 2m 58s ⏱️ ±0s Results for commit ddb5625. ± Comparison against base commit bb4ab7e. This pull request removes 67 and adds 162 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #249 +/- ##
==========================================
+ Coverage 93.59% 93.78% +0.18%
==========================================
Files 176 190 +14
Lines 20613 21499 +886
Branches 1361 1410 +49
==========================================
+ Hits 19292 20162 +870
- Misses 1048 1062 +14
- Partials 273 275 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
7624af2 to
9a5bcec
Compare
9a5bcec to
de7f4da
Compare
238dc5a to
a40c136
Compare
de7f4da to
06586d8
Compare
06586d8 to
03eac9c
Compare
|
@timkpaine IS this ready for review yet? |
timkpaine
left a comment
There was a problem hiding this comment.
dagre rendering a bit messed up, also its global and not model-local so it looks a bit weird
materializing a model collapses the tree, need to fix
Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
Replace the hand-built wa-tree with spaday-trees, which derives the hierarchy from registry paths and brings its own search box. Add a Dependencies tab rendering the registry dependency DAG with spaday-dagre; clicking a node selects that model, so the graph shares the sidebar's selection state. Registered names are root-relative and leading-slashed, so they are normalized before matching leaf paths. Bridge the tree's color-scheme to the wa-dark page theme and add a dark toggle. Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
Rename the server bind option from --address to --host. Make the dependency graph model-local: each model's card gets a Dependencies tab showing only what is reachable from that model, with the focused node marked, instead of one global graph in a page-level tab. Models with no dependencies get no tab. Bind the tree's selected_paths to the seeded selection so materializing a model reveals it again after the redirect instead of leaving the tree collapsed. Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
Label nodes with the full registry path so the hierarchy an entry comes from is visible. Point edges from a dependency to the model that uses it, so the chain reads in dataflow order and the inspected model is the last node. Replace navigate-on-click with a context menu: a pointer event on a node opens a popup naming it, and its Open model action both selects the model and reveals it in the sidebar tree. Each node carries its own menu body with literal actions, because the action DSL cannot build the single-element list the tree's selected_paths needs from a store value. Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
3e6952b to
5197c01
Compare
Materialize a lazy model in place: the button posts to /materialize and refreshes the tree, instead of a form post and a redirect that reloaded the page and collapsed the sidebar. Failures were previously visible only in the server log; the endpoint now returns its error and a toast reports it. Route detail cards with Switch instead of one Show per model, and defer each card to /card so only the visible one is fetched. A 500-model registry drops from 1.6 MiB of tree.json to 129 KiB. Bind the selection to a query parameter, so a model is linkable and back/forward navigate between models, and derive the tree's reveal from it so a deep link expands to the model. Remember the theme with persist. Browse with spaday-trees, badging models that are still configuration, and give the dependency graph full registry paths as labels, edges that point from a dependency to its dependent, and a node context menu in place of navigating on click. Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
5197c01 to
7b03470
Compare
A leading-slash path resolves against the process-global root registry, so the materialize endpoint could instantiate models outside the registry being served. Only paths the page itself lists are accepted now, and the endpoint requires a JSON object body with a matching content type so it cannot be driven by a cross-origin form post. Also resolve dependency names against every alias in a group rather than only the first, so edges survive when a subregistry is served, and re-check the loaded state when a lazy entry is materialized between the two registry lookups. Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
The console script passes no hydra.main() defaults, so a bare invocation left resolve_config_paths to raise ValueError and print a traceback. Route it through argparse instead, so the command exits 2 with its usage message. Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
The console script has no hydra.main() defaults to fall back on, so it demanded --config-path even though --config-dir plus --config-name already identifies a usable config. Resolve the config dir the same way load_config does and use it as the root when no --config-path is given, leaving it off the search path so it is not added twice. Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
Compare the materialize content type case-insensitively, since media types are case-insensitive and an uppercase spelling was rejected with a spurious 415. Skip a lazy registry entry that is neither loaded nor pending, which happens when it is removed between the two lookups and previously reached dependency_edges as None. Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
ptomecek
left a comment
There was a problem hiding this comment.
Approved. The spaday registry browser is cleanly structured (cli/registry/model/graph) with matching test coverage — all 86 spaday tests pass locally.
The latest security fix (restricting materialization to the served registry) is correct and important: the content-type + JSON-object + leaf-path allow-list closes the leading-slash root-registry escape and blocks cross-origin form-post CSRF. The dependency alias resolution and lazy re-check race fixes are sound.
Both review nits (case-insensitive content-type, and skipping a lazy entry removed between lookups) were addressed in ddb5625.
Backwards compatible: no released CLI changes ([project.scripts] was empty on main; ccflow-ui-spaday is new, and --address→--host only touches the unreleased spaday CLI), and the old ccflow.ui.model/registry/cli modules remain as compatibility shims re-exporting all public symbols.
No description provided.