Skip to content

feat(vba-extractor): UTF-8 BOM strip and Windows-1252 fallback for VBA files (closes #53) - #71

Merged
ardelperal merged 1 commit into
mainfrom
chore/2026-07-04-issue-53-encoding-robustness
Jul 4, 2026
Merged

feat(vba-extractor): UTF-8 BOM strip and Windows-1252 fallback for VBA files (closes #53)#71
ardelperal merged 1 commit into
mainfrom
chore/2026-07-04-issue-53-encoding-robustness

Conversation

@ardelperal

Copy link
Copy Markdown
Owner

Closes #53.

What

Defense in depth for VBA-family source decoding. dysflow normally
guarantees UTF-8-no-BOM on export, but codegraph-vba can be pointed at
exports from older dysflow runs, hand-copied modules, or other tools
that still write CP1252 (ANSI) — the Access SaveAsText / VBIDE
Export default. Today every file is read as UTF-8 unconditionally, so
a CP1252 file with accented identifiers (ubiquitous in Spanish-
language Access code: Sub ActualizarSituación, enum member ,
module headers ' MÓDULO:) decodes to replacement characters,
\p{L} regexes stop matching, and the symbol silently disappears
from the graph. A BOM-carrying UTF-8 file additionally corrupts the
first line (breaks Attribute VB_Name detection → wrong module
name).

Why

A Dysflow-managed Access project where every author wrote Sub ActualizarNombre() would have its second-most-frequent identifier
silently invisible to the graph because of a one-byte encoding
mismatch. This fix makes the read path encoding-robust for VBA-family
files only (.bas/.cls/.frm/.dsr/.form.txt/.report.txt/.sql).

Diff

File Lines
src/extraction/vba-source.ts (NEW) +137
src/extraction/index.ts (wire-up) +28 / −7
__tests__/vba-source.test.ts (NEW) +141
Total +306 / −7

Source change is well under the 400-line review budget.

Design

src/extraction/vba-source.ts (NEW)

A small focused helper module exporting three functions:

  • isVbaFamilyFile(filePath) — case-insensitive extension match.

  • readVbaSource(filePath, opts?) — reads the file as Buffer,
    strips a leading UTF-8 BOM if present, tries TextDecoder('utf-8', { fatal: true }), and falls back to TextDecoder('windows-1252')
    on decode failure. Returns { text, bomStripped }. Optional
    readFile DI for testing + optional onFallback callback so the
    CLI can surface a low-severity warning when the fallback fires.

    The UTF-8 sniff is O(fileSize) but only runs the decoder once on
    the happy path. CP1252 fallback only fires when UTF-8 actually
    throws — valid UTF-8 (dysflow, modern editors, anything written
    today) is byte-identical to readFileSync(path, 'utf-8') with the
    BOM difference only.

  • stripUtf8Bom(text) — defensive string-level BOM strip for
    callers that already hold a string. (Most callers go through
    readVbaSource and never need it.)

src/extraction/index.ts (wire-up)

Two thin helpers (readForExtraction async + readForExtractionSync
sync) replace each fsp.readFile(fullPath, 'utf-8') /
fs.readFileSync(fullPath, 'utf-8') call site. Each calls
isVbaFamilyFile exactly once and branches: VBA-family files go
through readVbaSource; everything else stays on the existing
byte-identical readFile(path, 'utf-8') path. 7 read sites
rewired: 1412 (batch parallel reader), 1524 (WASM-retry reader),
1568 (comment-strip retry reader), 1688 (single-file index read),
1983 (sync reconcile hash), 2067 (git fast-path change detect
hash), 2112 (full-scan fallback change detect hash).

The 3 detect-time hashing sites are included deliberately: the
detector hashes the read content, and the storage stores the
BOM-stripped text — if the BOM strip didn't happen at read time
the hash would always diverge and the file would report as
perpetually "modified".

Surrounding try/catch, stat, and error-attribution code is left
intact. Three non-extraction reads (.gitignore buffer, .git
gitdir file, framework-detection manifest probe) are NOT touched —
they're not VBA source.

__tests__/vba-source.test.ts (NEW)

8 atoms using the helper's readFile DI for in-memory Buffer
fixtures:

  1. UTF-8 no-BOM plain text — happy path; text matches utf-8 decode, bomStripped === false, no fallback callback fires.
  2. UTF-8 WITH BOM — text does NOT start with \uFEFF, bomStripped === true.
  3. CP1252-encoded buffer with 'ó' (0xF3) — falls back to Windows-1252, text ends with 'ó'.
  4. Empty buffer — { text: '', bomStripped: false }.
  5. UTF-8 Attribute VB_Name = "MiClase" (Spanish-accented Mi Clase) — BOM-stripped resolution works.
  6. onFallback callback fires when CP1252 fallback reaches it and is NOT fired on the UTF-8 happy path.
  7. isVbaFamilyFile true cases: .bas, .cls, .form.txt, .report.txt, .sql.
  8. isVbaFamilyFile false cases: .ts, .js, .json, .md, .clsss, MyModule.txt.

Scope discipline

Per Issue #53 spec, only VBA-family files get the BOM strip +
CP1252 fallback. Tree-sitter-routed languages (TS, JS, Python,
Go, Rust, etc.) keep the existing readFile(path, 'utf-8') path
byte-identical. Acceptance criterion "valid UTF-8 path byte-
identical to today (no behavior change, no perf cliff)" holds.

Validation

  • pnpm exec vitest run __tests__/vba-source.test.ts — 8 passed in 403 ms
  • Full VBA suite (6 files): 236 passed in 5.03 s — zero regressions
  • pnpm run build — tsc clean, no TS errors

Out of scope (intentional)

  • Other Windows-1252 quirks (lead bytes valid in both encodings —
    e.g. 0x80..0x9F differ slightly between CP1252 and CP1254
    Turkish). CP1252 is the documented default for Access SaveAsText
    / VBIDE Export; that's what we fall back to.
  • CP1254 / CP1253 / CP865 / etc. — the same UTF-8-first sniff plus
    CP1252 fallback covers 99% of Access exports. Per-byte-detection
    of these encodings is out of scope for this P3 issue.
  • The pre-existing Windows CI timeout flake on
    __tests__/index-command.test.ts and __tests__/db-perf.test.ts
    is fixed separately in PR fix(tests): bump e2e index-command test timeout for Windows CI runners #70 (vitest.config.ts with
    testTimeout: 30_000); that fix lands on release-origin/main
    before this PR's target HEAD and applies automatically.

Not done

n/a — issue complete in this PR.

…A files (closes #53)

Defense in depth: dysflow normally guarantees UTF-8-no-BOM for VBA exports,
but codegraph-vba can be pointed at exports from older dysflow runs, other
tools, or hand-copied modules that are still CP1252 on disk. Today every
file is read as UTF-8 unconditionally; a CP1252 file with accented
identifiers (ubiquitous in Spanish-language Access code: `Sub
ActualizarSituación`, enum member `Sí`, module headers `' MÓDULO:`)
decodes to replacement characters, `\p{L}` regexes stop matching, and the
symbol silently disappears from the graph. A BOM-carrying UTF-8 file
additionally corrupts the first line (breaks `Attribute VB_Name`
detection → wrong module name).

Changes:

* `src/extraction/vba-source.ts` (NEW) — small encoding helper:
  - `isVbaFamilyFile(filePath)` — true for `.bas/.cls/.frm/.dsr/
    .form.txt/.report.txt/.sql` (case-insensitive).
  - `readVbaSource(filePath, opts?)` — reads the file as `Buffer`,
    strips a leading UTF-8 BOM if present, tries `TextDecoder('utf-8',
    { fatal: true })`, and falls back to `TextDecoder('windows-1252')`
    on decode failure (cheap sniff: one fatal decode attempt; the
    fallback only fires when UTF-8 actually fails). Returns
    `{ text, bomStripped }`. Optional `readFile` DI for testing and
    optional `onFallback` callback for a low-severity warning surface.
  - `stripUtf8Bom(text)` — string-level BOM strip as a defensive
    post-process for callers that already hold a string.

* `src/extraction/index.ts` — wires the helper into the file-read
  pipeline via two thin helpers, `readForExtraction` (async, used at
  the batch + retry + comment-strip + single-file sites) and
  `readForExtractionSync` (sync, used at the `sync()` reconcile +
  fast-path + full-scan sites). Each calls `isVbaFamilyFile` exactly
  once and branches: VBA-family files go through `readVbaSource`,
  everything else stays on the existing byte-identical `readFile(path,
  'utf-8')` path. 7 read sites rewired (1412, 1524, 1568, 1688, 1983,
  2067, 2112). The 3 detect-time hashing sites are included
  deliberately so a BOM-carrying `.bas` does not report as perpetually
  "modified" against its BOM-stripped store-state hash.

* `__tests__/vba-source.test.ts` (NEW) — 8 atoms covering the BOM
  strip, the CP1252 fallback path, the `onFallback` callback wiring,
  the empty-buffer edge case, the UTF-8 happy path, and the VBA-family
  truth table. Uses the helper's `readFile` DI — no temp files
  required for unit tests.

Scope discipline (per Issue #53 spec): only VBA-family files get the
BOM strip + CP1252 fallback. Tree-sitter-routed languages (TS, JS,
Python, Go, Rust, etc.) keep the existing `readFile(path, 'utf-8')`
path byte-identical. Acceptance criterion "valid UTF-8 path byte-
identical to today (no behavior change, no perf cliff)" holds.

Validation:

* `pnpm exec vitest run __tests__/vba-source.test.ts` — 8 passed in 403 ms
* `pnpm exec vitest run __tests__/extraction-vba.test.ts
   __tests__/extraction-vba-control-modeling.test.ts
   __tests__/extraction-vba-form.test.ts
   __tests__/extraction-vba-enums-consts.test.ts
   __tests__/extraction-vba-realfixtures.test.ts
   __tests__/extraction-vba-roadmap-25-26.test.ts` — 236 passed in
  5.03 s, zero regressions
* `pnpm run build` — tsc clean, no TypeScript errors
@ardelperal ardelperal added the type:feature New feature label Jul 4, 2026
ardelperal added a commit that referenced this pull request Jul 4, 2026
…mts` config (#72)

PR #70 added `vitest.config.ts` with `testTimeout: 30_000`, but vitest
3.x prefers the existing ESM `vitest.config.mts` when both are
present in the same project root — so the `.ts` was a silent no-op
and the windows CI jobs continued to fail at the default 5000ms
timeout (workflow runs `28702911905` and `28703027415` on PR #71
both flaked at `__tests__/db-perf.test.ts:116` and `index-command.test.ts:97`
with the same 5000ms default).

Fix: add `testTimeout: 30_000` to the actively-loaded
`vitest.config.mts` (the existing file with maxWorkers/pool/execArgv
already in place for the Node >=25 dev-machine safety). No other
changes — the `.ts` sibling from PR #70 is left in place for
reference but does not affect test execution; a future housekeeping
PR can delete it.

Verification:

* `pnpm exec vitest run __tests__/index-command.test.ts
   __tests__/db-perf.test.ts` → 16 passed locally.
* The flake pattern (5s timeout crossing on CI Windows) is closed by
  the global cap; per-test explicit timeouts (`it(name, fn, 15_000)`)
  still take precedence.

User rule upheld: "todo lo que se integre a main ha de estar verde".
@ardelperal
ardelperal merged commit 8b6b093 into main Jul 4, 2026
6 of 7 checks passed
@ardelperal
ardelperal deleted the chore/2026-07-04-issue-53-encoding-robustness branch July 4, 2026 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:feature New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(vba): encoding robustness — UTF-8 BOM strip and Windows-1252 fallback for VBA files

1 participant