feat(vba): recognise the module-variable error channel - #291
Merged
Conversation
Error propagation in an Access codebase of this shape does not use VBA's error mechanism. 16 handlers out of 3,774 re-raise; `Err.Raise 1000` unwinds exactly one frame and the house guard `If Err.Number <> 1000` means "an inner procedure already wrote a human-readable message". The message itself travels through a field the failing procedure writes and the caller reads. That is module-variable data flow, which #251 already models as `property-set` / `property-get` references onto a `variable` node. This change only labels it: a read or write of a channel variable now carries `metadata.errorChannel: true`, on the reference and on the resolved edge. No new node kind, no new edge kind, and no new row — the corpus indexes to byte-identical `nodesByKind` / `edgesByKind` / `unresolvedByKind` totals (26,089 / 29,521 / 26,755, unchanged against origin/main). Decisions taken: - The channel names and the write matcher move into a new leaf module, `src/extraction/vba/error-channel.ts`. `errors.ts` owned both before, and its own comment deferred the config knob to this task; leaving the list there and importing it from `module-vars.ts` would have forked two matchers the moment the knob became config-aware. Both consumers now read one compiled object, so `vba.errorChannel` drives the reference flag AND `errorPolicy.behavior` rather than only the former. - `vba.errorChannel` takes bare VBA identifiers, matched as whole names, and EXTENDS the built-in list — the same contract `vba.sqlWrappers` established in #244. No user-supplied regex: this runs per identifier per line, which is exactly where one is a backtracking hazard. Matching a name rather than a substring is what keeps `ErrorCount` out. - The compiled form (a `Set` plus RegExps) lives on the extractor context, not in `VbaExtractionOptions`, because the options object crosses the `structuredClone` worker boundary. - The flag is only ever `true`; its absence encodes "not the channel", so it is added to a minority of rows instead of a `false` to every one — the shape #260 chose for `inErrorHandler`. Closes #261 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019gmKKUq1ng5ESk6Qhxu77d
* feat(vba): emit label nodes and handles-error edges Task E6 of `docs/vba-error-handling-plan.md`. #259 records *whether* a procedure has an error handler and #260 marks *which* edges come from inside one, but neither gives the handler an identity you can point at, search for or traverse to. This does: one `label` node per VBA line label, and a `handles-error` edge from the procedure to the handler it routes to. The plan's §4 rejected this design for #259 and §4.3 named the condition that reopens it — a query `inErrorHandler`'s per-procedure boolean cannot serve. Addressing a handler as a thing is that query: a stable id per handler, `kind:label` search, and dangling/duplicate/control-flow-label detection as a graph query rather than a scan. This adds no parsing. Every fact published here was already computed by the error-policy classifier while the procedure body was open — the label definitions, the `On Error GoTo` targets, the handler region and the dangling-target resolution. `handlerBehavior` is #260's derived `errorPolicy.behavior`, copied verbatim. The one genuinely new signal is the plain-`GoTo` jump, which the policy classifier had no reason to look at while it emitted nothing, and which arrives as a fifth rule on the same declarative table rather than as a second scanner. Decisions taken: - `qualifiedName` is always `<ModuleOrClass>.<Procedure>.<label>`. VBA scopes labels to the procedure and this corpus writes `errores` 3,735 times; without the procedure segment every handler in a project collapses into one symbol. Same shape #257's parameters and #251's module variables chose. - `handles-error` is not deduplicated per procedure. 47 procedures issue more than one `On Error GoTo`, and each is a distinct routing decision with its own line, so each emits its own edge. - A plain `GoTo` reuses the generic `references` kind tagged `vba-goto`. A jump is not an error-handling fact and 192 sites do not justify a second kind; the synthesizer tag keeps them filterable. - A `GoTo` whose target the procedure never defines emits an `UnresolvedReference` and **no node**. A graph that invents its own targets cannot be used to find that defect, which is the only reason to look for it. - Calls inside a handler stay attributed to the enclosing procedure. The label is addressable, not a container; re-parenting would change `callers`/`callees` for the 3,774 procedures that have a handler. - Only the label whose region `errorPolicy` actually resolved carries `handlerBehavior` and the region lines. A procedure that swaps to a second handler label has a second region nobody classified, and deriving one here would be exactly the drift this split avoids. - A numeric `GoTo` target is a VBA line number, not a line label. The label detector cannot define one, so referencing it would fabricate a permanent dangling reference for legal code. - `label` stays out of `HIGH_VALUE_NODE_KINDS` and `CONTAINER_NODE_KINDS`, for the reason #257 kept `parameter` out of both: it is now the most numerous VBA symbol in the graph. Measured on the three-project Access corpus with the committed probe: 3,911 label nodes, 3,832 `handles-error` edges, 192 `vba-goto` references, zero new unresolved references, and no other node or edge kind moved. That is +15.0% nodes and +26.9% edges — the extractor matches the probe's census exactly on all three counts. `EXTRACTION_VERSION` is bumped to 26: a new node kind and a new edge kind change what a re-index would produce. Closes #263 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019gmKKUq1ng5ESk6Qhxu77d * docs(vba): correct the E6 landing-order claim in the plan The E6 section said it landed "after E1-E5 were built". E4 (#261) is still in flight and E5 (#262) has not started, so that sentence asserted an order that did not happen. Corrected to what is true: E6 landed after E1-E3, alongside E4, and before E5. The rest of the section — the three §4.3 conditions, the measured budget, and the warning not to read it as licence to add a kind elsewhere — is unchanged and accurate. Refs #263 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019gmKKUq1ng5ESk6Qhxu77d --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ed Error (#293) `scanModuleVariableReferences` walks every identifier on a line and emits a reference for any that names a module-level variable. It guarded a `.` / `!` prefix and procedure-local shadowing (#205), but not VBA keyword context — so in a module declaring `Public Error As String`, the word `Error` in `On Error GoTo errores` was read as an access to that variable. `Public Error As String` is this codebase's error-channel convention and appears in dozens of classes, so this fired constantly. On its own it is a stray edge; #261 labels channel references with `errorChannel: true`, which would have turned every one of them into a confident claim that an `On Error` statement takes part in error propagation — the failure mode `CLAUDE.md` and guardrail 1 of `docs/vba-error-handling-plan.md` both name as the worst available here. #261 is held until this lands so it is measured on clean data. The `On Error` pair is blanked out before the identifier walk, replaced with spaces of the SAME length. That is load-bearing rather than incidental: the emitted reference carries `column: m.index`, so a substitution that shifted offsets would corrupt every column on the line. A fixture pins the column of a genuine reference sharing a line with `On Error GoTo`. Scoped to the `On Error` pair only. VBA spells the `Error` statement (`Error 5`) and the `Error$()` function with the same word; both have zero occurrences in this corpus, and telling those from an identically-named variable is a parser problem rather than a masking one. They are left for a corpus that contains them. Measured on the corpus (`00_EXPEDIENTES`, `00_GESTION_RIESGOS`, `HPS_SOLICITUDES`): unresolved references fall 26,755 -> 25,211. The entire delta is `property-get`, 5,636 -> 4,092 — 1,544 false reads removed, and no other reference kind, node kind or edge kind moves. Nodes stay at 30,000 and edges at 37,456. The issue estimated ~909; the measured figure is 1,544. The estimate counted handler bodies, while the sweep de-duplicates per (procedure, variable, direction) — so every procedure whose ONLY apparent read of `Error` came from its own `On Error` line contributed one, including procedures the estimate did not look at. Closes #292 Claude-Session: https://claude.ai/code/session_019gmKKUq1ng5ESk6Qhxu77d Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Error propagation in an Access codebase of this shape does not use VBA's error mechanism. 16 handlers out of 3,774 re-raise; `Err.Raise 1000` unwinds exactly one frame and the house guard `If Err.Number <> 1000` means "an inner procedure already wrote a human-readable message". The message itself travels through a field the failing procedure writes and the caller reads. That is module-variable data flow, which #251 already models as `property-set` / `property-get` references onto a `variable` node. This change only labels it: a read or write of a channel variable now carries `metadata.errorChannel: true`, on the reference and on the resolved edge. No new node kind, no new edge kind, and no new row — the corpus indexes to byte-identical `nodesByKind` / `edgesByKind` / `unresolvedByKind` totals (26,089 / 29,521 / 26,755, unchanged against origin/main). Decisions taken: - The channel names and the write matcher move into a new leaf module, `src/extraction/vba/error-channel.ts`. `errors.ts` owned both before, and its own comment deferred the config knob to this task; leaving the list there and importing it from `module-vars.ts` would have forked two matchers the moment the knob became config-aware. Both consumers now read one compiled object, so `vba.errorChannel` drives the reference flag AND `errorPolicy.behavior` rather than only the former. - `vba.errorChannel` takes bare VBA identifiers, matched as whole names, and EXTENDS the built-in list — the same contract `vba.sqlWrappers` established in #244. No user-supplied regex: this runs per identifier per line, which is exactly where one is a backtracking hazard. Matching a name rather than a substring is what keeps `ErrorCount` out. - The compiled form (a `Set` plus RegExps) lives on the extractor context, not in `VbaExtractionOptions`, because the options object crosses the `structuredClone` worker boundary. - The flag is only ever `true`; its absence encodes "not the channel", so it is added to a minority of rows instead of a `false` to every one — the shape #260 chose for `inErrorHandler`. Closes #261
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task E4 of
docs/vba-error-handling-plan.md. Closes #261.What this does
A read or write of an error-channel variable now carries
metadata.errorChannel: true— on theUnresolvedReferenceand on the resolvedreferencesedge — alongside theproperty-get/property-setkind #251 already emitted. New config knobvba.errorChannel(bare identifiers, whole-name match, extends the four defaultsm_Error/p_Error/g_Error/Error, no user regex), threaded end-to-end throughVbaExtractionOptions.No new node kind, no new edge kind, no new row.
src/types.tsis untouched.One design decision worth flagging: the channel names and the channel-write matcher moved out of
errors.tsinto a new leafsrc/extraction/vba/error-channel.ts.errors.tsowned both, and its own comment deferred the knob to this task. Leaving the list there would have produced two matchers the moment the knob became config-aware, so both consumers — E3'serrorPolicy.behaviorclassifier and this task's reference flag — now read one compiled object. Consequence:vba.errorChannelalso drivesbehavior, so a project that configureslastFailuregets its handlers classifiedchannelrather thanunknown.Corpus measurements
Probe run over all three roots (
00_EXPEDIENTES/src,00_GESTION_RIESGOS/src,HPS_SOLICITUDES/src), branch vs a cleanorigin/mainexport of the same tree:nodesByKind,edgesByKind,unresolvedByKind,edgesBySynthesizer,filesand the wholeerrorHandlingblock are byte-identical between the two runs. Nothing was added, dropped or reclassified.References that gained the flag, across the three roots: 5,386 of 9,378 module-variable references (3,373 reads / 2,013 writes), in 232 files. By name:
m_Error3,842,Error1,544. Indexed, that is 2,619 resolved edges in00_GESTION_RIESGOSand 1,653 in00_EXPEDIENTES.The end-to-end chain
Verified by indexing a read-only copy of
00_GESTION_RIESGOS/srcand querying the graph. The chain connects, within one module. Real file, real procedures, real variable —src/forms/Form_FormGestionRiesgos.cls:Source:
EstablecerDatosline 324 ism_Error = "Es un usuario no autorizado";Form_Loadline 276 isEstablecerDatos m_Error, 277 isIf m_Error <> "" Then, 293 ispregunta = MsgBox(m_Error, vbCritical, "Error"). Both hops target the samevariablenode, so the join is a real traversal, and thecallsedge fromForm_LoadtoEstablecerDatosgives the propagation its direction. The suite's integration test reproduces exactly this shape end-to-end throughCodeGraph.indexAll().The display hop is
errorPolicy.behavioronForm_Load, not an edge:MsgBoxis a VBA built-in, so it is not a node and there are zeroMsgBoxcall edges in the index. E3'sbehaviorfield is the graph's display marker, and it reportsmixed(channel + display) for this procedure.What does NOT connect — read this before merging
Four honest findings from the corpus. None of them is caused by this change; all of them bound what it delivers.
1. The class-to-form hop does not exist, and cannot with this model. A form reads a class's message with
p_Error = m_ObjRiesgoActivo.Error. That is a dotted access to another object's member, which #251's sweep skips by design (a dotted name is a member of something else). Confirmed in the index: zero edges from any form procedure toRiesgo.cls'sErrorvariable. So the traversable chain isinner writes -> outer reads -> outer displayswithin one module, not across the class/form boundary. Bridging that needs member resolution against a declared type, which is a different task.2. The corpus has a second channel this change cannot reach: the ByRef out-parameter.
Optional ByRef p_Error As Stringis used heavily — 174 occurrences inExpedienteOperaciones.clsalone. It is a parameter, so #205 scoping keeps it out of the module-variable sweep entirely and there is no reference to flag. Consequence for the acceptance criterion, measured per class:behaviorchannel/mixedRiesgo.clsEdicion.clsExpedienteOperaciones.clsExpedienteOperaciones.clsdeclares nom_Errorand no publicError; it propagates purely throughp_Errorparameters. So the acceptance criterion "every channel-writing handler produces a flagged reference" is met for the module-variable half of the corpus and not for the ByRef half. The uncovered handlers inRiesgo.cls/Edicion.clsare the same shape (getHTML*helpers that write onlyp_Error). I did not stretch the flag to cover parameters: it needs a different data-flow model, and inventing one here would have been the half-bridged flowCLAUDE.mdforbids.3. A pre-existing #251 false positive that this flag now amplifies. These projects declare
Public Error As Stringas the object's public message field. The identifierErroralso appears in everyOn Error GoTo erroresstatement, and the module-variable sweep reads that as a bare-name access. Result: 909 references across the corpus to a module variable literally namedError, all reads, one per handler procedure — and they are now labellederrorChannel: true. The reference itself is already wrong onmain; this change makes it more visible. Fixing it means suppressing identifiers inside anOn Error …statement, which would change edge counts and is therefore out of scope for a task whose invariant is "unchanged totals". Worth its own issue.4.
inErrorHandlerrarely co-occurs with a channel write on real code. #251 de-dupes references per(procedure, variable, direction)and keeps the FIRST occurrence. The corpus's dominant handler shape initialisesm_Error = ""at the top of the body and writes the message again in the handler, so the surviving write reference is the initialisation one, outside the handler region. Corpus-wide: 5,386 flagged references, of which only 618 carryinErrorHandler. The unit test pins the flag on a fixture where the handler write is the only write. Changing which occurrence survives would alter existing rows'linevalues, so I left #251's de-dup alone.Tests
New
__tests__/extraction-vba-error-channel.test.ts, 20 tests, all passing. Covers every acceptance checkbox that is a unit test: the in-handler write (property-set+errorChannel+inErrorHandler), the caller'sIf m_Error <> "" Thenread, theErrorCountnegative (plus four neighbouring names), case-insensitivity,vba.errorChannel: ["lastFailure"]matching that name and keeping the defaults, config validation dropping dotted/regex-ish entries, the knob drivingbehaviorfrom the same list, the no-new-rows invariant, the two boundaries above (Me.Error =and a ByRefp_Error), the traversal integration test, and reading the knob out of a realcodegraph.json.Verification run:
npx tsc --noEmitclean; VBA suites 377 + 716 passed, resolution/config suites 227 passed, non-VBA suites 1,166 + 537 + 828 passed. The 21 failures observed are the pre-existing environmental ones on this machine (worktree-detectionx15,multi-repo-workspacex2,extractionx2,npm-sdkx2 — allafterEachtemp-dir removal EPERM/EBUSY). Zero new failures. A full singlenpx vitest runOOMs on this machine, so suites were run in batches.Not verified
HPS_SOLICITUDESwas measured by the probe (counts) but not indexed for a graph traversal.g_Errordefault matched nothing in this corpus — it is carried over from the probe's list, not corroborated here.🤖 Generated with Claude Code
https://claude.ai/code/session_019gmKKUq1ng5ESk6Qhxu77d