# Batch 9 — VS Code Extension — Findings

**Date:** 2026-03-26
**Files reviewed:** `diagnostics.js`, `diagnostics/validationScheduler.js`, `codeActionProvider.js`, `completionProvider.js`, `hoverProvider.js`, `definitionProvider.js`, `templateProvider.js`, `config/standardsConfig.js`, `commands/batchValidateTxtFolder.js`, `commands/discoveryReport.js`, `commands/setupWizard.js`
**Status:** Complete

---

## Summary

The extension layer uses `require('vscode')` legitimately throughout — no architecture violations.

One **Critical** runtime crash: `diagnostics.js` calls `path.normalize` inside `normalizeFilePath`
but never imports the `path` module. Any include dependency change (file create/modify/delete on
disk) triggers this path and throws `ReferenceError: path is not defined`, silently breaking
dependent-document revalidation.

One **Major** correctness defect: the `createIndexSaveRestoreFixes` quick fix in
`codeActionProvider.js` generates `INDEX_SAVE_POSITION("handleVar")` using the handle variable
name as the argument — but `INDEX_SAVE_POSITION` takes a table synonym string, not a handle
variable. Any user who accepts this quick fix will have incorrect code inserted.

Two command files (`batchValidateTxtFolder.js`, `discoveryReport.js`) both contain leftover debug
`showInformationMessage` calls that fire on every command invocation, and both re-`require('vscode')`
inside the function body despite importing it at the top of the file.

The remaining issues are Minor and Observations — mostly around synchronous blocking in
`discoveryReport.js`, incomplete TXT glob pattern, and mismatched JSDoc in `hoverProvider.js`.

---

## Findings

### `diagnostics.js`

#### [CRITICAL] `path` module used but not imported — line 91–92

**Issue:** `normalizeFilePath` at line 91 calls `path.normalize(filePath)` and `.toLowerCase()`.
There is no `const path = require("path")` anywhere in this file. The function is called at line
142 inside `scheduleDependentRevalidations`, which is in turn called from:
- `handleIncludeFileSystemEvent` (line 176) — fired on any watched `.txt`/`.TXT` file change
- `handleDocumentChanged` (line 187) — fired on any document edit when the doc has a path

At runtime, the first include-related file event throws `ReferenceError: path is not defined`.
The `scheduleDependentRevalidations` function is wrapped in no try/catch, so the error propagates
to the event handler and silently kills dependent-document revalidation for the remainder of the
VS Code session.

**Recommendation:** Add `const path = require("path");` to the imports at the top of the file.
This is the only fix required — the logic in `normalizeFilePath` is otherwise correct.

---

### `codeActionProvider.js`

#### [MAJOR] `createIndexSaveRestoreFixes` generates wrong argument for `INDEX_SAVE_POSITION` — lines 564–576

**Issue:** The quick fix for ACU-PAT-003 extracts the handle variable name from the flagged
line:

```js
const handleMatch = lineText.match(/INDEX_(?:READ_BY_KEY|LOAD_KEY_\w+)\s*\(\s*(\w+)/);
const indexName = handleMatch ? handleMatch[1] : "indexHandle";
```

`indexName` is the **handle variable** (e.g., `lMyHandle`). The generated snippet then emits:

```
INDEX_SAVE_POSITION("lMyHandle")
INDEX_RESTORE_POSITION("lMyHandle")
```

`INDEX_SAVE_POSITION` takes a **table synonym string** as its first argument (e.g.,
`"MEMBER"`) — not a handle variable name. The generated code is semantically wrong. A user
who accepts the quick fix gets a syntactically valid but semantically incorrect insertion.

**Recommendation:** The quick fix cannot reliably derive the correct table synonym from context.
Options:
1. Generate a snippet placeholder instead: `INDEX_SAVE_POSITION("$\{1:TABLE_SYNONYM\}")` so
   the user is prompted to fill in the correct value.
2. Remove the auto-insert fix entirely and keep only the suppress action — ACU-PAT-003 is a
   low-severity recommendation and inserting wrong code is worse than no fix.

---

#### [MINOR] `createRemoveDatabaseFieldPrefixFix` and `createVariableRenameFix` only handle single-variable VAR declarations — lines 382, 422

**Issue:** Both methods use `lineText.match(/VAR\s+([...]):\s*/i)` which captures only the first
variable name. For `VAR hFoo, hBar : STRING`, only `hFoo` is identified and renamed. `hBar` is
silently ignored. The rename action label says "Rename 'hFoo' to 'Foo'" but `hBar` is not
touched, leaving the file in a partially-renamed state.

**Recommendation:** Parse all variable names from the VAR declaration line (using
`parseVarDeclaration` from `parser.js`) or document the single-variable limitation.

---

### `commands/batchValidateTxtFolder.js`

#### [MINOR] Debug `showInformationMessage` and duplicate `require` inside `runBatchValidation` — lines 92–93

**Issue:** Line 92 redeclares `const vscode = require('vscode')` inside the function body,
shadowing the module-level import at line 1. Line 93 calls
`vscode.window.showInformationMessage('[Acurity] runBatchValidation handler invoked')` — a
development debug message that fires a notification popup every time the command is run.

**Recommendation:** Remove lines 92–93. The module-level `vscode` import is sufficient.

---

### `commands/discoveryReport.js`

#### [MINOR] Debug `showInformationMessage` and duplicate `require` inside `runDiscoveryReport` — lines 36–37

**Issue:** Same pattern as `batchValidateTxtFolder.js`: line 36 re-`require`s vscode, line 37
emits a debug notification on every invocation.

**Recommendation:** Remove lines 36–37. The module-level `vscode` import is sufficient.

---

#### [MINOR] `workspaceFolders[0]` accessed without null check in `chooseOutputFile` — line 27

**Issue:** `chooseOutputFile` accesses `vscode.workspace.workspaceFolders[0].uri.fsPath`
unconditionally. If `workspaceFolders` is null or empty — possible in a window-only VS Code
instance with no folder open — this throws `TypeError: Cannot read properties of undefined`.
`chooseTargetFolder` (line 12) already handles the no-workspace case, but `chooseOutputFile` is
an independent function and can be called without that guard.

**Recommendation:** Guard: `const root = vscode.workspace.workspaceFolders?.[0]?.uri.fsPath ?? "";`

---

#### [MINOR] Synchronous `fs.writeFileSync` in async context — line 119

**Issue:** `fs.writeFileSync(outUri.fsPath, outData, 'utf8')` blocks the Node.js event loop
inside an async function. For large reports this causes UI freeze. The rest of the file uses
`await vscode.workspace.fs.writeFile(...)` pattern (see `batchValidateTxtFolder.js:170`).

**Recommendation:** Replace with `await vscode.workspace.fs.writeFile(outputUri, Buffer.from(outData, "utf8"))` matching the established pattern in the sibling command.

---

#### [MINOR] `findFiles` glob only matches lowercase `.txt`, misses `.TXT` — line 41

**Issue:** `'**/*.txt'` only matches lowercase extensions. On case-sensitive file systems
(Linux), `.TXT` files are not found. `batchValidateTxtFolder.js` uses `**/*.{txt,TXT}`.

**Recommendation:** Change to `'**/*.{txt,TXT}'` to match the sibling command's pattern.

---

### `hoverProvider.js`

#### [MINOR] Misplaced JSDoc — `buildCharToTypeMap` has hover-content description — line 372

**Issue:** The JSDoc comment at line 372 reads "Builds hover content for a variable. @param
{Object} variableInfo" but the function immediately below it (line 377) is `buildCharToTypeMap`.
The docstring belonging to `buildVariableHoverContent` (line 400) has been placed above the
wrong function. `buildVariableHoverContent` has no JSDoc.

**Recommendation:** Move the JSDoc to line 400 (above `buildVariableHoverContent`). Add a
correct JSDoc to `buildCharToTypeMap`.

---

#### [MINOR] `search(/[A-Z]/)` returns -1 for all-lowercase variable names — line 458

**Issue:** `variableInfo.name.search(/[A-Z]/)` returns -1 when the name has no uppercase
characters (e.g., `zfoo`). `String.prototype.substring(-1)` treats -1 as 0, returning the
whole string. The `recommendedName` becomes `recommendedPrefix + fullVariableName` rather than
`recommendedPrefix + nameWithoutPrefix`. The hover note would display an incorrect suggestion.

This is the same logic duplication from `namingValidator.js` — both files have independent
copies of this prefix-stripping logic, both with the same bug.

**Recommendation:** Extract to a shared helper, or use the `stripExistingVariablePrefix`
function already in `namingValidator.js`. Fix the negative-index edge case.

---

#### [MINOR] Duplicate `require` of `parser` — lines 7 and 15

**Issue:** Line 7 and line 15 both `require("../../core/src/parser")`. Node.js caches the
module so there is no double-load, but two separate destructuring imports from the same module
is inconsistent and confusing.

**Recommendation:** Merge into a single `require` statement.

---

### `commands/setupWizard.js`

#### [MINOR] `runSetupWizard` launched in `setTimeout` without error handling — line 265

**Issue:** `checkFirstRun` fires `runSetupWizard` inside a `setTimeout` callback but does not
`await` it and has no try/catch:

```js
setTimeout(() => {
    runSetupWizard(context, true);  // async function, returned Promise not handled
}, 2000);
```

If `runSetupWizard` throws (e.g., VS Code API throws during multi-step dialog), the resulting
unhandled promise rejection is reported to the VS Code runtime.

**Recommendation:**
```js
setTimeout(async () => {
    try { await runSetupWizard(context, true); } catch (_) { /* swallow */ }
}, 2000);
```

---

## Observations

### `diagnostics.js` — `forEach(async …)` for initial document validation — line 66

`vscode.workspace.textDocuments.forEach(async (document) => { ... })` launches async work that
`forEach` cannot await. Errors from `scheduleIncludeCacheWarmup` on startup are silently dropped.
This is a common VS Code pattern but any errors here are invisible. Using `Promise.all` or
logging the error would be safer.

---

### `hoverProvider.js` — three sequential full-document scans in `findVariableInfo`

`findVariableInfo` (line 255) performs three sequential full scans: (1) build function scopes,
(2) find declaration, (3) find assignments. On large files with frequent hover requests this is
O(3n) per hover event. Using `collectDocumentFacts` (already called on the same document at
line 57) could eliminate at least the function scope and declaration scans.

---

### `completionProvider.js` — synchronous `collectDocumentFacts` on every completion

`collectDocumentFacts(document)` is called synchronously inside `provideCompletionItems` on
every completion trigger. While `collectDocumentFacts` is memoized by document version, the
memoization check itself happens synchronously and will re-run the full analysis on any edit.
This is acceptable for small files but could cause completion latency on large ones.

---

### `config/standardsConfig.js` — `getStandardsConfig()` called per diagnostic

`getRuleSeverity` calls `getStandardsConfig()` (→ `vscode.workspace.getConfiguration(...)`)
on every invocation. During validation of a 1 000-line file, this is called once per diagnostic
emitted — potentially hundreds of times. `vscode.workspace.getConfiguration` is documented as
cheap but it is not free. Caching per validation run (passing the config object to
`collectDiagnostics`) would be more efficient.

---

### `templateProvider.js` — hardcoded Bootstrap CDN link in generated template — line 69

The report template embeds a `<link>` to `https://stackpath.bootstrapcdn.com/bootstrap/...`.
Users in air-gapped environments or those who need specific styling will need to edit every
generated file. This is a cosmetic/usability note rather than a bug.

---

## Summary table

| Severity | Count | Items |
|---|---|---|
| Critical | 1 | `path` not imported in `diagnostics.js` — runtime crash on any file-change event |
| Major | 1 | `createIndexSaveRestoreFixes` generates wrong `INDEX_SAVE_POSITION` argument |
| Minor | 9 | Debug messages/duplicate requires ×2; null-check gap; sync write; TXT glob; misplaced JSDoc; substring(-1); unhandled Promise in setTimeout; duplicate parser require |
| Observation | 5 | forEach async; triple scan in findVariableInfo; sync completion; per-diagnostic config read; hardcoded CDN |
