planner: oschema CRUD ergonomics — N+1 workspace load, dead _list stub, stale currency-delete comments #30

New issue

Open

opened 2026-06-15 13:03:02 +00:00 by sameh-farouk · 0 comments

sameh-farouk commented

2026-06-15 13:03:02 +00:00

Member

Surfaced as a byproduct of the hero_collab oschema architecture review (2026-06-15). Low priority — planner is shipped and works; these are ergonomics/clarity debt, not functional bugs. Filing so the lessons aren't lost.

Context

The migrated planner makes all 12 entities [rootobject]. The macro's generated _list/_find return sid-string arrays (not full objects) and _find takes an opaque filter. That's a fine fit for planner's typed-column storage (no data loss), but the workspace-scoped UI pays an ergonomic cost:

1. N+1 on every workspace load (perf debt)

crates/hero_planner_web/src/index.html:362-369 (listEntities) reconstructs each scoped list via _find (sids) then _get per item — its own comment: "Server *_list_full stubs return empty, so we reconstruct here." So opening a workspace is 11 entity types x (1 _find + N _get) round-trips. Parallelized, but still N+1 over the socket.

2. `_list` is a dead stub

crates/hero_planner_server/src/rpc/main_impl.rs:341-343 — generated _list returns vec![]. Nothing uses it; it's dead surface.

3. `_list_full` is mis-scoped

main_impl.rs:345-367 — iterates ALL workspaces with no scope param (and is capped at 200). Correct only for the top-level workspace_list; useless/footgun for scoped entities.

Lesson (transferable from the collab review): where a workspace-scoped full-object list is a primary access pattern, add an explicit custom entity_list(workspace_sid) -> [Entity] service method rather than relying on generated _find+_get. Removes the N+1. (Likely moot if the Dioxus rewrite #18 reworks this layer — so defer unless that slips.)

4. Stale/contradictory comments (cosmetic)

index.html:506-510, 522-523 still say currency-delete "cannot work" / "Best-effort". This was already fixed by bdb3935 (CurrencyRate.id now in the list output; server deletes by rowid via parse_sid). The delete works; only the misleading comments remain. Clean them up to avoid confusion.

Explicitly NOT recommended

Reversing planner's rootobjects (the collab review's all-custom conclusion) does not apply here: planner stores typed SQL columns (INSERT INTO swot_items (title, description, kind, ...)), so it has none of collab's blob/array/derived/side-effect/caller_id drivers. Its entities genuinely fit [rootobject]. Leave the data model alone.

Ref: hero_collab docs/superpowers/collab-oschema-review-2026-06-15.md.

Surfaced as a byproduct of the hero_collab oschema architecture review (2026-06-15). **Low priority — planner is shipped and works; these are ergonomics/clarity debt, not functional bugs.** Filing so the lessons aren't lost. ### Context The migrated planner makes all 12 entities `[rootobject]`. The macro's generated `_list`/`_find` return **sid-string arrays** (not full objects) and `_find` takes an opaque filter. That's a fine fit for planner's typed-column storage (no data loss), but the workspace-scoped UI pays an ergonomic cost: ### 1. N+1 on every workspace load (perf debt) `crates/hero_planner_web/src/index.html:362-369` (`listEntities`) reconstructs each scoped list via `_find` (sids) then `_get` per item — its own comment: *"Server `*_list_full` stubs return empty, so we reconstruct here."* So opening a workspace is 11 entity types x (1 `_find` + N `_get`) round-trips. Parallelized, but still N+1 over the socket. ### 2. `_list` is a dead stub `crates/hero_planner_server/src/rpc/main_impl.rs:341-343` — generated `_list` returns `vec![]`. Nothing uses it; it's dead surface. ### 3. `_list_full` is mis-scoped `main_impl.rs:345-367` — iterates ALL workspaces with no scope param (and is capped at 200). Correct only for the top-level `workspace_list`; useless/footgun for scoped entities. **Lesson (transferable from the collab review):** where a workspace-scoped full-object list is a primary access pattern, add an explicit custom `entity_list(workspace_sid) -> [Entity]` service method rather than relying on generated `_find`+`_get`. Removes the N+1. (Likely moot if the Dioxus rewrite #18 reworks this layer — so defer unless that slips.) ### 4. Stale/contradictory comments (cosmetic) `index.html:506-510, 522-523` still say currency-delete *"cannot work"* / *"Best-effort"*. This was **already fixed** by `bdb3935` (CurrencyRate.id now in the list output; server deletes by rowid via `parse_sid`). The delete works; only the misleading comments remain. Clean them up to avoid confusion. ### Explicitly NOT recommended Reversing planner's rootobjects (the collab review's all-custom conclusion) does **not** apply here: planner stores typed SQL columns (`INSERT INTO swot_items (title, description, kind, ...)`), so it has none of collab's blob/array/derived/side-effect/caller_id drivers. Its entities genuinely fit `[rootobject]`. Leave the data model alone. Ref: hero_collab `docs/superpowers/collab-oschema-review-2026-06-15.md`.