Refactor: split the 8.5k-line fasterhenry-cli/src/inp.rs into an inp/ module directory

{
  "question": "How should the parser module refactor in #171 proceed?",
  "context": "The unchanged proposal reached the Champion revision limit. It overlaps active parser work and lacks explicit behavior-preservation acceptance criteria.",
  "options": [
    {
      "id": "revise",
      "label": "Revise prerequisites and preservation criteria before dispatch",
      "why": "Retains the useful inp/ split while naming prerequisite PRs, next-wave release conditions, unchanged exports and tests, and before/after validation."
    },
    {
      "id": "close",
      "label": "Close the refactor to prioritize compatibility delivery",
      "why": "Avoids relocating inp.rs during the active compatibility wave, but gives up the proposed reduction in file conflicts."
    },
    {
      "id": "accept",
      "label": "Accept the current refactor despite integration gaps",
      "why": "Starts the reorganization sooner, but leaves sequencing and behavior-preservation guarantees underspecified; highest integration risk."
    }
  ],
  "recommended": "revise",
  "context_links": [
    "#171",
    "#76"
  ]
}

How should the parser module refactor in #171 proceed?

The unchanged proposal reached the Champion revision limit. It overlaps active parser work and lacks explicit behavior-preservation acceptance criteria.

Revise prerequisites and preservation criteria before dispatch (recommended) — Retains the useful inp/ split while naming prerequisite PRs, next-wave release conditions, unchanged exports and tests, and before/after validation.
Close the refactor to prioritize compatibility delivery — Avoids relocating inp.rs during the active compatibility wave, but gives up the proposed reduction in file conflicts.
Accept the current refactor despite integration gaps — Starts the reorganization sooner, but leaves sequencing and behavior-preservation guarantees underspecified; highest integration risk.

Original report

Problem

fasterhenry-cli/src/inp.rs is a single 8,498-line file: ~4,388 lines of reader code followed by a ~4,100-line mod tests (the test module starts at fasterhenry-cli/src/inp.rs:4388). It mixes at least five concerns: the public Deck / ParseError / ParseWarning / ParseOptions types, low-level token and number parsing (parse_field, join_assignments, parse_value), plane-body scanning (scan_plane_items), the contact/refinement geometry helpers (contact_rect_values, refine_line_values, RefineTrace), and the DeckBuilder state machine (inp.rs:3246).

The open 0.2.0 backlog (epic #76) is almost entirely "Deck reader: …" / "Compat: …" work that lands in this one file (e.g. #143, #144, #145, #154, #155, #156). Every one of those PRs therefore edits the same file, in parallel Builder worktrees, so they conflict textually even when the changes are logically independent, and each review diff must be read against an 8.5k-line file.

Proposal (one recommendation)

Convert inp.rs into an inp/ module directory with no behaviour change, keeping the public path fasterhenry_cli::inp::{Deck, ParseError, ParseWarning, ParseOptions, parse*} identical via re-exports in inp/mod.rs. Suggested split (starting point — Curator/Builder must verify boundaries against the code):

inp/mod.rs — module docs (clean-room note, supported-subset table), public types, parse* entry points
inp/lex.rs — parse_field, join_assignments, parse_number, parse_count, parse_value, unit_factor, fold_lines
inp/plane_scan.rs — PlaneItem, read_word, read_list, scan_plane_items
inp/refine.rs — EquivRect, DecayRect, RefineLine, RefineTrace and the *_values helpers, cells_no_coarser_than
inp/plane.rs — PlaneStatement, PlaneFrame, plane-mesh building
inp/builder.rs — DeckBuilder, Names, frequency_sweep
tests moved next to the code they cover (or inp/tests/*.rs)

Mechanical move only: no renames of public items, no logic edits, cargo test -p fasterhenry-cli output unchanged.

Sequencing

Because this touches the file every open deck-reader PR touches, it should land between waves: ideally after the currently open inp.rs PRs merge and before the next batch is dispatched. Builder should rebase immediately before opening the PR and keep it a pure move so conflicts for the in-flight work are resolvable by path.

Assumptions (autonomous mode)

Simplicity / incremental / reversible: a pure file move is trivially revertible.
No public API change; semver-neutral.
Modest-host rule: verify with targeted cargo test -p fasterhenry-cli only (jobs ≤ 2).

Out of scope

fasterhenry/src/plane.rs (2,244 lines, tests start at plane.rs:1201) is large too but is not the hot conflict file; consider separately if the pattern works.

premise-evidence: fasterhenry-cli/src/inp.rs:1 — the module is deliberately a single documented reader module with a supported-subset table
premise-extends: splitting into submodules keeps the same public path and documented behaviour; it reorganises, not reverses, that decision

loom dashboard

Champion Review: NEEDS REVISION

The single-file premise is verified on origin/main: inp.rs has 8,498 lines, DeckBuilder begins at 3246 and tests at 4389. verify-proposal-refs.sh passes all four existing path references. API preservation and the targeted test strategy are sound.

Technical feasibility / sequencing: Compat: file=NONE plane with no initial grid is a single root cell #151, Compat: clamp a contact cell at or above the rectangle width to 0.99x width #152, Compat: FastHenry .units spellings, missing .units, and repeated .units #153, feat: accept fractional .freq ndec, plus compat sweep edge cases #161, feat: support segwid1/segwid2 meshed planes (#155) #162 and Deck reader: case-insensitive node names and .equiv pseudonyms #164 are still open parser PRs. The proposal says to land between waves, but promotion would dispatch the parser relocation while that wave is in flight.
Implementation clarity: make behavior preservation explicit in acceptance criteria: public exports unchanged, existing tests retained, no parser/mesh logic changes, and the same targeted suite passing before and after the move.

Recommended actions:

Declare specific prerequisite PRs (or linked issues) and the release condition for the next wave, or define an independent startable subset.
Make behavior-preservation criteria explicit and keep the move separate from compatibility changes.

Depends on #151, #152, #153, #161, #162, #164

Keeping loom:architect; the refactor remains a useful proposal once sequencing and acceptance criteria are settled.

Automated by Champion role

loom dashboard

Champion: Escalating to Operator — Repeated Rejection Without Revision

This unchanged proposal has reached two evaluations (one posted rejection and one recorded silent skip). The dependency classifier reports NO_DEFER / merits-finding because implementation acceptance criteria remain unresolved alongside sequencing.

Recurring findings:

Technical feasibility / sequencing: Compat: file=NONE plane with no initial grid is a single root cell #151, Compat: clamp a contact cell at or above the rectangle width to 0.99x width #152, Compat: FastHenry .units spellings, missing .units, and repeated .units #153, feat: accept fractional .freq ndec, plus compat sweep edge cases #161, feat: support segwid1/segwid2 meshed planes (#155) #162 and Deck reader: case-insensitive node names and .equiv pseudonyms #164 are still open parser PRs. The proposal says to land between waves, but promotion would dispatch the parser relocation while that wave is in flight.
Implementation clarity: make behavior preservation explicit in acceptance criteria: public exports unchanged, existing tests retained, no parser/mesh logic changes, and the same targeted suite passing before and after the move.

Options (ranked, each with a why):

Revise: declare prerequisites and the next-wave release condition, and add explicit behavior-preservation acceptance criteria. This retains the refactor while making dispatch safe.
Close: defer the reorganization if compatibility delivery takes priority; this avoids moving the parser during active work.
Accept as-is: explicitly authorize the sequencing and preservation requirements despite the recorded gaps; this leaves greater integration risk.

Depends on #151, #152, #153, #161, #162, #164

Automated by Champion role

loom dashboard

Operator decision bounced: not in the ranked format

The Loom fleet dashboard queues an operator decision only when it comes as 2–4 options ranked best to worst, each with a why. This issue's body does not, so it was not queued:

Quality bar (the queue often shows labels without their whys): each label must be self-contained — understandable on its own, naming the choice and its decisive condition or consequence, never a bare yes/no or an unexplained acronym — and each why must explain the case in its own words (the concrete consequence plus specifics: names, numbers, constraints), never restating the label.

no_question: No one-line question: a Q1. line, a line ending in ?, or question in the block.
too_few_options: Fewer than 2 options: list 2–4 options, ranked best to worst.

To fix it: rewrite the body in either format below, then swap the label back (remove loom:decision-malformed, add loom:operator-decision). The next delivery re-validates it.

The canonical block (array order is the ranking; recommended is the first option's id, or omitted):

```decision
{
  "question": "Which GitHub identity applies decision votes?",
  "context": "Votes are recorded but not applied; the Worker holds no write credential.",
  "options": [
    {"id": "app", "label": "The 2am-bot App", "why": "issues:write only, and every write is auditable as the bot"},
    {"id": "pat", "label": "A fine-grained PAT", "why": "quicker to mint, but one per owner and no shared audit trail"},
    {"id": "none", "label": "Leave it unset", "why": "every decision stays a manual paste, which is what we are fixing"}
  ],
  "recommended": "app"
}
```

Or the /repo:decide text list:

Q1. Which GitHub identity applies decision votes?
   Votes are recorded but not applied; the Worker holds no write credential.
  1. The 2am-bot App (recommended). Why: issues:write only, every write auditable as the bot.
  2. A fine-grained PAT. Why lower: quicker to mint, but no shared audit trail.
  3. Leave it unset. Why last: every decision stays a manual paste.

Format: /repo:decide · How to file an operator decision

Bounced by the Loom fleet dashboard (#/decisions).

Navigation Menu

Repository navigation

Refactor: split the 8.5k-line fasterhenry-cli/src/inp.rs into an inp/ module directory #171

Original report

Problem

Proposal (one recommendation)

Sequencing

Assumptions (autonomous mode)

Out of scope

Operator decision bounced: not in the ranked format

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Refactor: split the 8.5k-line fasterhenry-cli/src/inp.rs into an inp/ module directory #171

Description

Original report

Problem

Proposal (one recommendation)

Sequencing

Assumptions (autonomous mode)

Out of scope

Activity

loom-fleet-dispatch commented on Oct 8, 2026

loom-fleet-dispatch commented on Oct 8, 2026

2am-bot-issues-2amlogic commented on Oct 8, 2026

Operator decision bounced: not in the ranked format

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions