Skip to content

feat: make steps editable in Form view (fo-totkk)#1

Open
cwalv wants to merge 6 commits into
mainfrom
fo-totkk
Open

feat: make steps editable in Form view (fo-totkk)#1
cwalv wants to merge 6 commits into
mainfrom
fo-totkk

Conversation

@cwalv
Copy link
Copy Markdown
Owner

@cwalv cwalv commented May 2, 2026

Summary

  • Makes each step row in the Form tab a collapsible editable card with all schema-defined fields (id, title, description, needs, max_attempts, on_exhausted, metadata)
  • Adds + add step and × delete with automatic needs scrubbing; drag-to-reorder rewrites source order
  • Extends formula-write.ts with writeStepField, renameStepId, addStep, removeStep, moveStep, extractStepDescription

Test plan

  • All 242 existing tests pass (vitest run)
  • New tests for each formula-write helper (writeStepField variants, renameStepId, addStep, removeStep, moveStep)
  • Editing step title in Form → title updated in Source; surrounding steps untouched
  • Renaming step id rewrites all needs references in file
  • Adding new step appends [[steps]] block; immediately editable
  • Deleting step removes block and scrubs id from other steps' needs
  • Drag reorder changes source block order

Closes fo-totkk

cwalv and others added 6 commits April 28, 2026 04:29
…(fo-zsazt)

Implements Decision 1 of architecture-decisions.md: define bd's wire
schema in protobuf, generate TS types from .proto, treat the proto as
the canonical bd interface contract. Read-side only for v1; write-side
deferred to v2.

## Proto schema (proto/beads/v1/)

- types.proto: Bead (with reserved field-number ranges per substrate),
  Dependency, Comment, Event, Workspace.
- service.proto: BeadsService — List, Show, Ready, ListWorkspaces,
  GetFormulaSchema, ListFormulas (read-side).
- formula.proto: FormulaSchema, FormulaSchemaField, FormulaEntry.
- Status / type / dep_type are `string`, not proto enums (bd supports
  user-configured customs). Canonical values listed in field comments;
  renderers narrow on those.
- Field numbers reserved liberally to leave room for v2 additions.
- JSON names set via `[json_name = "..."]` to match bd's existing
  snake_case wire format — protojson `*Json` types align with
  `bd-server`'s output without a translation layer.

## Codegen pipeline

- buf v2 config (proto/buf.yaml): STANDARD lint, WIRE_JSON breaking.
- proto/buf.gen.yaml: TS via @bufbuild/protoc-gen-es, output to
  ui/src/gen/. Default codegen target.
- proto/buf.gen.go.yaml: Go via protoc-gen-go, opt-in (no Go consumer
  in this repo yet — bd-server lives elsewhere).
- ui/package.json scripts: postinstall + pre* lifecycle hooks regen on
  every entry point (build, test, test:watch, dev, typecheck). Fresh
  clones bootstrap on `npm install`; proto edits picked up
  automatically by every subsequent script.
- Generated code is .gitignored — never committed, always built fresh.
  Avoids drift / stale-gen errors.

## UI types refit (ui/src/types/index.ts)

Replaced hand-rolled type union with proto-backed aliases. The previous
file truncated bd's surface in several places; the refit broadens to
match handbook reference:

- BeadStatus: +pinned, +hooked (was 5 of 7 canonical statuses).
- BeadType: +decision, +story, +milestone, +spike, +event (was missing
  4 built-ins; included some customs as if built-in).
- DepType: full 20-entry surface (was 7 of 20).
- Bead: ~25 previously-missing schema fields now visible — gate
  fields, owner, started_at/closed_at/defer_until/due_at,
  source_formula provenance, mol_type/work_type, ephemeral flags,
  epic-progress rollup, etc.

## Call sites swept

Four consumers updated to the new wire shape:

- client/fleet.ts: dependency edges read embedded `Bead.id` instead of
  the non-existent `BeadDependency.depends_on_id`.
- lib/graph-walk.ts: same — dependency edges are full nested beads
  with `dependency_type`, mirroring `IssueDetails.Dependencies` Go
  struct (`[]*IssueWithDependencyMetadata`).
- components/peek/IssuePeekBody.tsx: dep rendering, comment.text
  (was `body`), event.event_type/actor/comment (was kind/author/message).
- routes/observe/graph.tsx: BeadStatus cast at narrowing sites.
- components/chrome/PeekDrawer.tsx + observe/GraphFilterRail.tsx:
  pinned/hooked status icons + colors.

## Side effect: fixed four latent wire-format bugs

The pre-refit code referenced fields bd never emits — it only worked
against test mocks. Refit aligns to bd's actual `--json` output:

- Comment.body → text (bd: `Text string \`json:"text"\``)
- Event.kind/author/message → event_type/actor/comment
- Dep edges: {depends_on_id, type} → embedded Bead with dependency_type
- Workspace.reachable now required (was missing in prior tests)

## Tests

All 183 UI tests pass against the new types. Test fixtures updated to
populate the now-required Bead fields (id, title, status, priority,
type, created_at, updated_at) and use the embedded-bead dep shape.
…-up)

bd's Issue Go struct tags IssueType as `json:"issue_type,omitempty"`.
The proto field stayed at name `type` without [json_name = "issue_type"],
so protojson silently drops bd's `issue_type` JSON output. UI consumers
that read bead.type against real bd-server traffic always saw an empty
string; tests didn't catch it because they mock bdClient.

Field number 22 stays the same (proto field numbers are forever); only
the JSON name needs to match the wire. Generated TS/JsonShape now
exposes `issue_type?: string` on BeadJson.

Surfaced by fo-1vtyi worker who hit the bug while building the queue
view (had to normalize `b.type ?? b.issue_type` as a workaround).

Also picks up package-lock.json drift: fo-zsazt added a postinstall
script (`npm run proto:generate`) which causes npm to flag
hasInstallScript=true in the lock file on first install. One-line
metadata update; no dep changes.

Tests: 183/183 pass against the regenerated types.
Implementation by foundations/worker-2 (session gc-wisp-d1x5).

Replaces /bead/:id 'not yet wired' stub. Peek drawer now renders all
proto-broadened Bead fields, opens from any source as a URL-routed
modal overlay, and resolves the modal-vs-route open question from
fo-zz4pz §4.

## Real-data wiring (IssuePeekBody, +275 lines)

- Overview: id, title, status, type, priority, assignee, owner, labels,
  external_ref, plus all six lifecycle dates (created/updated/started/
  closed/due/defer), close_reason, and the four body sections
  (description/design/acceptance/notes).
- Tabs: Overview / Deps / Comments + Metadata (when present) + Events
  (when bead.events is populated).
- Pack-aware metadata: inline METADATA_LABELS map for gc.* + the
  delegated_from key, friendly labels rendered with raw-JSON fallback
  for unknown keys. Marked for migration into fo-0qdg9 convention pack.
- Edit mode adds labels (comma-separated, diffed into addLabels[]/
  removeLabels[]) and external_ref. updateBead client gained the array
  variants.
- Dep ids are clickable to chain peeks; hides Events tab when
  bead.events is empty (matches fo-zsazt's 'dead UI' note).

## URL routing (App.tsx, hooks/usePeek.ts, routes/bead/index.tsx,
   routes/observe/graph.tsx, components/peek/BeadModal.tsx)

- useOpenPeek pushes /bead/:id with state.backgroundLocation; App.tsx
  swaps to backgroundLocation for the main Routes pass and renders a
  sibling modal Routes for /bead/:beadId.
- close() prefers navigate(-1); deep-link close falls through to
  /observe/queue.
- Deep links work: BeadDeepLinkUnderlay (routes/bead/index.tsx) lazy-
  loads ObserveQueue as the configured underlying view; modal Routes
  always renders the drawer when the URL matches.
- open() preserves an existing backgroundLocation when chaining (so
  dep-row clicks keep the original underlying view).
- bead-patched CustomEvent on save lets the (now decoupled) graph
  canvas re-sync without prop-drilling.

## Tests (IssuePeekBody.test.tsx, peek-routing.test.tsx)

- 12 new tests covering field rendering, edit mode, modal open/close
  routing, deep-link handling, peek chaining.
- 195/195 total pass against pre-bb1030b proto (fo-zsazt without the
  issue_type fix); rebase on bb1030b is non-conflicting.

## Notable choices (per worker review mail)

- Inline METADATA_LABELS rather than waiting on convention library —
  short-term to give the user a useful pack-aware render today; will
  move to fo-0qdg9.
- Fleet rows still drill into graph (unchanged); 'open from anywhere'
  read as the drawer mechanism being available, not a mandate to
  change every existing row click.

References:
- fo-zz4pz §4 (peek drawer real data + routing decision)
- ui-review.md cross-cutting §3 + View 8
- architecture-decisions.md (Decision 3 — pack-aware metadata)
Implementation by foundations/worker-sonnet (session gc-m2bwz).
Originally salvaged into commit 8e1a9d9 alongside fo-08ei2's work due
to both workers operating in primary's dirty tree concurrently; this
commit isolates fo-icz85's contribution for clean attribution and
git-blame.

Click a DAG node → source pane scrolls to the matching `[[steps]]`
block AND transient highlight flashes (~500ms ease-out). Reverse
direction (cursor in source → DAG node selection) preserved.

## Implementation pattern

Mirrors the existing errorScrollTo nonce pattern:
- `dagScrollTo: { id: string; n: number } | null` prop on SourcePane.
  When set, useEffect resolves the matching stepRange and scrolls
  the textarea so the block is in view.
- `flashNonce` state on SourcePane bumped per-click (so repeated
  clicks on the same node re-fire the flash).
- `onDagSelect(id)` callback in edit.tsx sets both `selected` (for
  persistent range highlight) and bumps `dagScrollTo` nonce.
- New CSS keyframe `@keyframes src-step-flash` (500ms ease-out) on
  the active range; `key={flashNonce}` on the highlight element
  forces React remount so the animation replays on each click.

Persistent range highlight (selected) is unchanged; the flash overlays
it briefly per click without disturbing the existing selection.

Quality gap from `fo-zz4pz §9` resolved.

Refs:
- fo-zz4pz §9 (quality gap)
- fo-08ei2 8e1a9d9 (mixed commit; this isolates the icz85 hunks)
Replaces the stub at /observe/queue with a flexible filter+grouping
interface over individual beads. Default lens is `bd ready`; the
ready+deferred and all lenses widen the query.

Per-row affordances: status glyph, priority pill, type chip, claim
button (open + unassigned + ready lens only), gc.routed_to chip,
labels, due/overdue indicator, age. Click opens the existing peek
drawer (fo-1w8xo).

Toolbar: lens selector, text search, unclaimed/overdue toggles, an
expandable filter strip (type/priority/status/label), group-by
chips (priority/status/formula/type/none). All state is URL-synced
so views are shareable and survive reload.

CLI mirror strip at the bottom shows the equivalent bd command for
the current view; copy-to-clipboard included.

Polling reuses the useFleetPoll cadence pattern (visibility-paused,
exponential backoff on errors). Defaults to 5s tick.

Acceptance (per bead):
- /observe/queue replaces stub
- Filters and groupings work against proto-broadened Bead surface
- Claim updates status atomically (`bd update <id> --claim`)
- Empty state renders cleanly (lens-aware)
- CLI mirror updates with filters

13 new tests; full UI suite (208) green; production build green.
Add collapsible editable StepCard components to the Form tab's Steps
section. Each card exposes id, title, description, needs (ChipList),
max_attempts, on_exhausted, and metadata fields backed by surgical
TOML round-trip helpers.

New write helpers in formula-write.ts:
- writeStepField / extractStepDescription
- renameStepId (rewrites all needs references)
- addStep / removeStep (scrubs needs on delete)
- moveStep (HTML5 drag-and-drop reorder)

34 new tests; all 242 tests pass. Typecheck clean on changed files.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant