Skip to content

Dev/dialog editor#100

Merged
burner1024 merged 361 commits into
masterfrom
dev/dialog-editor
Jul 11, 2026
Merged

Dev/dialog editor#100
burner1024 merged 361 commits into
masterfrom
dev/dialog-editor

Conversation

@burner1024

Copy link
Copy Markdown
Member

Dialog editor and assorted fixes.

…ranch refuse)

Wire addBranch/addElse/removeBranch ops into DialogGraph actions (Tier 3c,
gated by structEditable + rebuild). Inspector gains three controls inside the
{#if state.branches} block only:

- Per-branch remove button (X) in the branch head, disabled-with-tooltip when
  b.opaque.length > 0 (side-effect refuse: cannot splice opaque stmts safely).
- After the branch list: an inline condition input + "+ if" button (disabled
  until a non-empty condition is typed) that calls addBranch then clears.
- A "+ else" button shown only when there is exactly one if-branch and no else.

The new-branch condition input uses a writable store instead of $state: the
Svelte 5 compiler misidentifies $state() as a store subscription when a local
binding named `state` (the node prop) is in scope, leaving the variable
non-reactive and breaking the disabled check and programmatic clear.
…ten unverified inspector comment

In branchStructureOps ADD-if path, the insert offset was computed as
Math.max over ALL original branches' stmtRange.end. When the last branch
was being removed in the same save, its end was used as the anchor for the
new branch splice, technically overlapping the removal. Filter to originals
whose stmtRange.start is in editedIfStarts (the survivor set the REMOVE pass
already built), anchoring at the max surviving end. Fallback when all
originals are removed: min original stmtRange.start (inside the procedure
body, safe under right-to-left application).

New test pins the edge: two if branches, remove the last and add a new
sibling if in one save - asserts the new branch lands after the surviving
branch (not at the deleted branch's end), the survivor is byte-exact, the
output re-parses bundleFaithful, and verifySSLEditApplied is ok.

Inspector.svelte: tried $state("") locally to verify the compiler warning -
build produced two warnings (non_reactive_update + state_referenced_locally,
both from the state prop name shadowing the $state rune). Kept writable;
updated the comment to state the observed build-warning behavior only, without
asserting the compiler-internals mechanism.
… an entry

The entry-call addition filter excluded every renamed node on the assumption
its call already existed, but a node that gains entry status for the first
time while being renamed has no prior call to rewrite - so it was silently
left unwired. Identify a renamed node in the original entry set by its old id
(renamedFrom) instead of dropping all renamed nodes.
applySplices applies ops right-to-left, which is only correct when their spans
are disjoint; an overlap silently corrupted the output. Add a guard that throws
on an overlapping or start>end op, and cover the splice contract (overlap,
adjacent, zero-width-inside-range, reversed, single-char, empty) directly - the
core every save path depends on previously had three tests.
Reachability: self-looping entry stays reachable, self-looping non-entry island
is orphan, single-state graph, empty-root classification. Layout: single node
with no edges, and a back-edge cycle terminates with finite distinct positions.
…on, relocate VerifyResult

- Remove targetLabel: exported but referenced nowhere.
- Drop layoutFlow's direction parameter and its dead DOWN path; the sole caller
  always uses the default RIGHT, matching the module's left-to-right design.
- Move VerifyResult to dialog-splice (the shared core both editors import) so
  dialog-ssl-edit no longer depends on dialog-d-edit for a format-neutral type.
…mment

- Replace three identical inline @N-id-parsing lambdas (and a fourth in the
  new-node path) with one module-level atMsgId helper.
- The badge-vocabulary comment claimed computed/random/side-effect/virtual-sink
  had no producer; all are produced now (stateBadges/choiceBadges) - correct it.
- Document why callTransitions dedups by target name and what fixing the
  duplicate-call rename gap would require (model-layer, not just the parser).
…view polish

- Tree view: a bundle (if/else) node now renders each branch with its own
  'shown when ... / otherwise' header, NPC line, and replies, instead of
  flattening to the if-branch line plus all options. The else branch's line was
  previously dropped. conversation-tree builds the branch structure; each choice
  is expanded once (via its branch), never double-marked shown.
- Tree: render the derived marker through the shared Badge component (drop the
  hand-rolled badge span + duplicated CSS).
- Inspector: compute branchChoices once per branch; rename the 'logic' drawer to
  'side effects' (it holds source-only side-effect statements).
- Graph legend: dash the 'continue' key so the dashed-edge encoding is documented.
A node may `call X;` more than once (e.g. one call per if-branch). The parser
deduped call transitions by target name, so the model carried only the first
call site - rename rewrote one site and left the others as dangling `call X;`
(sslc-rejected), and delete spliced out only the first inbound call.

Record one call transition per statement (callTargets stays deduped to one graph
edge / one call-choice per unique target). Replace the single-cardinality
callStmtRange/callTargetRange/callTopLevel on DialogChoice with a callSites list
so rename rewrites every target token and delete removes every top-level site.
49 node-to-node duplicate-call sites exist in the test corpus.
…imap

The inspector (top-right Panel) capped at calc(100vh - 96px), but the svelte-flow
minimap sits bottom-right in the same column. A tall node's inspector grew down
into it (measured 112px overlap across the minimap's full width at a 640px-tall
viewport). Reserve the minimap band (~152px widget + 15px margin) plus a gap in
the cap so the inspector bottom stays ~12px above the minimap top at any viewport
height.
…rness

Applying the render-STATE sweep across the graph view surfaced two more layout
collisions between individually-correct panels:

- The top-center legend was overwritten by the variable-width top-left toolbar
  (a wide toolbar reaches center), hiding the Source/Issues buttons. Fold the
  legend into the toolbar as one flex-wrap container capped to leave the
  top-right inspector's column clear - it wraps downward, never into a sibling.
- The bottom-left D source panel covered the fixed bottom-left zoom controls.
  Lift it above them (not right, which would push it into the centered Issues).

Add a render-STATE guard to the production-path harness: with the selection
inspector + Source + Issues open at once, assert no two overlays' bounding boxes
intersect - the busy state a single default frame never exercises.
… its body

A node DELETE removed the procedure body and retargeted inbound options but left an
orphaned 'procedure <name>;' forward declaration in the saved SSL. Capture the full
forward-decl statement span in the parser (distinct from the name-token span used by
rename) and splice it out on delete. Test asserts zero dangling symbols after delete.
…de the canvas

The inspector, D-source preview, and issues panels floated over the graph as
absolutely-positioned overlays. At the narrow width the webview actually runs at
(it opens beside the text editor, sharing the VS Code window, ~270-540px) they
collided with each other and the canvas chrome (minimap, zoom controls) - the
prior collision guard only tested at 1100px and never saw it.

Dock all three auxiliary panels into one shared right rail beside the canvas, for
both graph and tree view modes. This removes the whole floating-panel collision
class by construction rather than repositioning per width, and unifies the two
modes' auxiliary-panel handling (the tree mode previously duplicated it as .tovl
overlays). Also: hide the minimap on a narrow canvas (it otherwise sits atop the
zoom controls, and is not useful that small), and cap the top-left toolbar to the
canvas width so it wraps down within the canvas instead of overflowing into the
rail. The tree toolbar and the legend now wrap too, so a narrow canvas can no
longer clip the legend or strand the Graph/Tree toggle.

The render harness now drives the collision guard at 300/520/900px (the real
embedded widths, narrow and wide) instead of a single roomy 1100px frame.
The dialog editor is a WebviewPanel opened beside the text document, not a
CustomTextEditor, so VS Code's own Ctrl+S does not reach the underlying .d/.ssl
from the webview - the shortcut was a dead key and saving required the toolbar
Save button. Wire a window keydown to the same host save the button uses, for
Ctrl+S (and Cmd+S on macOS), guarded so it no-ops with no host.

The chord predicate is extracted to a pure keyboard.ts and unit-tested (the
cross-platform Cmd case is the one worth guarding), matching how the webview's
other pure decisions live in app-messages.ts; the window binding stays in the
component.
…ntry be unset

Two friction points a modder hit:

- Deleting a state silently redirected every transition pointing at it to EXIT.
  Now a delete that would redirect inbound transitions pops a confirm naming the
  count ("N transitions pointing here will be redirected to EXIT"); with no inbound
  refs it deletes straight through. countInboundGotos is a pure helper in
  dialog-edit-ops (unit-tested), and the render harness drives the confirm/cancel
  flow end to end.

- The SSL entry checkbox was one-way: marking a node as a dialog entry immediately
  disabled the box (the removable check read the on-disk entry calls, which don't
  yet include the pending toggle), so it couldn't be un-marked until save+re-parse.
  A node made an entry in-session can now be un-marked freely (the save adds no
  call, so nothing is orphaned); a genuinely non-removable original entry (wired by
  force_dialog_start or a conditional call) stays disabled and now carries a tooltip
  explaining why and pointing to the .ssl source.
…source tree

The binary-editor render harnesses wrote their shot-*.png outputs into the
harness dir itself, cluttering the working tree whenever a harness runs. Route
them through a shared shotPath() helper (out-dir.ts) that resolves the repo-level
tmp/ dir, matching the dialog harness (client/src/dialog-editor/test/harness/
render.mts). All 16 screenshot-writing harness files updated; verified the harness
typecheck passes and a sample run lands its PNGs in tmp/.
…path

svelte-flow's built-in delete key (Backspace/Delete on a selected node) removed
the flow node directly, bypassing the model edit ops: no confirm, inbound GOTOs
left dangling, the Issues panel unaware, and nothing undoable - a live review hit
exactly this and could silently corrupt a dialog. The confirm added earlier only
covered the inspector button and tree menu; the keyboard was an unguarded cohort.

Disable svelte-flow's built-in delete (deleteKey={null}) and handle Delete/
Backspace ourselves, routing every delete path through one requestDeleteState gate
(reject non-deletable, confirm the EXIT redirect, then delete). The key handler is
skipped while typing in a field or while the confirm modal is open. Harness now
asserts Backspace on a referenced node pops the confirm instead of silently
deleting.

Also gitignore .playwright-mcp/ (Playwright MCP snapshot output written to the repo
root during live UI review).
…sation tree

The Tree view had two accessibility gaps a live review caught: each state row was a
focusable div wrapping a focusable caret button (two inconsistent tab stops per
row), and the arrow keys did nothing. Adopt the WAI-ARIA tree pattern: the
container is role=tree, each state row is a role=treeitem with aria-level,
aria-selected and aria-expanded; the expand caret is taken out of the tab order
(tabindex=-1, aria-hidden) so each row is a single tab stop; and a roving tabindex
plus ArrowUp/Down (move between rows) and ArrowRight/Left (expand/collapse) give
real keyboard navigation. Harness asserts the roles, the absence of tabbable
carets, and that ArrowDown roves focus.
…logSourceEdit

The two existing tests only exercised weidu-d, leaving the allocateNodeIds /
allocateOptionIds branch - the exact logic the extraction exists to protect -
with zero executed assertions. Add a fallout-ssl test that adds a new node and
a new option with un-allocated literal text, drives the real allocation, and
asserts both the spliced source and the merged messages map. Also assert
result.messages on the existing D tests, which previously checked only newText.
…ovider

Replace the command-owned WebviewPanel with a CustomTextEditorProvider bound
to the source document, so VS Code supplies native dirty tracking, undo, and
save. resolveCustomTextEditor wires the webview HTML and message routing; on
"ready" it parses and posts the model; on an edit action it splices into the
live document via a single full-range WorkspaceEdit (one native undo step)
and write-throughs message text to .tra/.msg on a short debounce plus on
native save. An EchoGuard suppresses re-projecting the graph in response to
the provider's own WorkspaceEdit, while still re-projecting external text
edits. Per-panel session state (guard, latest model, debounce timer, pending
verify) replaces the old module-level mutable panel/docUri/refreshTimer.

The webview still posts a legacy whole-model "save" message alongside the
target "edit" message; both route through the same applyEdit path until the
webview switches to per-action edit emission and drops the Save button.

The command becomes a thin vscode.openWith wrapper. registerDialogEditor
keeps its existing signature so extension.ts is unchanged.
… shared text gate

A just-added SSL node's NPC line was locked ("this line has no plain @n
message id") the moment the webview adopted the reparse: the per-call-site
`isNew` proxy used `isPendingState`, which flips false once the node gains a
`procRange` from the fresh parse, even though its Reply is still unwritten.

Introduce `textEditability`, the single decision both views (inspector fields
and inline tree edits) consume, returning the lock and its reason together so
neither re-assembles `textFieldLocked`'s inputs at the call site. It resolves
the authorable proxy by owner: an option keys on `isPendingChoice`, a node's
NPC line on the new `npcLineAuthorable` (pending OR a faithful reply-less node
whose Reply the save path allocates).

Back the reply-less case with a `replyless` flag on DialogState, set at SSL
parse under the exact `replyOps` precondition (faithful, no source Reply, and a
node-level insertAnchor). The writer's orphan-id set now reads the same flag, so
the UI gate and the save path agree by construction - a bare `text === ""` check
would mint ids for anchorless TSSL nodes the writer can never splice.
…y can't mutate the original

A duplicated node cloned its source line's bare @n ref verbatim, so the copy
pointed at the SAME .msg/.tra entry as the original. Editing the copy's line
(writeText keyed on that shared id) then silently rewrote the ORIGINAL's line
too - a copy that corrupts its source on the first edit.

duplicateState now detaches each resolvable @n ref to its literal, so the copy
is a new-node literal the save allocator mints a FRESH id for and the copy owns
an independent string. An unresolved ref (its .msg never loaded) has nothing to
detach and stays a ref - harmless, since editing an unresolved line is already
locked. Textless "continue" options and computed/random text are left untouched.
Applies to both families (SSL and D both store shared strings as @n refs).
…write an invisible orphan

+State on an existing SSL dialogue splices `procedure NodeXXX begin end`, but
the projection only kept an unreachable procedure that already had a reply or
option - so the empty node was written to disk yet dropped on reparse, vanishing
from the tree while leaving a husk in the source (BUG A).

An empty `NodeNNN`-named, non-hook procedure is a dialog node in progress; keep
it visible. Its NPC line is authorable through the `replyless` gate, so the node
shows AND can be filled in immediately, and it survives every reparse because it
genuinely round-trips (an empty body is faithful). Existing scaffolded empty
stub nodes now show too - a net improvement for a tree editor.

Reserved sinks (Node998 combat / Node999 end) stay excluded from the orphan
path: they are transition targets, not nodes. A renamed empty node (not NodeNNN)
still needs wiring to appear; the general pending-node layer will cover that.
…silently vanish

Editing a D/TD NPC line backed by an @n whose .tra never loaded looked like it
worked - the tree updated, the unresolved count dropped, the tab read "saved" -
but the edit only touched the in-memory message map; the server's writeMessages
found no .tra target and returned without throwing, so nothing reached disk and
no error surfaced (BUG E).

Unify the text gate: a bare @n line is editable only when its .msg/.tra entry
actually loaded, for BOTH families - an unresolved ref has nowhere to write, so
lock it and fail visibly (a disabled field with a "point translation.directory
at the .tra" reason) instead of accepting an edit that drops on save. SSL already
did this; the D family short-circuited to always-editable. Literals stay editable
for D (persisted via the .d splice) and locked for SSL, unchanged. The reason is
now family-aware (.msg for SSL, .tra for D).
…s to the gate

The silent "what I see isn't what's saved" bug class hid because no gated test
drove the full production edit pipeline and asserted the on-disk bytes against
the intended view. Add two end-to-end guards through the real parse -> edit op ->
computeDialogSourceEdit -> writer -> reparse path:

- +State on an existing dialogue creates a node that survives the reparse and can
  then be filled in and persisted (BUG A + the replyless NPC-line authoring loop).
- Duplicating a node and editing the copy leaves the original's on-disk string
  untouched, the copy owning a fresh independent @n (BUG C, end-to-end on disk).

Both run in the server unit gate; each fails against the pre-fix code (Node002
dropped on reparse; @200 rewritten by the copy's edit).
…-line .msg/.tra edit

The inspector's NPC and option fields are textareas (they wrap/autosize long
lines for reading), so pressing Enter inserted a newline that the write-through
persisted - the closing delimiter ended up on its own line and the stored string
carried a trailing newline (BUG D). The inline tree editor, a single-line input,
never had this.

Two guards: writeText folds every CR/LF/CRLF to one space on write, so no
newline can ever reach a .msg/.tra entry regardless of the input widget (a paste
included) - newlines only, not a trim, since writeText runs on each keystroke and
trimming would strip a space the user is mid-typing. And Enter now commits (blur)
on both fields instead of inserting a newline, matching the tree's inline input;
Shift+Enter stays a browser escape hatch that the fold still neutralizes.
… legend

The "Del delete" legend read as applying to whatever was selected, but the key
handler only ever deleted the selected STATE - selecting an option and pressing
Del silently removed its parent node instead (BUG G). Options could only be
removed via the row `x`.

Del now removes the selected option through the same gate its row `x` uses (a
structurally-editable flat node, not a conditional SSL option that sits in its
own `if` the writer won't rewrite); a locked or branch option is a no-op, never
a surprise node delete. With no option selected, Del deletes the selected state
as before. This unifies the delete gestures so the keyboard, the row `x`/`-`,
and the context menu are one path, and makes the legend honest.
…ad of dropping the delete

Deleting an NMessage/GMessage/BMessage terminal option was a silent no-op: a
terminal carries a stmtRange but no callRange, so it was invisible to nodeOps'
callRange-based remove pass and the option stayed on disk. Since a new option
defaults to EXIT -> NMessage, "add an option then delete it" hit this exactly -
the added option could not be removed once saved.

nodeOps already special-cased the terminal->node flip for the same
callRange-blindness; add the missing terminal-removal pass alongside it, splicing
out any flat source terminal whose id is gone from the edited node (a surviving
or retargeted terminal is skipped; a conditional terminal is left to Tier 3).
Shared by the SSL and TSSL writers, so both families gain it. The line-aware
splice leaves no blank-line residue.
…t-gate unification

The E-fix removed textFieldLocked's D short-circuit, so the parenthetical
explaining why isPendingChoice gates on sourceRange was stale. A D option
mis-read as pending is now treated as isNew (always editable), which would let an
unresolved-@tra option slip past the very lock the D gate just added - restate
the rationale in those terms.
… usability testing

The @N-unresolved help (webview banner + the locked-field tooltip) and
docs/settings.md all told the author to set `translation.directory` in
.bgforge.yml, but the reader (settings.ts) only reads it nested under a top-level
`mls:` block - so a hand-written top-level `translation:` silently never resolves
and every @n stays raw. Point all three at `mls.translation.directory` and show
the nested YAML the code actually parses (the auto-created config already used
this shape; only the guidance was wrong).

Separately, a node forced read-only by a trailing non-dialog statement (e.g. a
`set_map_var`) was told it "nests if/else conditions" when it has none: the
structured tier is entered by a nested if/else OR a preserved non-dialog
statement, so the reason now names both instead of if/else alone.
…, not just the first

A WeiDU D `SAY @A = @b = @c` state kept all its lines in the model (sayTexts) but
every view rendered only the first, so a 9-line monologue showed one line and the
rest were invisible and uneditable.

Surface them everywhere: the tree renders the continuation lines as muted,
indented rows after the primary line (a monologue reads as a sequence); the
inspector shows each as its own "NPC line (cont. N)" field; the graph card - the
compact overview - shows the first line plus a "+N more" chip. ConvState gains
`sayLines` (the resolved continuation lines) and `sayLineEditability` gates each
field with the same @N-resolvability / literal rules as any other line.

Editing any line persists: an @n line writes its .msg/.tra entry (the .d keeps
`@a=@b=@c`); a literal line updates sayTexts and the writer re-joins it. That
exposed a silent-drop the display made reachable - stateUnchanged and the D
writer's SAY-change check both keyed on line 0 (`text`) only, so a
continuation-only edit read as unchanged and never spliced. Both now compare the
whole sayTexts, so a literal continuation edit reaches the source. Live-verified:
editing continuation line 2 rewrote only that alternate on disk, lines 1 and 3
byte-preserved.
…isn't in source yet

An item you add lives in the webview's optimistic model before the reparse
adopts it into the source parse; an empty option is deferred until its text
commits. Until then it rendered identically to a committed item, so there was no
cue that it was unsaved (the "no cue" usability note).

Surface it: ConvState/ConvReply gain a `pending` flag (from the existing
isPendingState/isPendingChoice predicates). The tree marks a pending node/option
with a 2px amber left-accent and an "unsaved draft" tooltip; the graph card gets
a dashed amber border and an "unsaved" badge (badge text, not colour alone,
carries the meaning). Committed items are unmarked.

Live-verified in code-server: an added-then-abandoned empty option shows the
amber draft accent while its committed siblings do not. The card cue follows the
same isPendingState predicate (svelte-check clean); a pending NODE commits within
the ~250ms emit/adopt window, so its badge is transient by nature.
New WeiDU D states default to StateNNN (next free, zero-padded) instead of new_state/new_state_1, mirroring the SSL NodeNNN scheme; duplicates use the same allocator rather than <id>_copy. The auto-name toggle tooltip and the add-node comment now say StateXXX for the D family.
… cleanly

The add-node writer anchored the new id just before the append list's close token, so a multi-line variadic append with a trailing comma after its last element (what a formatter emits) got a second comma - an empty call argument that is a hard TypeScript syntax error, breaking the parser for the whole file. Anchor after the last element instead, leaving any trailing comma valid. Adds a regression test over the real multi-line variadic-with-trailing-comma shape.
Three write-back bugs that only surface on real authored dialogs, each found by the new corpus round-trip tests:

- D: rename/delete did not rewrite GOTO references inside EXTEND/CHAIN/INTERJECT constructs, leaving a dangling jump (a WeiDU compile error). Mirror the model retarget into those derived spans (rename rewrites the label; delete flips the GOTO clause to EXIT).
- SSL: deleting a node whose inbound option carried a computed message id left the option pointing at the deleted node. Preserve the computed id text and emit a terminal NMessage instead of bailing.
- SSL/TSSL: renaming a node referenced from inside an if/else bundle branch double-spliced the reference span (the per-node bundle engine already rewrites it), crashing applySplices. Skip the standalone rewrite for bundle nodes.

Adds a shared corpus round-trip harness that runs the full edit battery (retarget, add/remove/reorder option, add/remove/rename node) through the real apply*DialogEdits over the external corpora for all four families, asserting targeted reachability-robust invariants.
… divider line

Adds a border-bottom under the beta/feedback row (matching the toolbar's other #2b303a divider) so the notice reads as a distinct band above the button row.
… sleep

render-map-jump-deep raced a fixed waitForTimeout(300) after clicking the SID
jump chip, then read the object detail's SID field. A deep jump is two async
round-trips - locateEntry fetches the full object list to resolve a selection
past the bounded window, then the detail pane fetches and renders the landed
object's fields - so at 300ms the detail pane was still empty and the read
returned null, failing the assertion even though the jump had landed on the
correct object (verified by screenshot). Replace the sleep with a waitForFunction
poll on the SID field rendering; the assertions still report the concrete value
on a real regression rather than a poll timeout.
The binary-editor and dialog-editor Playwright harnesses assert through PASS/FAIL
gates (driver-util, clip-gate, per-driver failed counters, page-error and CSP
checks) but nothing ran them, so they caught regressions only when a human
remembered to. Wire them into CI so they guard every push.

- scripts/test-harness.sh: build the binary core + both app.html bundles, then
  run every assertion-bearing driver (all 20), failing on any non-zero exit.
- .github/workflows/harness.yml: a SEPARATE job (not the main Build gate) so the
  Chromium download and browser runs add no wall-clock to every other check. It
  installs Chromium, builds grammars (the dialog edit drivers parse .d through the
  weidu-d WASM), resets the external fixture repos, then runs the suite.
- playwright becomes a pinned devDep (exact 1.61.1): tsx/ESM won't resolve a
  global install (NODE_PATH is ignored for bare ESM specifiers), so it must live
  in workspace node_modules. pnpm's build-script gate skips its browser download,
  so a plain install stays lean; the harness paths install Chromium explicitly.
- With playwright resolvable in CI, the harnesses are typechecked too: the
  dialog-editor harness via "Typecheck client" (it sits under client/src), the
  binary-editor harness via its own DOM tsconfig as a new test.sh step.
- Docs updated for the devDep + the new CI suite (AGENTS.md, harness READMEs).
…ew drivers

Every render/drive harness driver raced fixed waitForTimeout(N) sleeps that,
under CI load, could read the DOM before the awaited async work (a fetch, a
re-render, a debounced round-trip) landed - the same flake the deep-jump driver
hit. Convert each to a condition wait:
- sleeps before a Playwright locator action: deleted (locators auto-wait);
- sleeps before a raw page.evaluate read that feeds an assertion: replaced with
  page.waitForFunction polling the specific condition the read expects, with
  .catch(() => undefined) so a real regression fails on the concrete assertion
  rather than a poll timeout;
- a few bounded waits kept + commented where no crisp condition exists: asserting
  an ABSENCE (no husk spliced, no echo-loop emits), a same-model re-post/adopt
  that yields no distinct DOM signal, or a negative invariant on a disabled control.

Also fix a latent timeout bug: page.waitForFunction(fn, { timeout }) is the 2-arg
form where the options object binds to the callback ARG, so the poll silently used
Playwright's 30s default instead of the intended 5s (verified against
playwright-core 1.61.1). Passed undefined as the arg at all 51 sites.

The suite runs deterministically green across repeated runs and ~2x faster (polls
resolve as soon as the condition holds; sleeps always waited in full).
…ushed past EOF

An empty CRE section's offset is preserved verbatim from the header (not
recomputed) so an unedited fixture round-trips byte-exact - across the corpus it
almost always already equals the computed in-place position, but a fixture can
store a different in-bounds value there (imoen.cre) and preserving it keeps that
file exact. A size-shrinking edit - removing a spell-memorization owner and its
slice on x#beador.cre drops 88 bytes - could leave that preserved offset pointing
PAST the new EOF (effects empty, offset 0x558 vs a 1360-byte file), and the parser
rejects `offset > size` even for a zero-length section, so the op produced bytes
that would not reparse ("Missing CRE group: CRE Header").

Fix at the single serialization choke point: where a preserved empty-section
offset now exceeds the new total size, fall back to the computed in-bounds
position; a still-valid offset is left untouched, so unedited round-trips (and the
imoen.cre outlier) stay byte-exact. Guarded by the existing fixture-driven
cre-entity-ops regression over x#beador.cre.
…onent flags

These three WeiDU component flags were missing from the grammar, so any
component using them (e.g. BG1NPC's bg1npc.tp2) failed to parse and the
formatter rejected the whole file. Add the flag rules (message required,
matching the sibling *_component / *_predicate flags), route the new
predicate flag through the same OR/AND condition splitter as
REQUIRE_PREDICATE in the formatter, and add tree-sitter highlight +
corpus coverage.

TextMate highlighting and completion/hover data for these flags are not
yet wired up; tracked as follow-up.
…e format sweep

BG1NPC (added to the IE corpus as a dialog test corpus) is heavily
templated. Two classes of its files are not valid standalone WeiDU and
so cannot go through the formatter's parse+idempotency check:

- 184 BAF files use WeiDU %variable% placeholders at trigger/action
  position (e.g. IF %BGT_VAR%), substituted at install time. weidu
  rejects them standalone too; they are the same class as the existing
  %variable% exclusions.
- 3 TPA files carry a stray extra END (unbalanced BEGIN/END). weidu
  tolerates the trailing END, our stricter grammar does not.

The remaining BG1NPC parse failures (valid .d and .tpa constructs weidu
accepts) are real grammar gaps, fixed separately.
Two valid WeiDU constructs BG1NPC uses that the grammar rejected:

- EXTEND_TOP/EXTEND_BOTTOM accept an optional USING ~tra~... clause
  after EVALUATE_BUFFER (as COMPILE already did); without it the tra
  path was mis-parsed and reported a spurious MISSING '='.
- REMOVE_CRE_ITEMS (plural, no argument) strips every item from a CRE,
  distinct from the singular REMOVE_CRE_ITEM ~resref~ that was already
  supported.

weidu accepts both (cross-checked with --parse-check); add corpus
coverage for each.
CHAIN3 is WeiDU's improved chain keyword; it is syntactically identical
to CHAIN (same optional IF/WEIGHT/THEN header, file, label, chain text,
epilogue) but was not accepted by the grammar, so all 26 BG1NPC dialogs
that use it failed to parse. Accept it as an alternative chain keyword;
the formatter reads the actual keyword token, so CHAIN3 round-trips
unchanged (verified it is not normalized to CHAIN). Add corpus coverage.
… constructs

Valid WeiDU D constructs (weidu --parse-check accepts them) that the
grammar rejected, so BG1NPC dialogs using them failed to format:

- %var%-interpolated names: %tutu_var%KELDDA etc. - a name assembled at
  compile time from a %var% substitution abutting literal text. Modeled
  as interpolated_name using token.immediate so pieces only combine with
  no intervening whitespace; added to the filename position.
- Hyphenated state labels (Quayle_Shar-Teel_1): allow '-' in
  state_label_alnum.
- ALTER_TRANS change with a bareword field (ACTION ~..~), not just the
  quoted "ACTION" form.
- A trailing IF/UNLESS guard after an ADD_TRANS_ACTION action string
  (WeiDU accepts the guard on either side).

Full IE .d corpus reparses with zero regressions; formatter round-trips
all four unchanged. Corpus coverage added for each.
weidu --parse-check rejects this file; it uses an unresolved %kivan17%
macro as standalone CHAIN content. Same class as the malformed .tpa -
weidu tolerates neighbouring valid constructs but the file itself is not
parseable standalone, so keep it out of the format sweep.
…empotency bug

x#faint.d is valid (weidu accepts it; grammar parses it) but the D
formatter is non-idempotent on it: the source has a tilde-string abutting
THEN with no space (~..NOTVALID)~THEN), and normalizeTransitionText emits
~..~THEN on the first pass but ~..~ THEN on the second. This is a
pre-existing formatter bug, masked until the format sweep got past the
parse failures fixed earlier in this branch; it is orthogonal to the
grammar work and needs a careful fix (add the space only for ~/" -
delimited strings, never % - interpolated names must stay abutted).
Parked here and tracked separately; unpark once the formatter is fixed.
A transition trigger written with no space before THEN (~..NOTVALID)~THEN,
valid WeiDU) formatted to ~..~THEN on the first pass but ~..~ THEN once the
broken line was reparsed, so the formatter was not idempotent and the
external format sweep failed on BG1NPC's x#faint.d.

normalizeTransitionText now inserts a separating space at a ~/"-delimited
string <-> word-character boundary (either direction), so the trigger and the
following keyword are always spaced. %-delimited strings are deliberately
excluded: %var%-interpolated names (%tutu_var%KELDDA, TAZOK%eet_var%) abut
literal text on purpose and must not be split.

Unparks x#faint.d (previously excluded for this bug). Full IE format sweep is
green with it back in; added a CLI idempotence regression test.
@burner1024 burner1024 merged commit 2fb2a57 into master Jul 11, 2026
8 checks passed
Comment thread server/src/translation.ts
const configPath = path.join(this.workspaceRoot, ".bgforge.yml");
if (fs.existsSync(configPath)) return;
try {
fs.writeFileSync(configPath, yaml.stringify({ mls: { translation: { directory: relDir } } }));
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.

2 participants