Skip to content

fix(vba): read the Access ERD export with Access encoding rules - #323

Merged
ardelperal merged 2 commits into
mainfrom
fix/issue-322-erd-encoding
Sep 12, 2026
Merged

ardelperal merged 2 commits into
mainfrom
fix/issue-322-erd-encoding

Conversation

@ardelperal

Copy link
Copy Markdown
Owner

Closes #322

What was wrong

#53 introduced readVbaSource() because Access writes Windows-1252 and accented identifiers are everywhere in Spanish-language projects — but it scoped the routing to extensions (isVbaFamilyFile). The Access structure export is ERD/*.md, so it stayed on the plain fs.readFile(path, 'utf-8') path.

Reproduced against the built dist/, decoding the same document from UTF-8 bytes and from CP1252 bytes exactly as readForExtraction would:

utf-8 on disk  -> ["class:TbSituación","type_member:Código","type_member:Descripción"]
cp1252 on disk -> ["class:TbSituaci<fffd>n","type_member:C<fffd>digo","type_member:Descripci<fffd>n"]

The damage is the asymmetry, not the ugly names. A .sql or .bas naming that same table does get the CP1252 fallback, so it emits a placeholder spelled TbSituación. The ERD declared TbSituaci<fffd>n. The two never matched, so:

  • the ERD declaration was never promoted onto the placeholder the query scanners emit,
  • the field list (type_member + contains) hung off an orphan node nobody references,
  • "what columns does this table have" answered nothing for exactly the tables whose names carry accents — in a Spanish-language Access project, most of the interesting ones.

AccessErdExtractor already carried its own stripBom(), so the encoding problem was half-solved: BOM handled, encoding not.

What changed

isVbaFamilyFile now also accepts the ERD path shape, reusing isAccessErdFile from grammars.ts so the shape keeps one owner and the two cannot drift.

Matching on shape, not extension is load-bearing: adding md to the extension regex would pull every markdown file in every repo through the CP1252 fallback.

And unlike the JSON artifacts in #316, the fallback is correct here. The ERD document comes out of the same Access tooling as the rest of the export and shares its encoding provenance; JSON is UTF-8 by specification and must fail loudly instead. Same reasoning, opposite conclusion, because the inputs differ.

Verification

  • Reverting only src/ fails the routing truth table and the join test, the latter with expected [ 'Estructura_Datos.md', …(3) ] to include 'TbSituación'.
  • The two readVbaSource decode tests pass either way by design — readVbaSource always did CP1252; what this PR changes is which files reach it. They are guards against a future regression in the decoder, not the regression proof. The routing and join tests are.
  • The join test asserts the real thing: with both files CP1252 on disk, the saved query's table reference resolves onto the ERD's own class node, and no node in the ERD file carries a replacement character.
  • pnpm exec vitest run vba extraction-sql-query sql-query-discovery — 74 files, 1220 passed, 1 skipped (the Windows CI leg, run locally on Windows).
  • npx tsc --noEmit — clean.

#53 gave the VBA family an encoding-robust read because Access writes
Windows-1252 and accented identifiers are everywhere in Spanish-language
projects, but it scoped that to extensions. The Access structure export
is ERD/*.md, so it stayed on the plain utf-8 path.

The damage was not cosmetic. A .sql naming a table DID get the CP1252
fallback and emitted a placeholder named TbSituación; the ERD declaring
that same table did NOT and declared a replacement-character spelling.
The two never matched, so the declaration never joined the placeholder
and the table's field list hung off an orphan node — for exactly the
tables whose names carry accents.

Widen isVbaFamilyFile with the ERD path shape, reusing isAccessErdFile
from grammars so the shape has one owner. Matching on shape rather than
extension is load-bearing: .md alone would pull every markdown file in
a repo through the CP1252 fallback.

Unlike the JSON artifacts in #316, the fallback is correct here — the
ERD document comes out of the same Access tooling as the rest of the
export and shares its encoding provenance, where JSON is UTF-8 by
specification and must fail loudly instead.

Refs #322
The first attempt put the ERD path shape inside isVbaFamilyFile by
importing isAccessErdFile into vba-source.ts. That turned a leaf module
into a grammars dependant and broke index-orphan-watchdog on CI: that
suite mocks grammars with a from-scratch object, so the newly imported
helper was undefined and the orchestrator's own index reported
success: false — a failure that looked unrelated to encoding.

Combine the two predicates at the read site instead, as
usesAccessEncoding in extraction/index.ts, the only place that needs
both. isVbaFamilyFile goes back to owning extensions only, grammars
keeps owning the ERD path shape, and vba-source.ts stays a leaf.

Also spread the real module in that suite's grammars mock instead of
rebuilding it. A from-scratch mock silently returns undefined for every
helper it forgot, so any future import from grammars fails there as an
unrelated-looking index error; the intentional stubs still override.

Refs #322
@ardelperal

Copy link
Copy Markdown
Owner Author

Update — the first push failed CI on Linux, and it was this PR's fault.

__tests__/index-orphan-watchdog.test.ts > index parse-pool cleanup (#241) failed with expected false to be true on result.success. The Error: too many open files lines nearby in the log are benign — they come from mcp-staleness-banner.test.ts, which asserts the watcher degrades gracefully, and that suite passed.

Cause. The first attempt put the ERD path shape inside isVbaFamilyFile by importing isAccessErdFile into vba-source.ts. That turned a leaf module into a grammars dependant. The orphan-watchdog suite mocks grammars with a from-scratch object, so the newly imported helper was undefined there, calling it threw, and the orchestrator's own index reported success: false — a failure with no visible connection to encoding.

Fix. The two predicates are combined at the read site instead, as usesAccessEncoding in src/extraction/index.ts — the only place that needs both. isVbaFamilyFile goes back to owning extensions only, grammars.ts keeps owning the ERD path shape, and vba-source.ts stays a leaf.

That suite's grammars mock now spreads the real module rather than rebuilding it. A from-scratch mock silently returns undefined for every helper it forgot, so the next import from grammars would have failed the same way; the intentional stubs still override.

What I should have done the first time: I ran the Windows CI leg locally (vitest run vba …) but not the full suite. That filter never matched index-orphan-watchdog.test.ts, so the failure only surfaced on CI. I ran the full suite locally before this push.

Local full suite on Windows: 21 failed | 3775 passed. Those 21 are the documented Windows-only failures in extraction.test.ts, multi-repo-workspace.test.ts, npm-sdk.test.ts and worktree-detection.test.ts — a clean main worktree on this machine produces the identical 21 failures across the identical 4 files. This branch adds none.

@ardelperal
ardelperal merged commit e125115 into main Sep 12, 2026
5 checks passed
@ardelperal
ardelperal deleted the fix/issue-322-erd-encoding branch September 12, 2026 20:30
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.

fix(vba): a CP1252 Access ERD export corrupts table names and breaks their join

1 participant