Skip to content

Decision: settle the AnalyzerContext member list (files/policy) before #28 is implemented #90

Description

@serge-ivo

Decision required before any #28 code is written. This issue asks for one thing: the final member list of AnalyzerContext, in particular whether files: FileInventory and policy: EffectiveScanPolicy are members. Everything below exists to make that decision answerable in one sitting.

Why this blocks #28

docs/internal-analyzer-contract.md is the implementation spec #28 names. Its AnalyzerContext sketch predates two things that have since shipped in 0.55.0, and it has not been amended for either:

The sketch as it stands today, docs/internal-analyzer-contract.md:82-94:

export interface AnalyzerContext {
  cwd: string;
  workspace: WorkspaceInfo;
  projects?: ProjectContext[];
  project?: ProjectContext;
  stack: StackInfo;
  config: VcqaConfig;
  settings: Record<string, unknown>;
  effectiveSettings: Record<string, unknown>;
  skipTests: boolean;
  srcRoots?: string[];
  ignoreNames: string[];
}

The words FileInventory, EffectiveScanPolicy and inventory do not appear anywhere in the document — verified by grep across all 338 lines. The only ignore-related member is ignoreNames: string[] (line 93), which is the pre-#71 representation: a flat list of names, with no reason codes, no precedence, and no notion of the security-sensitive override that #71 added. In main that list is now a derived view, not a source of truth — src/core.ts:125:

setGlobalIgnoreNames(policyIgnoreNames(scanPolicy));

The same sketch is duplicated in the body of #28 itself, in an even older form (no projects / project members at all). Both copies need the same amendment.

app/CLAUDE.md:100 makes this a precondition rather than a follow-up:

If these docs disagree, fix the docs before implementing the feature.

The doc and main disagree today. So the ordering is: settle this issue, amend docs/internal-analyzer-contract.md and the sketch in #28's body, then write #28 code.

The gap is already recorded in the code's own history

This is not a hypothetical. The closing comment on #70 says so explicitly, under "Scoped out, deliberately":

I implemented the equivalent as module-level queries — inventorySourceFiles, inventoryTestFiles, inventoryAllFiles, inventoryFiles({ kind, ext, includeGenerated, includeIgnored }) — rather than a ctx.files object, because there is no AnalyzerContext object to hang it off yet.

And #71's closing comment describes the policy surface as landing where the context would have:

FileInventory now carries the EffectiveScanPolicy it was built from, plus inventoryIsIgnored / inventoryClassify / inventoryExplain / inventoryHas — the ctx.isIgnored(path) / ctx.classify(path) surface from #70's sketch.

That carrying is real, src/file-inventory.ts:54-58:

export interface FileInventory {
	root: string;
	/** The one EffectiveScanPolicy this inventory was built from. Carried here so
	 *  every runner that receives the inventory receives the same policy, and can
	 *  classify or explain a path the walk never produced (#71). */
	policy: EffectiveScanPolicy;

Meanwhile the de-facto context in main is a widening positional argument list — src/core.ts:136-156 passes (resolvedCwd, stack, workspace, fileInventory), (resolvedCwd, workspace, fileInventory), and for testing a six-argument call (resolvedCwd, stack, skipTests, srcRoots, workspace, fileInventory). Every runner takes the inventory as an optional trailing parameter (src/runners/secrets.ts:255, src/runners/flutter.ts:36, src/runners/structure.ts:34). Replacing that with a typed context is exactly what #28 is for, which is why the member list has to be right before it is frozen into 30+ call sites.

The decision

D1 — Does AnalyzerContext carry the file universe? (primary)

Recommendation: yes — add files: FileInventory and delete ignoreNames.

Option What it means Cost
A. files: FileInventory (recommended) The context carries the built inventory object. Runners keep calling the existing module-level queries, passing ctx.files. One member. Zero change to file-inventory.ts. Matches what core.ts already passes positionally.
B. Accessor functions on the context ctx.sourceFiles(), ctx.testFiles(), ctx.allFiles(opts) — closures bound to the inventory, inventory itself not exposed. Nicer call sites, but it is a second API over the one shipped in #70, and it hides ctx.files.policy, which #71 deliberately put on the inventory. Also cannot express includeIgnored cases like flutter's generated-Dart ratio without re-adding an escape hatch.
C. Leave it out; runners keep taking the inventory positionally No decision needed now. Defeats #28's purpose — core.ts keeps a bespoke argument list per runner, which is the thing the issue says it is removing.

Rejecting B specifically: it is not wrong, but it duplicates a surface that shipped four days ago and has tests (src/file-inventory.test.ts). If accessors are wanted later they can be added as sugar over ctx.files without another migration. Rejecting C: it makes #28 a rename rather than a contract.

ignoreNames: string[] should be deleted, not kept alongside. It is a lossy projection of ctx.files.policy and, per #71's commit message, keeping two representations of one contract is what caused the fs-utils drift that #71 existed to remove. If a caller genuinely wants the flat list, policyIgnoreNames(ctx.files.policy) produces it.

D2 — Is policy a separate member, or reached via ctx.files.policy?

Recommendation: ctx.files.policy only. Do not add a top-level policy member.

The inventory already owns exactly one policy and documents why (src/file-inventory.ts:56-58, quoted above). A second reference creates a shape where ctx.policy !== ctx.files.policy is constructible, which is precisely the two-copies-of-one-contract failure mode.

Counter-argument worth recording: out-of-process analyzers (#32) have to serialise the context over JSON/stdio, and EffectiveScanPolicy is a small flat record (src/scan-policy.ts:43-54, nine fields) while FileInventory carries every walked path plus ignoredFiles. A subprocess protocol will likely send the policy and a filtered file list rather than the whole inventory. That is an argument about the wire shape, not the in-process shape, and #32 is free to define its own projection. It is not a reason to split the member here — but it should be written into the doc so #32's author does not read files: FileInventory as a serialisation requirement.

D3 — Do settings / effectiveSettings stay as sketched? (unblocks #30)

The sketch has both (docs/internal-analyzer-contract.md:89-90). Recommendation: keep both, and specify that settings is the raw user block and effectiveSettings is defaults-merged-and-validated, since the names do not say so.

There is prior thinking on this that is not in the repo — see "Uncommitted work" below. That WIP resolves settings once per scan in core.ts and delivers them via details.effectiveSettings / details.settingsWarnings per check plus meta.analyzerSettings on the report, with a new EffectiveAnalyzerSettings type. That approach needs no AnalyzerContext at all.

So D3 has a sequencing sub-question the maintainer should answer: does #30 land on today's positional runners (as the WIP does), or does it wait for #28's context? Landing first is cheaper and unblocks the app surface sooner; waiting avoids writing a settings path that #28 immediately reworks. I lean toward landing #30 first provided the report-level meta.analyzerSettings shape is treated as the stable contract and the plumbing is understood to be temporary — but this is genuinely the maintainer's call, not mine.

D4 — Does the context carry scope? (unblocks #58)

#58's first task is "Add analyzer scope metadata: repo, project, file-graph, derived". The doc already half-specifies this in prose — line 109, "Repo-level analyzers receive the whole projects list. Project-scoped analyzers receive one project at a time" — and lifecycle step 2, line 140, "Select repo or project scope from the analyzer manifest". But no scope field exists on AnalyzerManifest (docs/internal-analyzer-contract.md:66-74). The lifecycle instructs the engine to read a field the type does not have.

Recommendation: add scope: "repo" | "project" | "file-graph" | "derived" to AnalyzerManifest, and keep both projects? and project? on the context as the sketch already has them. ProjectContext is already a shipped type (src/types.ts:50-63), so this costs nothing new. Rejected alternative: inferring scope from whether the analyzer reads ctx.project — unknowable statically, and the engine has to decide iteration before calling the analyzer.

D5 — Does the context carry analyzer identity, for tool-run provenance? (relates to app#52)

app#52 wants ToolRun to record where a run came from, and proposes { analyzerId, source: "builtin" | "plugin", pluginId?, version? }. Lifecycle step 7 (line 145) already says "Attach package-tagged tool provenance from exec.ts", but nothing in the context tells exec.ts which analyzer is running.

Recommendation: add analyzerId: string to AnalyzerContext so the engine can stamp origin at the recording boundary rather than asking every analyzer to self-report. This is a one-word change here that makes app#52 implementable without touching each runner. Note app#52 lives in vibecodeqa/app but the field it wants is in @vibecodeqa/schema's ToolRun — that part is not this issue's to decide, and this issue only commits to making the CLI side supplyable.

D6 — #58 vs #27: merge or parent? (sub-decision, as raised by the audit)

Verified: both are open and both are labelled analyzer-platform, and #58 already lists #27 under its Related section, so the overlap is acknowledged but not resolved.

Recommendation: explicit parenting, not merging. Keep #58 as the implementation issue and rewrite #27's body to be the acceptance checkpoint for it — "verified by" rather than "also does". Reason for not merging: #27 carries the observable symptom a user reported, and folding it into #58 loses a separately closable proof that the symptom is gone. Reason for not leaving them as-is: two open issues describing one body of work will get picked up twice.

If the maintainer prefers merging, close #27 as duplicate of #58 after copying its symptom evidence into #58's acceptance criteria — not before.

What each choice unblocks

Decision Unblocks
D1 files: FileInventory #28 (a context worth migrating to); the ctx.files sketch #70 deferred; retires the residue of #70
D2 policy via ctx.files.policy the residue of #71 ("every runner receives the same policy through analyzer context" — currently satisfied only by the inventory hop); informs #32
D3 settings / effectiveSettings #30
D4 manifest.scope + project / projects #58, and through it #27
D5 analyzerId the CLI half of app#52

Known residues that are not resolved by this decision

Both are named in #70's closing comment and neither has an issue, so they should not be assumed in scope:

  • files.html({ deliverable: true }) and files.config({ kind: "github-actions" }) were sketched in Introduce FileInventory: classify scan files once and require runners to consume it #70 and have no implementation. The missing config query is why lint and best-practices still read .github/workflows/ directly. If the context is to expose them, that needs its own issue.
  • External tool adapters (lint, types) let the tool discover files and filter output afterwards through isIgnoredPath(). That is deliberate and documented in docs/exclusion-policy.md under "Walks that remain, and why". The context does not change it.

Uncommitted work that should be read before deciding, and rescued

There is a stash in the local cli checkout:

stash@{0}: On main: WIP cli#30 analyzer-settings schema (backed up /tmp/vcqa-wip-backup/) [stashed by dev-agent 2026-08-08]

It touches docs/internal-analyzer-contract.md (+11 lines, a "Current implementation" block under Settings), src/types.ts (adds EffectiveAnalyzerSettings and meta.analyzerSettings), src/core.ts (+64/-8), src/config.ts, and tests. It is the prior thinking behind D3 and contains a concrete answer to "where do effective settings surface".

It is backed up only to /tmp/vcqa-wip-backup/ (stash-0.patch, plus cli-uncommitted.patch, schema-uncommitted.patch, app-uncommitted-*.patch, cli-untracked.tgz). macOS prunes /tmp and the stash lives in one working copy on one machine; neither survives indefinitely. Whoever picks this up should move that backup somewhere durable, or land/drop the stash, before it evaporates.

Acceptance criteria

Constraints

  • app/CLAUDE.md:100 — "If these docs disagree, fix the docs before implementing the feature." The doc amendment is a precondition, not cleanup.
  • cli/CLAUDE.md — trunk-based, commit straight to main, no branches and no PRs. There is no PR to hold this decision in; the decision has to live in this issue.
  • cli/CLAUDE.md stack-gating rule still applies to whatever the context exposes: a stack.framework === ... branch inside a generic analyzer is a rejected diff regardless of how the context is shaped.
  • Doc-only pushes trigger publish.yml, which then skips publishing unless package.json version changed. Amending the doc is safe; it just starts a job that no-ops.

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

analyzer-platformAnalyzer engine, registry, contracts, and normalized resultsquestionFurther information is requested

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions