Fix C# metadata-target ambiguity (signature-LIKE false edges) (#435) - #886
Merged
Conversation
The deps/impact metadata-attribute edge relied on `signature LIKE '%: %'` to detect attribute-derived classes. That heuristic could not tell a real `class FooAttribute : Attribute` apart from an impostor `class FooAttribute : BaseService` that shares the name, causing the corresponding metadata edges to be dropped silently. This introduces an authoritative resolver that walks C# class base lists at index time with fixed-point transitive resolution against same-DB class rows, falling back to the BCL `Attribute` suffix convention only for unresolved external bases. Results are persisted to a new `symbols.is_metadata_target` column and gated on a per-language readiness stamp (`metadata_target_version_csharp`) in `codeindex_meta`. The reader now uses a three-way branch: (1) readiness-stamped DB uses the persisted column; (2) column present but row not stamped (legacy) falls back to the old signature heuristic; (3) column missing on a read-only legacy DB uses a name-suffix fallback. `status --json` exposes the new trust flag as `csharp_metadata_target_ready`. Fixes #435 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Three fixes from the #435 codex iteration 1 adversarial review: 1. Qualified vs unqualified base resolution. `IsMetadataTargetByBases` previously collapsed every base type to its simple head, so `class FooAttribute : B.BaseAttr` saw `A.BaseAttr` (a real target in a different namespace) and was wrongly promoted, while `class MyValidatorAttribute : ThirdParty.ValidationAttribute` was wrongly demoted when a same-simple-name `InRepo.ValidationAttribute` (non-target) suppressed the BCL suffix fallback. The resolver now builds a separate fully-qualified index keyed off `container_qualified_name` and routes qualified bases (containing `.` or `::`) through it exclusively; unresolved qualified bases are treated as external and fall through to the suffix heuristic. 2. Generic constraint misread as base list. `FindBaseListColon` used to return the first top-level `:` even when it was the `where T :` of a constraint clause on a class without a base list, causing `class NotAnAttributeClass<T> where T : Attribute {}` to be marked `is_metadata_target = 1`. The scanner now returns -1 on the first top-level `where` keyword reached before any base-list colon. 3. Reader three-way branch now keys off `is_metadata_target` column presence, not `signature`, matching the documented contract. A legacy DB missing the `is_metadata_target` column correctly degrades to the name-suffix-only fallback (branch 3) instead of silently continuing to use the signature heuristic (branch 2). Adds four regression tests covering qualified-base disambiguation, external-qualified-base suffix fallback, the `where`-clause miscount, and the column-missing legacy branch. Refs #435 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codex iteration 2 review found a single actionable follow-up to the
iter 1 qualified-base fix: the qualified-FQN index in
`ResolveCSharpMetadataTargets` used `Dictionary<string, long>` with
`TryAdd`, so whichever partial-class declaration row was inserted first
silently won. Legal C# `partial class` can split a single logical type
across multiple symbol rows (one per declaration site), and only one of
them may carry the real `: Attribute` base list. With a single-id map,
the base-less partial (if inserted first) would suppress resolution of
a downstream qualified reference like `class FooAttribute : B.BaseAttr`,
dropping the metadata edge in a file-order dependent way.
Changes:
1. `qualifiedToId` becomes `qualifiedToIds: Dictionary<string, List<long>>`.
The build loop now accumulates ALL row ids per FQN key produced by
`EnumerateQualifiedKeys`, instead of keeping only the first.
2. `IsMetadataTargetByBases` iterates the candidate id list for a
qualified base. If any candidate is already in `resolvedTargets`,
the derived class is promoted; if none are resolved yet, we still
`continue` (wait for the next fixed-point iteration) rather than
falling through to the BCL suffix heuristic, preserving the rule
that an in-repo qualified reference must resolve specifically.
Adds `GetFileDependencies_CSharpMetadataTargetResolverHandlesPartialClassBase`
which pins the scenario: `partial class B.BaseAttr {}` in file 1 (no base,
inserted first) + `partial class B.BaseAttr : Attribute {}` in file 2 +
`class FooAttribute : B.BaseAttr {}` + `[Foo] class Svc {}`. The test
asserts the `Svc → FooAttribute` metadata edge is preserved.
Refs #435
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codex review iteration 3 found that the #435 contract only surfaced through index --json. `status --json` / MCP status returned null for the new field, and the human `cdidx index` / `cdidx status` outputs said "all ready" even when the resolver had not stamped metadata_target_version_csharp. AI flows and humans therefore could not see when deps/impact metadata-attribute edges had degraded to the signature / name-suffix heuristic. - Add `CSharpMetadataTargetReady` to `StatusResult` (serialized as `csharp_metadata_target_ready`) and populate it in `DbReader.GetStatus` using `!hasCSharpFiles || _csharpMetadataTargetReady`, mirroring the existing `csharp_symbol_name_ready` semantics. - Extend `cdidx status` human output: include the flag in the DEGRADED summary and add an explicit WARN line pointing at `cdidx index .` when the contract is not ready. - Extend `cdidx index` human output: both update and full-scan display blocks now print `C# meta : ready/degraded`, and `GetIndexReadinessWarning` folds the flag into the degraded-parts list. - Regression tests: `GetStatus_ExposesCSharpMetadataTargetReady*` (no C# files, stamp missing, stamp current) plus a shared `ClearMetaStamp` helper for stamp-deletion scenarios. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…iter 4 Codex iteration 4 review found two remaining bugs that still leaked a false metadata-attribute edge for the #435 feature: 1. DbWriter.ResolveCSharpMetadataTargets resolved unqualified C# base names through a repo-global simple-name bucket, so a real `A.BaseAttr : Attribute` in namespace A would falsely promote `B.FooAttribute : BaseAttr` in namespace B even though B's own `B.BaseAttr : BaseService` was the actual base. The resolver now builds a `(enclosing_scope, simple_name) -> ids` map alongside the qualified-name map and walks the deriving row's scope chain inside -> outside; the BCL Attribute-suffix fallback only fires when no scope level matches. A new GetEnclosingScope helper derives the enclosing scope from container_qualified_name so partial families share a scope. 2. DbReader.IsMetadataTargetUnambiguous used `count <= 1`, which let the metadata bypass fire with zero candidate definitions. `[Foo]` cannot resolve to a non-attribute definition, so `count == 0` is not a safe bypass either and impact's file-dep hint still produced the false edge even after the writer stopped setting the column. The predicate is now `count == 1`. Added GetFileDependencies_CSharpMetadataTargetResolverPrefersSameNamespaceBaseOverGlobalImpostor to pin the scope-aware resolution and the tighter unambiguous predicate using the iter 4 codex repro fixture. Verified end-to-end with the locally built binary on both negative (cross-namespace impostor) and positive (same-namespace transitive) fixtures. CHANGELOG updated in both English and Japanese sections. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Codex iteration 5 review found that the iter-4 scope-aware unqualified-base
resolver regressed the common C# pattern `using A; namespace B { class
FooAttribute : BaseAttr {} }` where `A.BaseAttr : Attribute` is indexed in a
sibling file: with same-scope-only lookup, `BaseAttr` never resolved from B's
scope chain, so `FooAttribute` stayed `is_metadata_target=0` and `deps` /
`impact` dropped the metadata edge from `[Foo]` consumers. The alias form
`using AliasAttr = A.BaseAttr;` with `class FooAttribute : AliasAttr` had the
same false-negative.
The resolver is now import-aware:
1. `LoadCSharpClassRows()` also returns `file_id` so the fixed-point loop
can associate each deriving row with its declaring file.
2. `LoadCSharpImportsByFile()` reads `symbols.kind='import'` rows for every
C# file and partitions each into a per-file `FileImportSet`:
* Namespace imports (`using Foo.Bar;`) → `Namespaces` list.
* Alias imports (`using Alias = Foo.Bar.Type;`) → `Aliases` map,
stripping `global::` prefixes so downstream `qualifiedToIds` lookup
sees a single key shape.
`extern alias` and `using static` rows are skipped — they do not open a
base-clause resolution path. `global using` directives are aggregated
across the repo into a second `FileImportSet` so C# 10+ directives widen
every file's lookup even when they are declared elsewhere.
3. `IsMetadataTargetByBases` accepts the deriving file's imports plus the
repo-wide global-imports set, and after a same-scope miss it probes
alias-then-namespace imports through `qualifiedToIds` before falling
through to the BCL `Attribute`-suffix convention. It returns `true` on
a resolved in-repo target, defers via `continue` when it finds an in-repo
match that is not yet a target (preserving fixed-point semantics and
preventing the BCL heuristic from false-promoting a non-attribute
imported class), and only falls through when nothing in scope or imports
matches.
4. `DbContext.MetadataTargetVersion` bumped `1` → `2`. Iter-4 DBs that only
walked the scope chain will degrade to the legacy reader heuristic until
a reindex republishes `is_metadata_target` through the import-aware
resolver, so they do not keep asserting incorrect metadata edges.
Added `DbReaderTests.GetFileDependencies_CSharpMetadataTargetResolverHandlesImportedNamespaceBase`
and `GetFileDependencies_CSharpMetadataTargetResolverHandlesUsingAliasBase`
pinning both forms. Verified end-to-end with the locally built binary on
namespace-import and alias fixtures (FooAttribute → `is_metadata_target=1`,
`file_impacts` contains the `Svc.cs → FooAttribute.cs` edge) and on the
iter-4 negative fixture (no regression — `B.FooAttribute` still resolves to
the in-scope `B.BaseAttr : BaseService` and stays non-target). CHANGELOG
updated in both English and Japanese sections.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…435 iter 6 Codex iteration 6 review (NO_BLOCK verdict, one Low finding) flagged that the iter-5 import-aware resolver stored the raw verbatim-identifier `@` prefix in the per-file import map, so valid C# code like `using @foo.@bar;` or `using @AliasAttr = @foo.@Bar.BaseAttr;` never matched the qualified index and `class VerbatimImportAttribute : BaseAttr` stayed `is_metadata_target=0`. The `[VerbatimImport]` attribute-consumer edge was silently dropped from `deps` / `impact`. A second, lower-severity gap: `SymbolExtractor`'s C# alias regex rejected verbatim alias names entirely, so `using @AliasAttr = A.BaseAttr;` never produced an `import` row in the first place. The fix is writer-side canonicalization: 1. `DbWriter.StripCSharpVerbatimPrefixes(qualified)` strips the leading `@` from each dotted segment of a qualified name. `@Foo.@Bar.BaseAttr` → `Foo.Bar.BaseAttr`; unchanged otherwise. 2. `RegisterCSharpImport` now applies `StripCSharpVerbatimPrefixes` to the captured alias name, alias target, and plain namespace-import path after stripping the `global::` prefix, so every entry in the per-file / repo-wide `FileImportSet` is in canonical form. 3. The alias-regex inside `RegisterCSharpImport` is widened from `\w+` to `@?\w+` so `using @AliasAttr = Target;` is captured even when the raw import row was stored with the `@` prefix intact. 4. `IsMetadataTargetByBases` normalizes `baseName` through `StripCSharpVerbatimPrefixes` before the qualified / scope / import lookups, matching the import side's canonical form. 5. `SymbolExtractor` C# alias regex widened from `\w+` to `@?\w+` so the extractor surfaces the row at all. 6. `DbContext.MetadataTargetVersion` bumped `2` → `3`. Iter-5 DBs that stored raw `@`-prefixed tokens degrade to the legacy reader heuristic until a reindex republishes `is_metadata_target` through the normalized resolver. Added `DbReaderTests.GetFileDependencies_CSharpMetadataTargetResolverHandlesVerbatimNamespaceImport` and `...HandlesVerbatimAliasNameAndTarget` pinning both forms. Verified end-to-end with the locally built binary: the verbatim namespace-import fixture promotes `VerbatimImportAttribute` to `is_metadata_target=1` and surfaces the `Consumer.cs → VerbatimAttr.cs` edge in `deps --json`; the verbatim alias fixture does the same for `VerbatimAliasAttribute`. Iter-5 non-verbatim fixtures and iter-4 scope-vs-import negatives still behave as expected. The version-stamp degradation test confirms iter-5 DBs now report `csharp_metadata_target_ready=false` until reindexed. Full `dotnet test` passes (2154/2156, 2 performance skips). CHANGELOG updated in both English and Japanese sections. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Iter 7 follow-up for #435. Codex review iter 7 flagged three gaps: HIGH: SymbolExtractor C# decl regexes rejected @-prefixed identifiers at the declaration site. `public class @BaseAttr : Attribute` was silently dropped at extraction time, so the metadata-target resolver could not see `@BaseAttr` in the class graph and transitive `class FooAttribute : @BaseAttr` resolution was broken even though iter 6 made the import / alias / base-identifier sides verbatim-aware. Widened the namespace / class / struct / interface / enum name capture groups to `@?\w+` and added `NormalizeCSharpSymbolName` so persisted names are stored in canonical form (no `@` prefix). MEDIUM: `DbWriter.StripCSharpVerbatimPrefixes` used a dot-only Split/Join and left the first segment intact when the qualified name started with `global::@Foo` or `Alias::@Foo`. Rewrote it as a character walk with an `atBoundary` flag that also treats `::` as a segment boundary, so `global::@foo.@Bar.BaseAttr` collapses to `global::Foo.Bar.BaseAttr`. LOW: Extended CLAUDE.md's "Authoritative C# metadata-target trust" bullet (English and Japanese) to describe the verbatim-declaration gate, the `::` boundary, and the 1→2→3→4 version history. Bumped `DbContext.MetadataTargetVersion` from 3 to 4 so iter-6 DBs that indexed without the verbatim-declaration gate or the `::`-aware normalization degrade to the legacy reader path until reindexed. Added regression tests: - `GetFileDependencies_CSharpMetadataTargetResolverHandlesVerbatimBaseClassDeclaration` - `GetFileDependencies_CSharpMetadataTargetResolverHandlesGlobalQualifiedVerbatimBase` Full test suite: 2156 passed, 2 perf-skipped, 0 failed.
Iter 8 follow-up for #435. Codex review iter 8 reproduced a real correctness miss with the locally built binary: a deriving class written as `class FooAttribute : Alias.MetaBase` (where `using Alias = A;` and `A.MetaBase : Attribute` lives in-repo) was persisted with `is_metadata_target = 0`, and the `[Foo] -> FooAttribute` metadata edge was silently dropped from `deps` / `impact`. Root cause: `IsMetadataTargetByBases` routed every base containing a `.` into the qualified branch, which looked the name up verbatim in the fully-qualified index. `Alias.MetaBase` is not stored there (the real FQN is `A.MetaBase`), so the lookup missed and the branch fell through to the BCL `Attribute`-suffix heuristic. `head = "MetaBase"` does not end with `Attribute`, so the resolver returned false. iter 5's alias-import path only handled fully-unqualified bases (`class Foo : Alias`). Fix: the qualified branch now expands the first dotted segment of the base against the deriving file's `using Alias = Target;` entries (and the repo-aggregated `global using` aliases) before the qualified-index lookup. If alias expansion resolves to `System.Attribute` the direct-attribute rule re-fires immediately. The new `ExpandCSharpAliasQualifiedBase` helper preserves the remaining dotted suffix verbatim and defensively strips any stored `global::` prefix from the alias target. Bumped `DbContext.MetadataTargetVersion` from 4 to 5 so iter-7 DBs that indexed without alias-qualified expansion degrade to the legacy reader path until reindexed. Added regression tests: - `GetFileDependencies_CSharpMetadataTargetResolverHandlesAliasQualifiedBase` - `GetFileDependencies_CSharpMetadataTargetResolverHandlesAliasNamespacePointingAtSystem` Full test suite: 2158 passed, 2 perf-skipped, 0 failed.
…esolver Iter 9 follow-up for #435. Codex review iter 9 flagged that C# accepts both `Alias.X` (member access) and `Alias::X` (qualified-alias-member, §7.8) for a using alias that names a namespace. Iter 8's `ExpandCSharpAliasQualifiedBase` only split on `.`, so a deriving class written as `class FooAttribute : Alias::MetaBase` (where `using Alias = A;` and `A.MetaBase : Attribute` lives in-repo) had `IndexOf('.') == -1` in the helper, skipped alias expansion entirely, and fell through to the BCL `Attribute`-suffix heuristic — which misses because `head = "MetaBase"` does not end with `Attribute`. I reproduced the miss end-to-end with the locally built binary: the `::` form produced `count:0,edges:[]` from `deps --json` even though the `.` form had already been fixed in iter 8. Fix: the helper now picks the earliest of `.` or `::` as the alias separator and splits on whichever comes first. The rest of the qualified name after the alias separator is always spliced with `.` so `Alias::Outer.Inner` collapses to `Target.Outer.Inner`, matching how `qualifiedToIds` keys are stored. The qualified-branch entry check in `IsMetadataTargetByBases` already recognized both `.` and `::` as qualifier markers (iter 7 widened it), so no change is needed on the caller side. Bumped `DbContext.MetadataTargetVersion` from 5 to 6 so iter-8 DBs that indexed without `::`-aware alias expansion degrade to the legacy reader path until reindexed. Added regression test: - `GetFileDependencies_CSharpMetadataTargetResolverHandlesAliasColonColonQualifiedBase` Verified against the locally built binary on a minimal temp repo (`Alias::MetaBase` → `[Foo] -> FooAttribute` edge now present) and full test suite: 2159 passed, 2 perf-skipped, 0 failed.
…metadata-target-column Bring PR #886 (C# metadata-target authoritative `is_metadata_target` column, #435) up to date with main. Conflicts resolved: - CHANGELOG.md / README.md: concatenated both branches' additive entries under Unreleased; unified `status --json` readiness flag list to expose `csharp_metadata_target_ready` alongside `sql_graph_contract_ready`, `hotspot_family_ready`, `csharp_symbol_name_ready`, etc. - Cli/IndexCommandRunner.cs: combined readiness parameter lists and warning trigger so full-scan + update-mode propagate both `priorMetadataTargetCsharp` (HEAD) and `priorSqlGraphContractVersion` (main). - Cli/QueryCommandRunner.cs: combined degraded-flag check to include CSharpMetadataTargetReady and SqlGraphContractReady. - Database/DbReader.cs: kept both `_csharpMetadataTargetReady` and `_sqlGraphContractCurrent` readiness fields and their StatusResult properties. - Indexer/SymbolExtractor.cs: adopted main's `CSharpIdentifierPattern` refactor (already encodes verbatim `@?[_\p{L}]\w*`, superset of iter-7's `@?\w+` widening) for enum/namespace/interface/struct/class regexes; unified C# symbol-name normalization on main's `NormalizeCSharpVerbatimIdentifiers` helper while preserving iter-6 explanation comments; kept main's new `NormalizeKotlinSymbolName` / `NormalizeSqlSymbolName` helpers. - Mcp/McpToolHandlers.cs: combined MCP index readiness tracking so `index_project` stamps both `csharp_metadata_target_ready` (runs `ResolveCSharpMetadataTargets` + `MarkMetadataTargetReady("csharp")` when C# files exist) and `sql_graph_contract_ready`. - Models/QueryResults.cs: exposed both `CSharpMetadataTargetReady` (HEAD) and `SqlGraphContractReady` / `SqlGraphContractDegradedReason` (main) on StatusResult. - Models/SymbolRecord.cs: kept both `IsMetadataTarget` (HEAD) and `SameLineSignatureOccurrenceIndex` (main). Build verified with `dotnet build` on both CLI and test projects. Full test suite: 2778 passed, 2 skipped (perf), 0 failed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.
Summary
signature LIKE '%: %'heuristic with a persistedsymbols.is_metadata_targetcolumn, populated byDbWriter.ResolveCSharpMetadataTargets. The resolver parses C# class signatures at index time and runs a fixed-point iteration over directAttribute/System.Attributebases plus transitive in-repo class graph (class FooAttribute : BaseAttris promoted whenBaseAttr : Attributelives in the repo). Closes C# metadata target ambiguity: non-attribute inheritors with same name as a real attribute class can silently drop deps/impact edges #435.using Namespace;/using Alias = FQN;plus repo-wideglobal usingaggregation). For qualified bases the first segment is expanded againstusing Alias = Target;— splitting on whichever of.(member access) or::(qualified-alias-member, §7.8) comes first — soclass FooAttribute : Alias.MetaBaseandclass FooAttribute : Alias::MetaBaseboth resolve toTarget.MetaBasebefore the qualified-index lookup. An alias whose target isSystemorSystem.Attributere-triggers the direct-Attribute rule after expansion.@Foo.@Bar,class X : @BaseAttr,global::@Foo.@Bar.BaseAttr) are canonicalized on both writer and reader sides — the declaration regexes were widened to accept@?\w+, theSymbolExtractoralias regex now capturesusing @AliasAttr = Target;, andDbWriter.StripCSharpVerbatimPrefixeswalks::boundaries in addition to.soglobal::@Foo.@Bar.BaseAttrcollapses toglobal::Foo.Bar.BaseAttr.codeindex_meta.metadata_target_version_csharp. The reader uses a three-way branch: (1) readiness-stamped →is_metadata_target = 1; (2) column present but row not stamped → legacysignature LIKE '%: %'; (3) column missing on read-only legacy DB → name-suffix fallback. The contract was bumped 1→6 across six review iterations; older stamps force reader degradation until a reindex republishesis_metadata_target.Cli/IndexCommandRunner.cs(full scan, update,--rebuild) andMcp/McpToolHandlers.cs(MCPindex_project) clear the stamp before write and restamp only on success — partial/failed runs leave the DB conservatively degraded.StatusResult.CSharpMetadataTargetReady/csharp_metadata_target_readyJSON flag and thecdidx status/cdidx indexhuman output now surface the readiness state with explicitWARNlines so contract degradation is visible without parsing JSON.::alias-qualified) and the status surface (ready/degraded for workspaces with and without C# files, contract-stamp-missing vs contract-stamp-current).Test plan
dotnet build(CLI + Tests) on .NET 8 — 0 warnings, 0 errors.dotnet testfull suite on .NET 8 — 2159 passed, 2 perf-skipped, 0 failed.Alias.MetaBase,Alias::MetaBase,Sys.Attributeviausing Sys = System;, verbatim declarationclass @BaseAttr : Attribute,global::@Foo.@Bar.BaseAttr, partial class family split across files) — all emit the expected metadata edges fromdeps --json.metadata_target_version_csharpin an indexed DB flipsstatus --json'scsharp_metadata_target_readytofalseanddeps --jsonfalls back to the legacy reader path; re-indexing restores stamped trust.NO_BLOCK — no actionable findings.🤖 Generated with Claude Code