Skip to content

fix(vba): remove duplicate Dysflow artifact extraction - #315

Merged
ardelperal merged 1 commit into
mainfrom
fix/issue-314-dysflow-double-extraction
Sep 12, 2026
Merged

ardelperal merged 1 commit into
mainfrom
fix/issue-314-dysflow-double-extraction

Conversation

@ardelperal

Copy link
Copy Markdown
Owner

Closes #314

What was wrong

The #154 refactor lifted the three Dysflow sub-extractors (VbaFormExtractor, VbaTestManifestExtractor, VbaTestSequenceExtractor) into dysflowExportResolver, but the pre-refactor dispatch ladder in src/extraction/tree-sitter.ts was never removed. Both ran inside a single extractFromSource call, and dysflowExportResolver.detect() is true for any project carrying a Dysflow artifact — so every real export parsed each .form.txt / .report.txt / test manifest / test sequence twice per index.

Measured on the pre-fix build:

manifest | no-fw nodes=1 refs=1  ||  with-fw nodes=2 refs=2
sequence | no-fw nodes=1 refs=1  ||  with-fw nodes=2 refs=2
form     | no-fw nodes=4 refs=1  ||  with-fw nodes=8 refs=2

Deterministic node IDs collapse the duplicate nodes on insert, so what survived was the doubled parse cost plus duplicate unresolved_refs rows for every manifest entry and sequence procedure.

What changed

extract() (and its pickSubExtractor helper) is removed from dysflowExportResolver. Per-file dispatch stays in the tree-sitter.ts ladder, which is the only site that runs on every path into extraction — including the many callers that pass no frameworkNames at all (single-file re-index, library consumers, the extraction unit tests). Removing the ladder instead would have silently emptied those paths.

The resolver keeps detect() and resolve(), and its docstring now states where extraction actually lives and why re-adding an extract() here reintroduces the duplication.

Second defect fixed on the way

The removed extract() never checked the dysflowExport flag. With the framework registered, vba.dysflowExport: false still emitted the full Dysflow shape instead of the file-only node it promises. Test D pins the opt-out under that combination.

Verification

  • pnpm exec vitest run vba extraction-sql-query sql-query-discovery — 72 files, 1207 passed, 1 skipped (the Windows CI leg, run locally on Windows).
  • npx tsc --noEmit — clean.
  • New Test D (8 cases) covers frameworkNames: ['dysflow-export'] with dysflowExport: true, the combination the real index path uses and the one nothing exercised before. Reverting only src/extraction/frameworks/dysflow-export.ts fails all 8; with the fix all 20 tests in the suite pass.

The #154 refactor lifted the three Dysflow sub-extractors into a
FrameworkResolver but never removed the dispatch ladder in
tree-sitter.ts, so both ran inside a single extractFromSource call.
dysflowExportResolver.detect() is true for any project carrying a
Dysflow artifact, so every real export parsed each .form.txt,
.report.txt, test manifest and test sequence twice per index and stored
duplicate unresolved_refs rows.

Drop extract() from the resolver and leave per-file dispatch to the
tree-sitter.ts ladder, which is the only site that runs on every path
into extraction, including callers that pass no frameworkNames. As a
side effect the vba.dysflowExport: false opt-out is honoured again when
the framework is registered; the resolver's extract() ignored the flag.

Test D covers frameworkNames: ['dysflow-export'] with dysflowExport:
true, the combination the real index path uses and the one no test
exercised before.

Refs #314
@ardelperal
ardelperal merged commit 7b4319e into main Sep 12, 2026
5 checks passed
@ardelperal
ardelperal deleted the fix/issue-314-dysflow-double-extraction branch September 12, 2026 19:44
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): remove duplicate Dysflow artifact extraction

1 participant