Conversation
ericpgreen2
left a comment
There was a problem hiding this comment.
This is a nice upgrade from the raw JSON dump, and the per-type structured views are thoughtfully done. But before we invest in polishing individual components, I want to raise a concern about the overall approach.
This PR adds 14 files and ~1,200 lines of bespoke per-type UI. Each resource type gets a hand-crafted Svelte component that renders specific fields from the proto definition. The maintenance concern is: when someone adds a field to V1MetricsView, V1Model, or any other resource type, they'll need to remember to update the corresponding *Describe.svelte component. In practice, they won't. This surface will silently drift out of sync with the actual resource specs, and there's no automated way to catch it.
The audience for this feature is data engineers and Rill support staff on the Project Status page. They're debugging resource configuration and runtime state. They can read structured data.
Some alternative approaches that would reduce maintenance burden:
-
Show the YAML source file. Every resource has
meta.filePaths. Render the actual source file with syntax highlighting, plus a small section for runtime state fields (refreshedOn, rowCount, etc.) that aren't in the YAML. Zero per-type maintenance, always in sync with the source of truth. The trade-off is less "human-readable" formatting (no friendly labels, color swatches), but for this audience that's probably fine. -
Improved generic renderer. Instead of 10 bespoke components, build one good structured JSON/YAML viewer with collapsible sections, syntax highlighting, and a few type-aware formatters (dates, byte sizes, colors). Covers all resource types, including future ones, with no per-type code.
-
Data-driven mapping. Replace per-type components with per-type mapping functions that produce a flat list of
{section, label, value}tuples. One generic component renders the list. The mapping functions are pure data transforms, easier to test and maintain than full Svelte components, and the rendering logic is shared.
I'd lean toward option 1 for its radical simplicity, but any of these would significantly reduce the long-term maintenance surface.
Developed in collaboration with Claude Code
|
▎ I've reworked this to use a generic structured renderer (your option 3). One mapResource() function recursively walks any resource's .spec and .state objects and produces {section, label, value} tuples. A single StructuredView.svelte component renders them. New proto fields appear automatically with no code changes. ▎ For option 1 (showing the YAML source file): the runtime's GetFile API requires ReadRepo permission, which cloud JWTs don't grant. There's no admin-side API that proxies file reads from the deployment's git repo either — GetVirtualFile only covers UI-created files, not GitHub-sourced project files. So source file viewing isn't feasible on cloud today without a backend change. I've left it out for now but it would be a nice follow-up if we add a cloud-compatible file read API. |
# Conflicts: # web-admin/src/features/projects/status/resource-table/ProjectResourcesTable.svelte
Svelte 5 migration:
- ResourceSpecDialog: $props(), $derived, callback props, onclick, direct
dynamic components, aria-label on back button
- StructuredView: $props(), $derived, $effect, $state, plain function for
hasGroups
Bug fixes:
- Fix off-by-one in firstMonthOfYear formatting (proto is 1-indexed)
- Remove dead LABEL_MAP entries that duplicate SKIP_KEYS
- Fix variable shadowing in flatten()
- Replace border-border-muted (nonexistent token) with border
- Consistent spacing between grouped/ungrouped items
- Add {#each} keys on all lists
- Add aria-hidden on decorative SVG chevron
- Convert on:view-component and on:back to callback props in consumer
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
royendo
left a comment
There was a problem hiding this comment.
Code review fixes pushed
Implemented
-
Bug: Off-by-one in
firstMonthOfYear— Proto uses 1-indexed months butMONTH_NAMESis 0-indexed. Fixed withMONTH_NAMES[idx - 1]. -
Svelte 5 migration of
ResourceSpecDialog.svelte(new file) —export let→$props(),$:→$derived,createEventDispatcher→ callback props (onback,onviewcomponent),on:click→onclick,<svelte:component>→{@const Icon}, addedaria-labelon back button. -
Svelte 5 migration of
StructuredView.svelte(new file) —export let→$props(),$:→$derived/$effect/$state, reactive function → plain function. -
on:view-component/on:back→ callback props inProjectResourcesTable.svelteconsumer. -
Dead
LABEL_MAPentries removed — 6 keys that duplicatedSKIP_KEYSwere never reached. -
FLAT_ARRAY_KEYScomment added — Explains scaffolding intent for the empty set. -
Variable shadowing fixed —
objrenamed torecordinsideflatten()to avoid shadowing the parameter. -
border-border-muted→border— Nonexistent design token replaced with the default border token (2 occurrences). -
Consistent item spacing — Grouped items now match ungrouped:
w-36/gap-x-4/min-h-[20px]. -
{#each}keys added — All 4 loops keyed: sections by name, groups by group, items by label. -
Accessibility —
aria-hidden="true"on decorative SVG chevron,aria-labelon back button.
Not fixing
- No tests for
resource-mappers.ts— 572 lines of pure transformation logic. Good candidate for a follow-up PR with unit tests formapResource,formatValue,cleanEnum,prettyLabel. - No loading state for canvas component fetch — Minor UX concern;
fetchComponentResourcesis fast for typical canvas sizes.
Developed in collaboration with Claude Code
…viewcomponent` - Add 35 unit tests covering mapResource: sources, models, metricsView, canvas components, SKIP_KEYS filtering, enum cleaning, date/boolean formatting, firstDayOfWeek/firstMonthOfYear, and label mapping - Add loading state to StructuredView while fetching canvas component resources - Wire onviewcomponent callback from ResourceSpecDialog through to StructuredView with clickable "View" buttons on canvas component groups - Fix unused-vars lint error on onviewcomponent prop Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds a resource describe modal to the Project Status resources table. Clicking the "View spec" action on any resource opens a dialog with structured, human-readable details about that resource.
https://www.loom.com/share/f0de3b4aec6b47a5886720f9e6d5dd8f
Supported resource types:
/-/alerts/{name}and/-/reports/{name}detail pagesChecklist:
Developed in collaboration with Claude Code