airglow
Automations and webhooks for the AT Protocol airglow.run
automation webhook atproto atprotocol
Star 36
Fork 1
Watch

refactor(form): split step-drafts.ts into focused modules #52

Closed tyb.there-hugo.dev
Closed opened by tyb.there-hugo.dev

app/islands/step-drafts.ts is now over 1000 lines (1042) and holds several unrelated jobs behind one import. A code review on the issue 7 fix flagged it as a maintainability risk: each new form feature adds to it, and its test file only covers parts of it.

Only app/islands/AutomationForm.tsx and step-drafts.test.ts import it, so a split is cheap to do now.

What's in it today:

Area Exports
Draft types and conversion StepDraft and the per-step draft types, mintStepId, stepsToDrafts, draftsToSteps, stepDraftName
Trigger gate hoisting hoistableTriggerBranch, hoistTriggerBranch
Tree navigation and editing ContainerKey, ROOT_CONTAINER, iterateDrafts, locateDraft, countDrafts, subtreeHeight, subtreeStepCount, replaceDraft, removeDraft, appendToContainer, moveWithinContainer, indent and outdent helpers
Reference checks and scope StepIssue, checkStepReferences, namesDefinedInDrafts, collectNamedOutputs
Field typing and schema checks resolveAssertionFieldType, recordPathStatus, operatorsForFieldType, OPERATOR_LABELS

Proposal:

Split along those lines into an app/islands/step-drafts/ folder, for example:

  • types.ts
  • convert.ts
  • tree.ts
  • references.ts
  • field-types.ts

Add an index.ts that re-exports everything, so AutomationForm.tsx keeps working unchanged. Split step-drafts.test.ts the same way.

Out of scope:

  • No behavior changes. This is a pure move.
  • Don't remove the re-exports through index.ts. That can be a follow-up once the split has settled.

Done when:

  • Each new module has one job and stays well under ~400 lines.
  • AutomationForm.tsx imports compile without edits, or with only import-path edits.
  • vp check is clean and vp test passes with the same number of tests.

You may want to tweak two things: the module names, and whether AutomationForm.tsx should switch to importing the modules directly instead of relying on the re-exports.

tyb.there-hugo.dev

Queued: the loop will work on this version of the issue.

loop:queued cid=bafyreib5n4jtxedguknalyhku7ky3hwbvjg6cl6yz2rx6s7nm4abityvdi

tyb.there-hugo.dev

Working on this, attempt 1.

loop:claim run=contribute-20261023-195640-3my3gqgcpa222 branch=loop/3my3gqgcpa222 attempt=1 issue=52

tyb.there-hugo.dev

Queued: the loop will work on this version of the issue.

loop:queued cid=bafyreib5n4jtxedguknalyhku7ky3hwbvjg6cl6yz2rx6s7nm4abityvdi

tyb.there-hugo.dev

Working on this, attempt 1.

loop:claim run=contribute-20261023-201425-3my3gqgcpa222 branch=loop/3my3gqgcpa222 attempt=1 issue=52

tyb.there-hugo.dev

refactor(form): split step-drafts.ts into focused modules

app/islands/step-drafts.ts (1042 lines) is now a folder app/islands/step-drafts/, one module per job, with an index.ts that re-exports the same public surface the old file had. The code was moved verbatim, so nothing behaves differently. The only code edits are three export keywords on helpers that now cross module lines. The test file is split the same way, and its 25 tests are unchanged.

Layout #

Module Lines Job
types.ts 95 Draft types, StructuralDraftKind, mintStepId
convert.ts 237 stepsToDrafts, draftsToSteps, stepDraftName, isValueLessOperator
hoist.ts 87 hoistableTriggerBranch, hoistTriggerBranch (trigger-gate advice)
tree.ts 239 ContainerKey, ROOT_CONTAINER, iteration, location, immutable edits, indent and outdent
references.ts 181 StepIssue, checkStepReferences, namesDefinedInDrafts, NamedOutput, collectNamedOutputs
field-types.ts 215 resolveAssertionFieldType, recordPathStatus, operatorsForFieldType, OPERATOR_LABELS, schema types
index.ts 64 Re-exports, listed explicitly

Tests: hoist.test.ts (12), references.test.ts (3), field-types.test.ts (10). They import the modules directly, not the index.

Verification #

  • pnpm run check (vp check): passes. Formatting, lint and type-check are all clean.
  • pnpm run test (vp test): 2502 tests pass in 159 files, the same count as on the base commit (a30b147). The step-drafts tests number 25 both before and after the split.

Acceptance criteria #

Decisions you can reverse #

  • Hoisting has its own module. I put trigger-gate hoisting in hoist.ts because the issue's table lists it as a separate area, so there are six modules instead of the five in the example.
  • index.ts uses explicit lists instead of export *. This keeps the public surface exactly what the old file exported. Two helpers are now exported between modules but are not re-exported from the index: assertionIsComplete (from convert.ts, used by hoist.ts) and bareRoot (from references.ts, used by field-types.ts).
  • AutomationForm.tsx still imports through the index, as the issue's out-of-scope note asks. Switching it to direct module imports can be a follow-up.
  • Comment and docs paths. I updated the path in one comment in lib/automations/steps.ts that pointed at the trigger-gate section, so it now names app/islands/step-drafts/hoist.ts. The old paths in docs/plans/ and docs/brainstorms/ are historical records, so I left them alone.

Review: https://tangled.org/exosphere.site/airglow/compare/main...loop/3my3gqgcpa222

loop:pushed run=contribute-20261023-201425-3my3gqgcpa222 sha=cce0cdbdc23b2bd041e2e933306f558439eb2774

tyb.there-hugo.dev

Merge. The branch splits app/islands/step-drafts.ts into app/islands/step-drafts/ (types, convert, hoist, tree, references, field-types, plus an index.ts that re-exports everything). It is a pure move with no behavior change. pnpm run check is clean and pnpm run test passes 2502 tests.

What I checked and how:

  • Pure move: I concatenated the six new modules and diffed their sorted lines against the old file from the merge base (a30b147). The only differences are import paths, per-module header comments, and export added to three helpers that now cross modules: assertionIsComplete, bareRoot and isValueLessOperator. The old file already exported isValueLessOperator through export { ... }. Every function body matches line for line.
  • Public surface: every name the old file exported is re-exported by index.ts. The two new cross-module helpers are not re-exported, so the index exports exactly what the old file did.
  • Module structure: no import cycles (types ← convert ← hoist/references; tree ← references ← field-types). The largest module is 240 lines, well under the ~400 limit.
  • Callers: AutomationForm.tsx and lexicon-schema.ts only change their import path to ./step-drafts/index.ts, which the issue allows. There is also a one-line fix to a path comment in lib/automations/steps.ts. The old path still appears in docs/plans/ and docs/brainstorms/, which are historical records.
  • Tests: the old test file is split into hoist, references and field-types test files. A sorted-line diff shows the test bodies are unchanged, and there are 25 tests before and after. In the full run, 159 files and 2502 tests pass. The PR says the file count matches the base, but that is a small slip: one test file became three. The test count is what the issue asks to keep, and it does.
  • Scope: the diff against the run's base sha also shows .tangled/workflows/check.yaml. That change comes from main commit a30b147, not from this branch. The branch's only commit (cce0cdb) does not touch CI.

loop:verdict run=review-20261023-202238-3my3gqgcpa222 sha=cce0cdbdc23b2bd041e2e933306f558439eb2774 verdict=merge

tyb.there-hugo.dev

Merged main into the branch: c88fcd6b9d5e8115529b134b56aa17bf30af4e75.

loop:carry from=cce0cdbdc23b2bd041e2e933306f558439eb2774 sha=c88fcd6b9d5e8115529b134b56aa17bf30af4e75

tyb.there-hugo.dev

Merged onto main as 08e556951c4baf09c843b3fdb2ea5c336f9e9333.

loop:merged sha=08e556951c4baf09c843b3fdb2ea5c336f9e9333 head=c88fcd6b9d5e8115529b134b56aa17bf30af4e75

Sign up or Sign in to add to the discussion.