refactor(binary): share file-derived ParseOptions across CLI and editor

burner1024 · burner1024 · commit 987bbb35837d · 2026-05-06T11:17:09.000Z
Split ParseOptions into two named axes:
  - file-derived (a function of where the file sits on disk — today the
    sibling proto/ pidResolver auto-load)
  - frontend-preference (skipMapTiles for editor render perf,
    gracefulMapBoundaries for CLI flag exposure / editor fallback)

Move the file-derived axis behind a single library function,
buildFileDerivedParseOptions(filePath), in @bgforge/binary. CLI and the
VS Code custom editor both delegate to it, so a behavior added there
propagates to every frontend instead of silently affecting only one.
Add a parity contract test that catches future divergence at PR time.

The editor also gains a graceful-map auto-fallback: best-effort display
is its job, so a strict parse error on a .map retries with
gracefulMapBoundaries enabled. The CLI keeps strict-fail semantics
because batch verification needs the error to surface. The actually-used
options propagate onto BinaryDocument so incremental edits and revert
reuse the same shape.
diff --git a/binary/src/cli.ts b/binary/src/cli.ts
@@ -16,9 +16,7 @@ import {
     createBinaryJsonSnapshot,
     loadBinaryJsonSnapshot,
     parseBinaryJsonSnapshot,
-    resolvePidSubType,
-    loadProDirResolver,
-    composePidResolvers,
+    buildFileDerivedParseOptions,
 } from "./index";
 import {
     type FileResult,
@@ -36,35 +34,25 @@ const CLI_PARSE_OPTIONS: ParseOptions = {
 const CLI_QUIET = process.argv.includes("-q") || process.argv.includes("--quiet");
 
 /**
- * Builds per-file ParseOptions, layering a sibling proto/ override resolver
- * on top of the bundled vanilla Fallout 2 table when the input is a MAP and
- * a `<map dir>/../proto/` tree exists. Reports stats to stderr (unless -q).
+ * Builds per-file ParseOptions for the CLI: file-derived options come from
+ * the shared `@bgforge/binary` builder (today: sibling proto/ pidResolver),
+ * the CLI's own preference axes (--graceful-map) layer on top. Reports
+ * proto-load stats to stderr (unless -q).
  */
 function buildParseOptionsForFile(filePath: string): ParseOptions {
-    if (path.extname(filePath).toLowerCase() !== ".map") {
-        return CLI_PARSE_OPTIONS;
-    }
-
-    const protoBaseDir = path.resolve(path.dirname(filePath), "..", "proto");
-    if (!fs.existsSync(protoBaseDir)) {
-        return CLI_PARSE_OPTIONS;
-    }
-
-    const { resolver, stats } = loadProDirResolver(protoBaseDir);
-    if (stats.filesScanned === 0) {
-        return CLI_PARSE_OPTIONS;
-    }
+    const fileDerived = buildFileDerivedParseOptions(filePath);
 
-    if (!CLI_QUIET) {
+    if (!CLI_QUIET && fileDerived.diagnostics) {
+        const { protoDir, stats } = fileDerived.diagnostics;
         const errSuffix = stats.errors.length > 0 ? `, ${stats.errors.length} errors` : "";
         console.error(
-            `Loaded ${stats.subtypesResolved} proto overrides from ${protoBaseDir} ` +
+            `Loaded ${stats.subtypesResolved} proto overrides from ${protoDir} ` +
                 `in ${stats.durationMs.toFixed(0)}ms${errSuffix}`,
         );
         for (const err of stats.errors) console.error(`  ${err}`);
     }
 
-    return { ...CLI_PARSE_OPTIONS, pidResolver: composePidResolvers(resolver, resolvePidSubType) };
+    return { ...CLI_PARSE_OPTIONS, ...(fileDerived.pidResolver ? { pidResolver: fileDerived.pidResolver } : {}) };
 }
 
 async function processFile(filePath: string, mode: OutputMode): Promise<FileResult> {
diff --git a/binary/src/index.ts b/binary/src/index.ts
@@ -68,6 +68,11 @@ export {
     type ProResolverResult,
     type ProResolverStats,
 } from "./pro-resolver-loader";
+export {
+    buildFileDerivedParseOptions,
+    type FileDerivedParseOptions,
+    type FileDerivedDiagnostics,
+} from "./parse-options";
 
 // Side-effect: register the bundled parsers on the registry.
 import { proParser } from "./pro";
diff --git a/binary/src/parse-options.ts b/binary/src/parse-options.ts
@@ -0,0 +1,71 @@
+/**
+ * Shared file-derived ParseOptions builder.
+ *
+ * `ParseOptions` has two distinct axes:
+ *   - **File-derived** — values that are a function of where the file sits
+ *     on disk (which sibling resources exist, which mod tree it belongs to).
+ *     Must agree across frontends; divergence here is always a bug.
+ *   - **Frontend-preference** — values that legitimately differ per caller
+ *     (`skipMapTiles` for editor render perf, `gracefulMapBoundaries` for
+ *     CLI flag exposure). These stay per-frontend.
+ *
+ * This module owns the file-derived axis. Both the CLI and the VS Code
+ * editor call `buildFileDerivedParseOptions(filePath)` and merge their own
+ * preferences on top. Adding a new file-derived behavior — e.g. scanning
+ * a sibling manifest for proto-search-path overrides — is a single edit
+ * here that propagates to every caller.
+ *
+ * Today the only file-derived option is `pidResolver`, auto-loaded from
+ * `<mapDir>/../proto/` (the standard Fallout 2 mod tree layout). When the
+ * sibling tree exists and contains at least one matching .pro file, the
+ * builder layers a filesystem resolver over the bundled vanilla defaults so
+ * MAP decoding covers modded pids. Otherwise it returns empty options and
+ * `parser.parse` falls back to its own bundled-table default.
+ */
+
+import * as fs from "fs";
+import * as path from "path";
+import { resolvePidSubType } from "./pid-resolver";
+import { loadProDirResolver, composePidResolvers, type ProResolverStats } from "./pro-resolver-loader";
+import type { ParseOptions } from "./types";
+
+export interface FileDerivedDiagnostics {
+    /** Absolute path of the proto/ tree that was scanned. */
+    readonly protoDir: string;
+    /** Stats reported by `loadProDirResolver` — files scanned, errors, duration. */
+    readonly stats: ProResolverStats;
+}
+
+export interface FileDerivedParseOptions extends Pick<ParseOptions, "pidResolver"> {
+    /**
+     * Optional diagnostics from filesystem-touching scans (e.g. proto/ load
+     * stats). Frontends use this for stderr or status-line logging; the
+     * parser itself never reads it.
+     */
+    readonly diagnostics?: FileDerivedDiagnostics;
+}
+
+/**
+ * Build the file-derived axis of ParseOptions for `filePath`. Returns
+ * empty options when there is nothing on disk to enrich the parse with.
+ */
+export function buildFileDerivedParseOptions(filePath: string): FileDerivedParseOptions {
+    if (path.extname(filePath).toLowerCase() !== ".map") {
+        return {};
+    }
+
+    const protoBaseDir = path.resolve(path.dirname(filePath), "..", "proto");
+    if (!fs.existsSync(protoBaseDir)) {
+        return {};
+    }
+
+    const { resolver, stats } = loadProDirResolver(protoBaseDir);
+    if (stats.filesScanned === 0) {
+        return {};
+    }
+
+    return {
+        pidResolver: composePidResolvers(resolver, resolvePidSubType),
+        diagnostics: { protoDir: protoBaseDir, stats },
+    };
+}
diff --git a/binary/test/parse-options.test.ts b/binary/test/parse-options.test.ts
@@ -0,0 +1,101 @@
+/**
+ * Shared file-derived ParseOptions builder.
+ *
+ * `buildFileDerivedParseOptions(filePath)` is the single source of truth for
+ * the *file-derived* axis of `ParseOptions`: any setting whose value is a
+ * function of where the file sits on disk (which sibling resources exist,
+ * which mod tree it belongs to, etc.). Both the CLI and the editor call this
+ * so a behavior added here propagates to every frontend.
+ *
+ * Frontend-preference axes (`skipMapTiles`, `gracefulMapBoundaries`) stay
+ * out of this builder by design — those legitimately differ per caller.
+ */
+
+import { describe, it, expect, beforeEach, afterEach } from "vitest";
+import * as fs from "fs";
+import * as path from "path";
+import * as os from "os";
+import { buildFileDerivedParseOptions } from "../src/parse-options";
+
+const FIXTURES = path.resolve("client/testFixture/proto");
+
+let tmpDir: string;
+
+beforeEach(() => {
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "fgbin-fdo-"));
+});
+
+afterEach(() => {
+    fs.rmSync(tmpDir, { recursive: true, force: true });
+});
+
+function makeMapDirWithProto(): string {
+    const mapsDir = path.join(tmpDir, "maps");
+    const protoDir = path.join(tmpDir, "proto");
+    fs.mkdirSync(mapsDir);
+    fs.mkdirSync(path.join(protoDir, "items"), { recursive: true });
+    fs.mkdirSync(path.join(protoDir, "scenery"), { recursive: true });
+    fs.copyFileSync(path.join(FIXTURES, "items", "00000031.pro"), path.join(protoDir, "items", "00000031.pro"));
+    fs.copyFileSync(path.join(FIXTURES, "scenery", "00000008.pro"), path.join(protoDir, "scenery", "00000008.pro"));
+    return path.join(mapsDir, "fake.map");
+}
+
+describe("buildFileDerivedParseOptions", () => {
+    it("returns empty options for a non-.map file path", () => {
+        const opts = buildFileDerivedParseOptions("/some/where/file.pro");
+        expect(opts.pidResolver).toBeUndefined();
+        expect(opts.diagnostics).toBeUndefined();
+    });
+
+    it("returns empty options for a .map file with no sibling proto/ dir", () => {
+        const mapsDir = path.join(tmpDir, "maps");
+        fs.mkdirSync(mapsDir);
+        const opts = buildFileDerivedParseOptions(path.join(mapsDir, "any.map"));
+        expect(opts.pidResolver).toBeUndefined();
+        expect(opts.diagnostics).toBeUndefined();
+    });
+
+    it("returns a pidResolver covering items from a sibling proto/items/", () => {
+        const mapPath = makeMapDirWithProto();
+        const opts = buildFileDerivedParseOptions(mapPath);
+        expect(opts.pidResolver).toBeDefined();
+        // 00000031.pro is a vanilla ammo proto (subType 4 / Ammo).
+        expect(opts.pidResolver!(31)).toBe(4);
+    });
+
+    it("returns a pidResolver covering scenery from a sibling proto/scenery/", () => {
+        const mapPath = makeMapDirWithProto();
+        const opts = buildFileDerivedParseOptions(mapPath);
+        // 00000008.pro is a vanilla door proto (subType 0 / Door); pid = (2<<24)|8.
+        expect(opts.pidResolver!(0x02000008)).toBe(0);
+    });
+
+    it("layers proto/ overrides on top of the bundled vanilla defaults", () => {
+        const mapPath = makeMapDirWithProto();
+        const opts = buildFileDerivedParseOptions(mapPath);
+        // pid 161 is a vanilla weapon in the bundled table → subType 3 (Weapon).
+        // It is NOT in the temp proto dir, so this exercises fallback.
+        expect(opts.pidResolver!(161)).toBe(3);
+    });
+
+    it("reports diagnostics so callers can log scan stats", () => {
+        const mapPath = makeMapDirWithProto();
+        const opts = buildFileDerivedParseOptions(mapPath);
+        expect(opts.diagnostics).toBeDefined();
+        expect(opts.diagnostics!.protoDir).toBe(path.join(tmpDir, "proto"));
+        expect(opts.diagnostics!.stats.filesScanned).toBe(2);
+        expect(opts.diagnostics!.stats.subtypesResolved).toBe(2);
+        expect(opts.diagnostics!.stats.errors).toEqual([]);
+    });
+
+    it("returns empty options when sibling proto/ exists but has zero matching files", () => {
+        const mapsDir = path.join(tmpDir, "maps");
+        const protoDir = path.join(tmpDir, "proto");
+        fs.mkdirSync(mapsDir);
+        fs.mkdirSync(path.join(protoDir, "items"), { recursive: true });
+        // Empty proto/items/ — filesScanned will be 0, no resolver attached.
+        const opts = buildFileDerivedParseOptions(path.join(mapsDir, "any.map"));
+        expect(opts.pidResolver).toBeUndefined();
+        expect(opts.diagnostics).toBeUndefined();
+    });
+});
diff --git a/client/src/editors/binaryEditor-parse.ts b/client/src/editors/binaryEditor-parse.ts
@@ -0,0 +1,36 @@
+/**
+ * Editor-side parse helper with graceful-map auto-fallback.
+ *
+ * The editor is best-effort display: a user opening a `.map` file expects
+ * a tree they can inspect, not an error stub. The CLI is batch
+ * verification: the same error must surface so a CI gate can fail. Same
+ * library, different purposes — so the editor (only) retries permissively
+ * when a strict parse returns errors that would prevent display.
+ *
+ * `parseForEditor` returns the actual options used. The caller stores
+ * those on the document so subsequent reparses (incremental field edits,
+ * revert) reuse the same shape — otherwise editing a graceful-loaded map
+ * would silently re-fail on the next byte rebuild.
+ */
+
+import * as path from "path";
+import type { BinaryParser, ParseOptions, ParseResult } from "@bgforge/binary";
+import { buildEditorParseOptions } from "./binaryEditor-parseOptions";
+
+export interface EditorParseOutcome {
+    readonly parseResult: ParseResult;
+    readonly parseOptions: ParseOptions | undefined;
+}
+
+export function parseForEditor(parser: BinaryParser, bytes: Uint8Array, filePath: string): EditorParseOutcome {
+    const baseOptions = buildEditorParseOptions(filePath);
+    const result = parser.parse(bytes, baseOptions);
+    if (path.extname(filePath).toLowerCase() === ".map" && result.errors && result.errors.length > 0) {
+        const gracefulOptions: ParseOptions = { ...baseOptions, gracefulMapBoundaries: true };
+        const fallback = parser.parse(bytes, gracefulOptions);
+        if (!fallback.errors || fallback.errors.length === 0) {
+            return { parseResult: fallback, parseOptions: gracefulOptions };
+        }
+    }
+    return { parseResult: result, parseOptions: baseOptions };
+}
diff --git a/client/src/editors/binaryEditor-parseOptions.ts b/client/src/editors/binaryEditor-parseOptions.ts
@@ -0,0 +1,27 @@
+/**
+ * Editor-side ParseOptions builder.
+ *
+ * Composes `ParseOptions` for the binary custom editor by combining:
+ *   - File-derived options (`buildFileDerivedParseOptions`) — shared with
+ *     the CLI; today this is the sibling proto/ pidResolver auto-load.
+ *   - Editor-preference options — `skipMapTiles: true` to skip tile field
+ *     materialization, which would otherwise cost ~40k field allocations
+ *     per tree render.
+ *
+ * Centralising this here means the parity contract test can call exactly
+ * the same builder the editor uses, and any future file-derived option
+ * added to the library propagates to the editor with no code change.
+ */
+
+import * as path from "path";
+import { buildFileDerivedParseOptions, type ParseOptions } from "@bgforge/binary";
+
+export function buildEditorParseOptions(filePath: string): ParseOptions | undefined {
+    const ext = path.extname(filePath).toLowerCase();
+    if (ext !== ".map") return undefined;
+    const fileDerived = buildFileDerivedParseOptions(filePath);
+    return {
+        skipMapTiles: true,
+        ...(fileDerived.pidResolver ? { pidResolver: fileDerived.pidResolver } : {}),
+    };
+}
diff --git a/client/src/editors/binaryEditor.ts b/client/src/editors/binaryEditor.ts
@@ -8,6 +8,7 @@ import * as path from "path";
 import { randomBytes } from "crypto";
 import {
     type BinaryParser,
+    type ParseOptions,
     type ParseResult,
     parserRegistry,
     formatAdapterRegistry,
@@ -29,6 +30,8 @@ import {
     resolveStringCharset,
 } from "./binaryEditor-lookups";
 import { saveBinaryDocumentArtifacts, writeBinaryJsonSnapshot } from "./binaryEditor-save";
+import { buildEditorParseOptions } from "./binaryEditor-parseOptions";
+import { parseForEditor } from "./binaryEditor-parse";
 import { BinaryEditorRefreshGate } from "./binaryEditor-refreshGate";
 import { BinaryEditorLocalEditTracker } from "./binaryEditor-localEditTracker";
 import { surfaceWebviewRuntimeError } from "../webview-error";
@@ -129,11 +132,11 @@ class BinaryEditorProvider implements vscode.CustomEditorProvider<BinaryDocument
         _openContext: vscode.CustomDocumentOpenContext,
         _token: vscode.CancellationToken,
     ): Promise<BinaryDocument> {
-        const { parseResult, parser } = await this.parseFile(uri);
+        const { parseResult, parser, parseOptions } = await this.parseFile(uri);
         const doc = new BinaryDocument(uri, parseResult, {
             parse: parser.parse.bind(parser),
             serialize: parser.serialize.bind(parser),
-            parseOptions: this.getParseOptions(path.extname(uri.fsPath)),
+            parseOptions,
         });
 
         const subscriptions: vscode.Disposable[] = [];
@@ -317,7 +320,7 @@ class BinaryEditorProvider implements vscode.CustomEditorProvider<BinaryDocument
         try {
             const jsonText = Buffer.from(await vscode.workspace.fs.readFile(jsonUri)).toString("utf8");
             const extension = path.extname(document.uri.fsPath);
-            const parseOptions = this.getParseOptions(extension);
+            const parseOptions = this.getParseOptions(document.uri.fsPath);
             const loaded = loadBinaryJsonSnapshot(jsonText, {
                 proParseOptions: parseOptions,
                 mapParseOptions:
@@ -635,11 +638,15 @@ class BinaryEditorProvider implements vscode.CustomEditorProvider<BinaryDocument
         return parser as EditableBinaryParser;
     }
 
-    private getParseOptions(extension: string): { skipMapTiles?: boolean } | undefined {
-        return extension === ".map" ? { skipMapTiles: true } : undefined;
+    private getParseOptions(filePath: string): ParseOptions | undefined {
+        return buildEditorParseOptions(filePath);
     }
 
-    private async parseFile(uri: vscode.Uri): Promise<{ parseResult: ParseResult; parser: EditableBinaryParser }> {
+    private async parseFile(uri: vscode.Uri): Promise<{
+        parseResult: ParseResult;
+        parser: EditableBinaryParser;
+        parseOptions: ParseOptions | undefined;
+    }> {
         const extension = path.extname(uri.fsPath);
         const parser = this.getEditableParser(extension);
 
@@ -659,14 +666,13 @@ class BinaryEditorProvider implements vscode.CustomEditorProvider<BinaryDocument
                     parse: () => parseResult,
                     serialize: () => new Uint8Array(),
                 },
+                parseOptions: undefined,
             };
         }
 
         const fileData = await vscode.workspace.fs.readFile(uri);
-        return {
-            parseResult: parser.parse(fileData, this.getParseOptions(extension)),
-            parser,
-        };
+        const outcome = parseForEditor(parser, fileData, uri.fsPath);
+        return { parseResult: outcome.parseResult, parser, parseOptions: outcome.parseOptions };
     }
 
     // -- HTML rendering (shell only, data sent via postMessage) --------------
diff --git a/client/test/binaryEditor-graceful-fallback.test.ts b/client/test/binaryEditor-graceful-fallback.test.ts
diff --git a/client/test/binaryEditor-parse-parity.test.ts b/client/test/binaryEditor-parse-parity.test.ts
