From b868196c78bf5cf8c91549bcf8e0d6f8b3769400 Mon Sep 17 00:00:00 2001 From: Julien CHABLE Date: Fri, 4 Sep 2026 09:49:45 +0200 Subject: [PATCH 01/36] fix(producer): four of the parked Important findings, and one that only half closes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Working the register's parked Importants after sifting the 24 unverified ones against the code (13 were already closed by the remediation waves; 11 were not). **E2-2 — the CHANGELOG had no entry at all for the code-graph stage.** Grepping `[Unreleased]` for `okfgen|code-graph|tree-sitter` found nothing; CLAUDE.md, ROADMAP.md and the producer README had all been updated in detail and the changelog had not. Four entries now: the stage itself, the completeness line, `--check`, and the recursive `.sln` detection. **B1-2 — one unreadable `.csproj` aborted the whole run.** `FileEligibility.ReferencesTestSdk` caught `XmlException` and `IOException` from `XDocument.Load` and nothing else, and it is called from `CodeGraphBuilder.Build`'s per-file loop, which deliberately does not wrap its body -- so an `UnauthorizedAccessException` did not degrade one file, it ended the repository's run. Now filtered on the five ways that call can fail on a path that existed a moment ago. The comment says in its own words that this is NOT covered by a test and why: the two provokable exceptions were already caught, and the three added need either an elevation the suite does not have or a loader seam this type does not offer. **D3-1 — the escape guard could be deleted with the suite green.** `SourceOwnershipMapTests` queried the ABSOLUTE spelling twice, but removing `SourceOwnershipMap.Relativize`'s escape guard stores the entry under its `../`-prefixed relative form, which neither lookup asked about. The test now also asserts on that spelling. Verified by mutation: with the guard replaced by `return relative;` the test fails, and it is the only one that does. **B2-3 — improved, NOT closed, and the commit says so rather than the register.** `The_size_check_never_loads_an_oversized_file_into_memory` asserted only `FileStatus.SkippedTooLarge` over content that decodes cleanly, so the ordering it is named for was invisible to it. The fixture is now undecodable bytes, which separates a decode hoisted above the size check (`SkippedEncoding`) from the correct order (`SkippedTooLarge`). MEASURED: this still does not catch the regression the finding actually names -- moving `File.ReadAllBytes` above the length check keeps the test green, because the decode that would betray it runs later either way. Catching a read requires observing the read, which needs a seam. Left open deliberately, not silently. 611 tests green, format clean, no fixture touched. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0166CcRSdoZrL8g1Suruy6Uz --- CHANGELOG.md | 27 +++++++++++++++++++ .../CodeGraph/FileEligibility.cs | 23 ++++++++++++---- .../CodeGraph/HostileInputTests.cs | 13 ++++++--- .../Generation/SourceOwnershipMapTests.cs | 9 +++++++ 4 files changed, 64 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8be512ee..4a90ed8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,33 @@ and this project adheres to ### Added +- **`okfgen` gains a C# code-graph stage** (`producers/OkfProducer`, outside + `OKF4net.sln` and outside CI by decision). `generate` now emits one `code/` + concept per namespace, type and member, with resolved `## Calls` links. Two + engines behind one contract: tree-sitter extracts symbols and call sites + language-agnostically, and Roslyn resolves C# call sites exactly — without + `MSBuildWorkspace`, querying project inputs through a bounded `msbuild -getItem` + subprocess — with a name-match resolver covering what Roslyn cannot reach. Call + sites are identified by UTF-8 byte offset, since the two engines natively speak + UTF-16. +- **`okfgen generate` prints a completeness report** to stderr, prefixed `run: `, + on every run that reaches the generation stage: files visited and how many fell + to each cause, whether the traversal was complete, projects detected and how + many of the closure compiled, exact-resolver coverage, and how many `code` + concepts are reachable from `overview`. It exists because every other account a + run gives of itself is a note gated on its own trigger, so a run printing + nothing was indistinguishable from a mechanism that did not fire. On stderr, so + no CI gate reading stdout changes and nothing lands in the bundle. +- **`okfgen generate --check`** compares a regenerated bundle against the one on + disk, over a copy, and reports drift without writing. Backed by a golden + fixture. +- **Project detection follows every `*.sln` in the tree**, not only one at the + repository root. A root-only lookup let the first root solution decide the whole + answer: measured on this repository, 9 of 17 `.csproj` were detected, and the + 194 `code` concepts of the undetected projects belonged to no package concept + and were unreachable from `overview` — which `okf validate` does not report, + because an orphan dangles nothing. A `.csproj` that no solution references is + still not a package. - **`ConceptSearch.TopDiversified`** — picks the top N of a scored result set while rotating across top-level id families, so one family cannot take every slot in a truncated window. `ConceptSearch.Search` is unchanged; this is an diff --git a/producers/src/OkfProducer.Core/CodeGraph/FileEligibility.cs b/producers/src/OkfProducer.Core/CodeGraph/FileEligibility.cs index 3d95f3f5..afc18c4f 100644 --- a/producers/src/OkfProducer.Core/CodeGraph/FileEligibility.cs +++ b/producers/src/OkfProducer.Core/CodeGraph/FileEligibility.cs @@ -172,11 +172,24 @@ private static bool ReferencesTestSdk(string absoluteCsprojPath) return (xml.Root?.Descendants().Where(e => e.Name.LocalName == "PackageReference") ?? []) .Any(e => string.Equals((string?)e.Attribute("Include"), TestSdkPackageId, StringComparison.OrdinalIgnoreCase)); } - catch (System.Xml.XmlException) - { - return false; - } - catch (IOException) + // Every way XDocument.Load can fail on a path that existed a moment ago, not just the two + // that were listed here. This method is called from CodeGraphBuilder.Build's per-file loop, + // which deliberately does NOT wrap its body -- so an exception escaping here does not degrade + // one file, it aborts the whole repository's run. `File.Exists` above narrows nothing: it + // answers for the instant it was asked, and it returns false for a directory, so the + // interesting failures are the ones it cannot see -- an ACL that denies read, a path the + // platform rejects, a file deleted between the two calls. + // + // NOT covered by an executable test, and that is a real gap rather than an oversight: + // UnauthorizedAccessException and SecurityException cannot be provoked here portably (a + // read-denying ACL is Windows-specific and needs elevation the suite does not have), and the + // two that CAN be provoked -- XmlException, IOException -- were already caught before this + // change. Covering the rest needs a loader seam on this type; see the register entry. + catch (Exception e) when (e is System.Xml.XmlException + or IOException + or UnauthorizedAccessException + or System.Security.SecurityException + or NotSupportedException) { return false; } diff --git a/producers/tests/OkfProducer.Tests/CodeGraph/HostileInputTests.cs b/producers/tests/OkfProducer.Tests/CodeGraph/HostileInputTests.cs index 25a291c6..3db44fd8 100644 --- a/producers/tests/OkfProducer.Tests/CodeGraph/HostileInputTests.cs +++ b/producers/tests/OkfProducer.Tests/CodeGraph/HostileInputTests.cs @@ -159,10 +159,17 @@ public void A_file_over_the_size_cap_is_skipped_whole_never_truncated() public void The_size_check_never_loads_an_oversized_file_into_memory() { // The file's declared length alone must decide SkippedTooLarge -- the extractor is never - // given a chance to read its bytes at all. A file that reports itself too large but whose - // actual bytes (if ever read) would decode fine still gets rejected on length. + // given a chance to read its bytes at all. + // + // The bytes are INVALID UTF-8 on purpose, and that is the whole point of the fixture. With + // content that decodes cleanly, this test asserts nothing an implementation could fail: + // checking the length first and reading first both end at SkippedTooLarge, so the ordering + // the test is named for is invisible to it. Undecodable bytes separate the two -- length + // first still yields SkippedTooLarge, while a regression that reads before it measures + // yields SkippedEncoding, because the decode throws before the size is ever consulted. using var tmp = new TempDir(); - var path = tmp.Write("big.cs", "namespace N;\npublic class T {}"); + var path = Path.Combine(tmp.Path, "big.cs"); + File.WriteAllBytes(path, [0x6E, 0x73, 0xFF, 0xFE, 0x00, 0x41]); var result = Extract(path, ExtractionLimits.Default with { MaxFileBytes = 1 }); diff --git a/producers/tests/OkfProducer.Tests/Generation/SourceOwnershipMapTests.cs b/producers/tests/OkfProducer.Tests/Generation/SourceOwnershipMapTests.cs index e0209308..3fbc6543 100644 --- a/producers/tests/OkfProducer.Tests/Generation/SourceOwnershipMapTests.cs +++ b/producers/tests/OkfProducer.Tests/Generation/SourceOwnershipMapTests.cs @@ -40,6 +40,15 @@ public void A_file_outside_the_repository_is_dropped_rather_than_keyed_absolutel Assert.Null(map.OwnerOf(outside)); Assert.Empty(map.ClaimantsOf(outside.Replace('\\', '/'))); + + // The spelling the guard's REMOVAL produces, and the only one that separates the two + // behaviours. Both assertions above query the ABSOLUTE form; Relativize does not store that + // form under any branch, so deleting the escape guard leaves the entry keyed as + // `../elsewhere/Far.cs` and both of them stay green over a map that did keep the file. + var relativized = Path.GetRelativePath(root, outside).Replace('\\', '/'); + Assert.StartsWith("../", relativized, StringComparison.Ordinal); + Assert.Null(map.OwnerOf(relativized)); + Assert.Empty(map.ClaimantsOf(relativized)); } [Fact] From 433dbf62c5424c1244ef17819f29c0023cc5fa78 Mon Sep 17 00:00:00 2001 From: Julien CHABLE Date: Fri, 4 Sep 2026 10:39:55 +0200 Subject: [PATCH 02/36] test(producer): make two stated guarantees executable, behind a two-member read seam B2-3 and B1-2 were both blocked on the same thing: the claims are about a read, and a `FileStatus` cannot describe one. Arbitrated with the user, who chose the seam over documenting the gap. `IFileSystemReader` has exactly two members -- `TryGetLength` and `OpenRead` -- and `SystemFileReader` is the default everywhere, so every seam parameter carries a default and no production call site changed. It is not a filesystem abstraction: writes, enumeration and metadata stay on `File`/`Directory` where their callers already are. **B2-3 closed.** `The_size_check_never_loads_an_oversized_file_into_memory` named an ordering it could not see: rejecting on the declared length and reading first then rejecting both end at `SkippedTooLarge`. Measured on the previous commit -- hoisting `File.ReadAllBytes` above the length check left it green. A recording reader now asserts the file was never opened, and the same mutation against the new code fails it. **B1-2 closed.** `ReferencesTestSdk` is reached from `CodeGraphBuilder.Build`'s per-file loop, whose body is deliberately unwrapped, so an escaping exception ends the repository's run rather than skipping one file. The catch was widened in the previous commit with a comment admitting it was untested; a reader that grants metadata and refuses to open now provokes exactly that, and removing `UnauthorizedAccessException` from the filter makes the test throw. Two deliberate non-changes. Reparse-point detection stays on the real `FileInfo` and is NOT routed through the seam -- it is a containment decision, and a seam able to answer it is a seam able to waive it. And an unreadable `.csproj` keeps its existing direction, "not demonstrably a test project", so the file stays in scope: treating it as a test project would silently drop real source on a permissions accident. 612 tests green, format clean, no fixture touched. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0166CcRSdoZrL8g1Suruy6Uz --- .../TreeSitterExtractor.cs | 42 ++++++++++++-- .../CodeGraph/CodeGraphBuilder.cs | 4 +- .../CodeGraph/FileEligibility.cs | 15 ++--- .../CodeGraph/IFileSystemReader.cs | 55 +++++++++++++++++++ .../CodeGraph/HostileInputTests.cs | 41 ++++++++++---- .../OkfProducer.Tests/CodeGraph/ScopeTests.cs | 33 +++++++++++ 6 files changed, 165 insertions(+), 25 deletions(-) create mode 100644 producers/src/OkfProducer.Core/CodeGraph/IFileSystemReader.cs diff --git a/producers/src/OkfProducer.CodeGraph.TreeSitter/TreeSitterExtractor.cs b/producers/src/OkfProducer.CodeGraph.TreeSitter/TreeSitterExtractor.cs index 38f1ed0d..f8679c45 100644 --- a/producers/src/OkfProducer.CodeGraph.TreeSitter/TreeSitterExtractor.cs +++ b/producers/src/OkfProducer.CodeGraph.TreeSitter/TreeSitterExtractor.cs @@ -24,6 +24,32 @@ namespace OkfProducer.CodeGraph.TreeSitter; /// public sealed class TreeSitterExtractor : ILanguageExtractor, IDisposable { + private readonly IFileSystemReader _reader; + + /// + /// Reads to its end, pre-sized from the length already measured so the + /// common case allocates once. is a hint and never a bound: the + /// bound is , checked before this is reached. + /// + private static byte[] ReadFully(Stream stream, long expectedLength) + { + using var buffer = new MemoryStream(expectedLength is > 0 and <= int.MaxValue ? (int)expectedLength : 0); + stream.CopyTo(buffer); + return buffer.ToArray(); + } + + /// + /// Creates an extractor reading through , or the real filesystem when it + /// is . + /// + /// The seam exists so a test can observe WHETHER a file was read, which no + /// can report: the guarantee that a file's declared length alone decides + /// is about an operation that did not happen, and only a + /// collaborator can witness that. Reparse-point detection is NOT routed through it -- see + /// TryReadSource. + /// + public TreeSitterExtractor(IFileSystemReader? reader = null) => _reader = reader ?? SystemFileReader.Instance; + private const string CommentNodeType = "comment"; private const string FileScopedNamespaceNodeType = "file_scoped_namespace_declaration"; private const string NamespaceDeclarationNodeType = "namespace_declaration"; @@ -590,30 +616,38 @@ private static IEnumerable Descendants(Node root) /// to the decoded text when every guard passes; otherwise returns the /// to report and sets to . /// - private static FileStatus? TryReadSource(string relativePath, string absolutePath, ExtractionLimits limits, out string source) + private FileStatus? TryReadSource(string relativePath, string absolutePath, ExtractionLimits limits, out string source) { source = string.Empty; byte[] bytes; try { + // The link check stays on the real FileInfo and is deliberately NOT behind the reader + // seam. It is a containment decision, and a seam able to answer it is a seam able to + // waive it -- a test double could then let a symlink through a check whose whole purpose + // is that nothing does. var fileInfo = new FileInfo(absolutePath); if (fileInfo.LinkTarget is not null || IsUnderReparsePoint(absolutePath, relativePath)) { return FileStatus.SkippedSymlink; } - if (!fileInfo.Exists) + // Length BEFORE any read, and the seam is what lets a test say so: a status cannot + // distinguish "rejected on its declared length" from "read, then rejected", and those are + // the same outcome for a caller and a very different one for memory. + if (_reader.TryGetLength(absolutePath) is not { } length) { return FileStatus.SkippedUnreadable; } - if (fileInfo.Length > limits.MaxFileBytes) + if (length > limits.MaxFileBytes) { return FileStatus.SkippedTooLarge; } - bytes = File.ReadAllBytes(absolutePath); + using var stream = _reader.OpenRead(absolutePath); + bytes = ReadFully(stream, length); } catch (IOException) { diff --git a/producers/src/OkfProducer.Core/CodeGraph/CodeGraphBuilder.cs b/producers/src/OkfProducer.Core/CodeGraph/CodeGraphBuilder.cs index c4fbca02..cd551d14 100644 --- a/producers/src/OkfProducer.Core/CodeGraph/CodeGraphBuilder.cs +++ b/producers/src/OkfProducer.Core/CodeGraph/CodeGraphBuilder.cs @@ -11,7 +11,7 @@ namespace OkfProducer.Core.CodeGraph; /// site's relative path and offset), so a missing or non-owning resolver degrades precision, never /// the shape of the output (§2.1). /// -public sealed class CodeGraphBuilder(ILanguageExtractor extractor, IReadOnlyList profiles, IReadOnlyList resolvers) +public sealed class CodeGraphBuilder(ILanguageExtractor extractor, IReadOnlyList profiles, IReadOnlyList resolvers, IFileSystemReader? reader = null) { /// /// Extracts every eligible file in 's repository, concatenates and @@ -126,7 +126,7 @@ public CodeGraph Build(RepositorySnapshot snapshot, ExtractionLimits limits, Sco continue; } - if (!FileEligibility.IsEligible(relativePath, snapshot, scope)) + if (!FileEligibility.IsEligible(relativePath, snapshot, scope, reader)) { continue; } diff --git a/producers/src/OkfProducer.Core/CodeGraph/FileEligibility.cs b/producers/src/OkfProducer.Core/CodeGraph/FileEligibility.cs index afc18c4f..fa17b6c8 100644 --- a/producers/src/OkfProducer.Core/CodeGraph/FileEligibility.cs +++ b/producers/src/OkfProducer.Core/CodeGraph/FileEligibility.cs @@ -33,7 +33,7 @@ public static class FileEligibility /// -- the same treatment already gives a file matching no /// . /// - public static bool IsEligible(string relativePath, RepositorySnapshot snapshot, ScopeOptions scope) + public static bool IsEligible(string relativePath, RepositorySnapshot snapshot, ScopeOptions scope, IFileSystemReader? reader = null) { var directorySegments = DirectorySegments(relativePath); @@ -58,7 +58,7 @@ public static bool IsEligible(string relativePath, RepositorySnapshot snapshot, } } - return !IsOwnedByTestProject(relativePath, snapshot); + return !IsOwnedByTestProject(relativePath, snapshot, reader ?? SystemFileReader.Instance); } /// @@ -104,7 +104,7 @@ private static bool ContainsIgnoreCase(string[] names, string segment) /// resolved path back off disk, rather than adding raw project data to /// , keeps that record's shape unchanged for every other consumer. /// - private static bool IsOwnedByTestProject(string relativePath, RepositorySnapshot snapshot) + private static bool IsOwnedByTestProject(string relativePath, RepositorySnapshot snapshot, IFileSystemReader reader) { var fileDirectory = DirectorySegments(relativePath); @@ -134,7 +134,7 @@ private static bool IsOwnedByTestProject(string relativePath, RepositorySnapshot } var absoluteCsprojPath = Path.Combine(snapshot.RepoPath, bestProjectPath.Replace('/', Path.DirectorySeparatorChar)); - return ReferencesTestSdk(absoluteCsprojPath); + return ReferencesTestSdk(absoluteCsprojPath, reader); } // Ordinal, not OrdinalIgnoreCase: every other path comparison in this codebase (§6.2's "never a @@ -159,16 +159,17 @@ private static bool IsAncestorOrSame(string[] directory, string[] descendant) return true; } - private static bool ReferencesTestSdk(string absoluteCsprojPath) + private static bool ReferencesTestSdk(string absoluteCsprojPath, IFileSystemReader reader) { - if (!File.Exists(absoluteCsprojPath)) + if (reader.TryGetLength(absoluteCsprojPath) is null) { return false; } try { - var xml = XDocument.Load(absoluteCsprojPath); + using var stream = reader.OpenRead(absoluteCsprojPath); + var xml = XDocument.Load(stream); return (xml.Root?.Descendants().Where(e => e.Name.LocalName == "PackageReference") ?? []) .Any(e => string.Equals((string?)e.Attribute("Include"), TestSdkPackageId, StringComparison.OrdinalIgnoreCase)); } diff --git a/producers/src/OkfProducer.Core/CodeGraph/IFileSystemReader.cs b/producers/src/OkfProducer.Core/CodeGraph/IFileSystemReader.cs new file mode 100644 index 00000000..a4d2eacf --- /dev/null +++ b/producers/src/OkfProducer.Core/CodeGraph/IFileSystemReader.cs @@ -0,0 +1,55 @@ +// SPDX-License-Identifier: LGPL-3.0-or-later +namespace OkfProducer.Core.CodeGraph; + +/// +/// The two filesystem reads the extraction stage performs, behind a seam so a test can observe them +/// and provoke them. +/// +/// Why this exists rather than direct File.* calls. Two guarantees on this path +/// were stated and could not be executed. That a file's declared length alone decides +/// -- proving it needs to observe whether the bytes were +/// read at all, and a status cannot say. And that an unreadable .csproj degrades one file +/// instead of ending the repository's run -- provoking it needs an +/// from a read, which on this platform needs an ACL and an elevation a test suite does not have. +/// Both are answerable once the read is a collaborator. +/// +/// Deliberately two members and no more. This is not a filesystem abstraction: writes, +/// enumeration and metadata stay on and +/// where their callers already are. It covers exactly the reads whose ORDER or FAILURE a test needs +/// to pin. +/// +public interface IFileSystemReader +{ + /// + /// The file's length in bytes, or when it does not exist or is not a file. + /// Answers for the instant it is asked and nothing later -- a caller that then opens the file + /// still handles failure. + /// + long? TryGetLength(string absolutePath); + + /// + /// Opens the file for reading. Throws exactly what throws; + /// callers on this path treat a failure as a skipped file, never as a failed run. + /// + Stream OpenRead(string absolutePath); +} + +/// +/// over the real filesystem -- the default everywhere in production, +/// and the reason every seam parameter can carry a default rather than ripple through call sites. +/// +public sealed class SystemFileReader : IFileSystemReader +{ + /// The single shared instance; the type is stateless. + public static SystemFileReader Instance { get; } = new(); + + /// + public long? TryGetLength(string absolutePath) + { + var info = new FileInfo(absolutePath); + return info.Exists ? info.Length : null; + } + + /// + public Stream OpenRead(string absolutePath) => File.OpenRead(absolutePath); +} diff --git a/producers/tests/OkfProducer.Tests/CodeGraph/HostileInputTests.cs b/producers/tests/OkfProducer.Tests/CodeGraph/HostileInputTests.cs index 3db44fd8..b39f36c4 100644 --- a/producers/tests/OkfProducer.Tests/CodeGraph/HostileInputTests.cs +++ b/producers/tests/OkfProducer.Tests/CodeGraph/HostileInputTests.cs @@ -158,22 +158,39 @@ public void A_file_over_the_size_cap_is_skipped_whole_never_truncated() [Fact] public void The_size_check_never_loads_an_oversized_file_into_memory() { - // The file's declared length alone must decide SkippedTooLarge -- the extractor is never - // given a chance to read its bytes at all. - // - // The bytes are INVALID UTF-8 on purpose, and that is the whole point of the fixture. With - // content that decodes cleanly, this test asserts nothing an implementation could fail: - // checking the length first and reading first both end at SkippedTooLarge, so the ordering - // the test is named for is invisible to it. Undecodable bytes separate the two -- length - // first still yields SkippedTooLarge, while a regression that reads before it measures - // yields SkippedEncoding, because the decode throws before the size is ever consulted. + // The claim is about an operation that does NOT happen, so no FileStatus can carry it: + // rejecting on the declared length and reading first then rejecting both end at + // SkippedTooLarge. Before the reader seam existed this test asserted only that status, and + // MEASURED green with `File.ReadAllBytes` hoisted above the length check -- it named an + // ordering it could not see. The recording reader is what turns the claim into an assertion. using var tmp = new TempDir(); - var path = Path.Combine(tmp.Path, "big.cs"); - File.WriteAllBytes(path, [0x6E, 0x73, 0xFF, 0xFE, 0x00, 0x41]); + var path = tmp.Write("big.cs", "namespace N;\npublic class T {}"); + var reader = new RecordingReader(); + using var extractor = new TreeSitterExtractor(reader); - var result = Extract(path, ExtractionLimits.Default with { MaxFileBytes = 1 }); + var result = extractor.Extract( + "big.cs", path, CSharpProfile.Instance, ExtractionLimits.Default with { MaxFileBytes = 1 }); Assert.Equal(FileStatus.SkippedTooLarge, result.Status); + Assert.False(reader.Opened); + } + + /// + /// An over the real filesystem that records whether the bytes were + /// ever opened. Length still comes from disk, so the size decision under test is the production + /// one. + /// + private sealed class RecordingReader : IFileSystemReader + { + public bool Opened { get; private set; } + + public long? TryGetLength(string absolutePath) => SystemFileReader.Instance.TryGetLength(absolutePath); + + public Stream OpenRead(string absolutePath) + { + Opened = true; + return SystemFileReader.Instance.OpenRead(absolutePath); + } } [Fact] diff --git a/producers/tests/OkfProducer.Tests/CodeGraph/ScopeTests.cs b/producers/tests/OkfProducer.Tests/CodeGraph/ScopeTests.cs index 8d74e43e..fe6af30d 100644 --- a/producers/tests/OkfProducer.Tests/CodeGraph/ScopeTests.cs +++ b/producers/tests/OkfProducer.Tests/CodeGraph/ScopeTests.cs @@ -221,4 +221,37 @@ private static RepositorySnapshot SnapshotWithProject(bool referencesTestSdk, st var package = new PackageManifest("nuget", csprojRelativePath, projectName, null); return new RepositorySnapshot(repoPath, "test-repo", [package], []); } + + [Fact] + public void An_unreadable_project_file_leaves_the_run_going_instead_of_ending_it() + { + // ReferencesTestSdk is reached from CodeGraphBuilder.Build's per-file loop, whose body is + // deliberately NOT wrapped -- so an exception escaping this call does not skip one file, it + // ends the whole repository's run. UnauthorizedAccessException is the reachable case (an ACL + // that denies read on a .csproj), and it cannot be provoked on a real file without an + // elevation this suite does not have: that is why the read is a seam and this reader is a + // double rather than a chmod. + var snapshot = SnapshotWithTestProject(projectDirectory: "integration/OKF4net.Verify"); + + var eligible = FileEligibility.IsEligible( + "integration/OKF4net.Verify/AuditTests.cs", snapshot, ScopeOptions.Default, new DenyingReader()); + + // Unreadable means "not demonstrably owned by a test project", so the file stays in scope. + // That direction is deliberate: the alternative -- treating an unreadable .csproj as a test + // project -- would silently drop real source on a permissions accident. + Assert.True(eligible); + } + + /// + /// Reports every path as present and then refuses to open it: the shape of an ACL that grants + /// metadata and denies read. Remove the matching catch from + /// FileEligibility.ReferencesTestSdk and the test above throws instead of failing, which is + /// exactly the production behaviour it exists to forbid. + /// + private sealed class DenyingReader : IFileSystemReader + { + public long? TryGetLength(string absolutePath) => 1; + + public Stream OpenRead(string absolutePath) => throw new UnauthorizedAccessException(absolutePath); + } } From 16e0a62750f84a74b2b7cb8b5090974024f63598 Mon Sep 17 00:00:00 2001 From: Julien CHABLE Date: Fri, 4 Sep 2026 11:01:07 +0200 Subject: [PATCH 03/36] test(producer): put the prefix-root guard and the validate verb on a critical path **D3-3.** `IsWithinPrefixRoot` is the only thing between `RemoveEmptyDirectories`' walk and the bundle root, and no test put it on the critical path: every fixture in `PruningTests` leaves a sibling directory that breaks the loop after one rung, so the walk never climbed far enough for the guard to be what stopped it. The new test owns `code/csharp` rather than `code`, and that is the whole fixture. With `code`, the rung above the prefix root is the bundle root, which `overview.md` already protects through the files check -- so deleting the guard changes nothing observable and a test written that way would prove nothing. With `code/csharp`, the rung above is `code`, empty by then, with no file and no sibling to protect it. Verified by mutation: replacing the guard with `true` fails this test and only this test. **E2-1.** Nothing in the solution called `Run("validate", ...)`, so everything the composition root wires for that verb was held by no test -- while `CliTests`' own doc claimed it exercises "the shipped composition, in process". Four tests now: the conformant path, the non-conformant path, the `BundleLoadException` branch, and the option name. Two mutations, both measured: inverting `IsConformant ? 0 : 1` fails the first two; renaming `--okf` to `--bundle` fails the fourth. The fourth test needed correcting before it did. Its first version asserted over a bundle that was never generated, so `--bundle` on a non-existent path exited non-zero whether the option was recognised or not -- it held under the rename it was named for, and the mutation is what showed it. It now generates the bundle first, and the comment says why that is the fixture rather than setup. 617 tests green, format clean, no fixture touched. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0166CcRSdoZrL8g1Suruy6Uz --- producers/tests/OkfProducer.Tests/CliTests.cs | 59 +++++++++++++++++++ .../Generation/PruningTests.cs | 30 ++++++++++ 2 files changed, 89 insertions(+) diff --git a/producers/tests/OkfProducer.Tests/CliTests.cs b/producers/tests/OkfProducer.Tests/CliTests.cs index 5fe11b42..11e26267 100644 --- a/producers/tests/OkfProducer.Tests/CliTests.cs +++ b/producers/tests/OkfProducer.Tests/CliTests.cs @@ -1158,6 +1158,65 @@ private static void CommitAndDetach(string repoPath) Assert.Null(GitRevision.CurrentBranch(repoPath)); } + + [Fact] + public void The_validate_verb_reports_a_conformant_bundle_and_exits_zero() + { + // Nothing in the solution called Run("validate", ...) at all, so everything the composition + // root wires for this verb -- the option name, the diagnostic lines, the count line, the + // IsConformant ternary and the BundleLoadException branch -- was held by no test, while this + // class's own doc claimed it exercises "the shipped composition, in process". + // BundleValidationRunnerTests covers the runner; these four cover the CLI around it. + using var workspace = NewWorkspace(out var repo, out var bundle); + Assert.Equal(0, Run("generate", "--repo", repo, "--out", bundle).ExitCode); + + var result = Run("validate", "--okf", bundle); + + Assert.Equal(0, result.ExitCode); + Assert.Contains("0 error(s)", result.Output, StringComparison.Ordinal); + } + + [Fact] + public void The_validate_verb_exits_one_on_a_bundle_that_is_not_conformant() + { + // The other side of the ternary, so inverting it fails one of the two rather than neither. + // A concept with no `type` is the one thing §11 hard-requires, so this is an error and not a + // warning -- warnings leave the bundle conformant and would not separate the branches. + using var workspace = NewWorkspace(out var repo, out var bundle); + Assert.Equal(0, Run("generate", "--repo", repo, "--out", bundle).ExitCode); + File.WriteAllText(Path.Combine(bundle, "broken.md"), "---\ntitle: t\n---\n\nbody\n"); + + var result = Run("validate", "--okf", bundle); + + Assert.Equal(1, result.ExitCode); + } + + [Fact] + public void The_validate_verb_reports_an_unloadable_bundle_on_stderr_and_exits_one() + { + // The catch (BundleLoadException) branch. Dropping it turns a missing bundle from a reported + // error into an unhandled exception, and no test noticed. + using var workspace = NewWorkspace(out _, out var bundle); + + var result = Run("validate", "--okf", Path.Combine(bundle, "does-not-exist")); + + Assert.Equal(1, result.ExitCode); + Assert.StartsWith("error: ", result.Error, StringComparison.Ordinal); + } + + [Fact] + public void The_validate_verb_names_its_bundle_option_okf() + { + // Pins the option NAME. The bundle is generated first, and that is the whole fixture: without + // it, `--bundle` on a path that does not exist exits non-zero whether the option is recognised + // or not, so the assertion holds under the rename it claims to catch -- measured, after the + // first version of this test did exactly that. + using var workspace = NewWorkspace(out var repo, out var bundle); + Assert.Equal(0, Run("generate", "--repo", repo, "--out", bundle).ExitCode); + + Assert.NotEqual(0, Run("validate").ExitCode); + Assert.NotEqual(0, Run("validate", "--bundle", bundle).ExitCode); + } // ---- assertions ------------------------------------------------------------------------- private sealed record CliResult(int ExitCode, string Output, string Error); diff --git a/producers/tests/OkfProducer.Tests/Generation/PruningTests.cs b/producers/tests/OkfProducer.Tests/Generation/PruningTests.cs index 76e69c75..db1f018d 100644 --- a/producers/tests/OkfProducer.Tests/Generation/PruningTests.cs +++ b/producers/tests/OkfProducer.Tests/Generation/PruningTests.cs @@ -1718,6 +1718,36 @@ public void The_manifest_records_the_scope_the_run_covered() Assert.Equal(new ScopeOptions(IncludeTests: true, IncludeInternal: false), GenerationManifest.TryRead(tmp.Path)?.Scope); } + [Fact] + public void The_directory_ladder_stops_at_the_owned_prefix_root_and_not_above_it() + { + // `IsWithinPrefixRoot` is the only thing between RemoveEmptyDirectories' walk and the bundle + // root, and no test put it on the critical path: every other fixture here leaves a sibling + // directory that breaks the loop after one rung, so the walk never climbs far enough for the + // guard to be what stops it. + // + // The prefix is two segments deep on purpose. With `code`, the rung above the prefix root is + // the bundle root, which `overview.md` already protects through the files check -- so deleting + // the guard changes nothing observable and a test written that way proves nothing. With + // `code/csharp`, the rung above is `code`, which is empty by then and has no file and no + // sibling to protect it. The guard is the only reason it survives. + using var tmp = new TempDir(); + + WriteRun(tmp, [A], complete: true, ownedPrefix: "code/csharp"); + var codeDirectory = Path.Combine(tmp.Path, "code"); + Assert.True(Directory.Exists(Path.Combine(codeDirectory, "csharp", "n", "t"))); + + var result = WriteRun(tmp, [], complete: true, ownedPrefix: "code/csharp"); + + // The ladder did climb -- without this the assertion below would pass over a walk that never + // ran at all, which is the failure mode the finding is about. + Assert.Contains(A, result.Pruned.Select(id => id.ToString())); + Assert.False(Directory.Exists(Path.Combine(codeDirectory, "csharp"))); + + // And it stopped where it had to. + Assert.True(Directory.Exists(codeDirectory)); + } + // --------------------------------------------------------------------------------------------- // Helpers. // --------------------------------------------------------------------------------------------- From 9c785f4cbe0b6902c5712fd1a057b77a5cafc5b4 Mon Sep 17 00:00:00 2001 From: Julien CHABLE Date: Fri, 4 Sep 2026 13:13:33 +0200 Subject: [PATCH 04/36] refactor(producer): remove two dead properties, rename a test to what it pins, surface the dirty-tree caveat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **C1-2 — `RoslynResolver.IsAvailable` and `IsComplete` removed.** Remediation wave 2b had already found that nothing in `producers/src` reads either one (the run's report comes from `GenerateRun.ReportProjects` iterating `Projects`) and had corrected the docs and §7.2 accordingly, choosing to KEEP the properties on the grounds that a host embedding the resolver would ask that question. Reversed here on the user's arbitration: no host does, `producers/` is not a published library, and a property whose only readers are the tests asserting on it is the exact shape this branch has paid for thirteen times over. The two predicates now live in `RoslynResolverTests` as `AnyCompiled` and `AllCompiled`, derived from `Projects` in one line -- which is what any caller wanting the summary would write. The `Count > 0` clause is documented there rather than lost: `All` over an empty list is vacuously true, and an empty project list is precisely the state in which every call fell back to name matching. Fourteen assertions rewritten, including the scratch fixture's own oracle. §7.2 records the reversal beside the decision it overturns rather than replacing it. **F-3 — the test is named for what it pins.** `The_registry_spans_the_code_family_as_well_as_packages_and_docs` built an EMPTY graph, so no `code/` id was produced and reverting the code family to a `Generate`-local `usedIds` left both assertions green. Adding code concepts would not have fixed that, and the comment now says why rather than leaving it to be rediscovered: the four families use disjoint prefixes by construction, so a cross-family collision cannot be built and a code-local registry would disambiguate code ids among themselves exactly as the shared one does. §3.4's "one registry" is structural -- it is why a class of collision is impossible -- not behavioural, and no test can separate it from its absence without changing the id scheme. What the test does pin, and what a regression could break, is that a doc titled "overview" coexists with the bare `overview` id. **D4b-1 — the dirty-tree caveat reaches a user.** `GitRevision` stated correctly, in a code comment and nowhere else, that `revision` names the committed HEAD and so can name a commit the bundle was not generated from. It now also appears in `--rev`'s help, in `BundleDrift.CheckDescription` (where it matters most: both sides carry the same revision, so `--check` can report no drift for a bundle neither side was generated from), and in a README section of its own. The fourth surface the finding names -- the `overview` concept itself -- is deliberately NOT in this commit. Writing it changes emitted bytes and therefore the golden, which is regenerated once at the end of this remediation rather than twice. 617 tests green, format clean, no fixture touched. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0166CcRSdoZrL8g1Suruy6Uz --- ...26-08-31-okf-producer-code-graph-design.md | 4 +- producers/README.md | 17 ++++ producers/src/OkfProducer.Cli/OkfgenCli.cs | 4 +- .../RoslynResolver.cs | 79 ++++--------------- .../Generation/BundleDrift.cs | 5 +- .../CodeGraph/RoslynResolverTests.cs | 53 +++++++++---- .../Generation/CodeConceptGeneratorTests.cs | 26 ++++-- 7 files changed, 98 insertions(+), 90 deletions(-) diff --git a/docs/superpowers/specs/2026-08-31-okf-producer-code-graph-design.md b/docs/superpowers/specs/2026-08-31-okf-producer-code-graph-design.md index 09fe47e8..a03f5518 100644 --- a/docs/superpowers/specs/2026-08-31-okf-producer-code-graph-design.md +++ b/docs/superpowers/specs/2026-08-31-okf-producer-code-graph-design.md @@ -489,7 +489,9 @@ La correction 1 récupère bien les fichiers générés **par le SDK** (`*.Globa > > Vérifié par `grep` sur tout `producers/src` : aucun lecteur de production pour l'une ou l'autre propriété ; les seuls lecteurs sont les tests. Ce que l'opérateur voit vient de `GenerateRun.ReportProjects`, qui itère `RoslynResolver.Projects` et émet **une note par projet non compilé**, en nommant le projet, sa `RoslynProjectAvailability` et le détail. C'est strictement plus d'information qu'un booléen. > -> Arbitrage retenu : **corriger le document et les commentaires plutôt que câbler une seconde voie**. Câbler `IsComplete` dans le rapport ajouterait une ligne agrégée à côté d'une voie par projet qui fonctionne déjà et qui dit davantage. Les deux propriétés restent : c'est la question qu'un hôte intégrant le resolver pose, et le fixture de `RoslynResolverTests` s'appuie sur `IsComplete` pour prouver que son dépôt scratch a réellement compilé. +> Arbitrage retenu : **corriger le document et les commentaires plutôt que câbler une seconde voie**. Câbler `IsComplete` dans le rapport ajouterait une ligne agrégée à côté d'une voie par projet qui fonctionne déjà et qui dit davantage. +> +> **Suite (dépilage des findings Important, 2026-09-04) : les deux propriétés ont été SUPPRIMÉES.** La vague 2b les avait gardées en écrivant ici qu'« un hôte intégrant le resolver pose cette question » et que le fixture de `RoslynResolverTests` s'appuyait sur `IsComplete`. Arbitrage de l'utilisateur : aucun hôte ne les lit, `producers/` n'est pas une bibliothèque publiée, et une propriété dont les seuls lecteurs sont les tests qui l'assertent est exactement la forme que cette branche a payée treize fois (« assertion incapable d'échouer »). `AnyCompiled` et `AllCompiled` vivent désormais dans `RoslynResolverTests`, dérivées de `RoslynResolver.Projects` en une ligne — ce qu'écrirait tout appelant voulant le résumé. La clause `Count > 0` reste documentée là-bas : `All` sur une liste vide est vrai par vacuité, et une liste vide est précisément l'état où *tous* les appels sont retombés sur le name matching. **Rayon d'impact — il déborde du projet en échec.** C'est le point le moins intuitif et il doit être écrit noir sur blanc. Un projet qui ne compile pas ne coûte pas seulement la précision *sur ses propres fichiers* : diff --git a/producers/README.md b/producers/README.md index ec6494ca..a535b758 100644 --- a/producers/README.md +++ b/producers/README.md @@ -103,6 +103,23 @@ It is **off by default on purpose**. Turning it on by default would silently deg resolution quality of every run that exists today, which is a worse trade than a documented hazard with a lever next to it. +### What `revision` on `overview` does and does not say + +`overview` carries a `revision` field holding the exact HEAD sha, and it names the +**committed HEAD — never the working tree**. Both `git show -s` and `git rev-parse` report +the commit currently checked out regardless of any uncommitted change to the source the run +actually scanned, so on a dirty tree `revision` names a commit this bundle was **not**, byte +for byte, generated from. There is no attempt to detect that and no fabricated value in its +place: a sha is either the checked-out one or absent. + +Two consequences worth stating rather than discovering. A reader treating `revision` as +"check out this commit and you get this bundle" is right only when the tree was clean at +generation time. And `--check` cannot see the difference either: both sides carry the same +revision, so a bundle generated from uncommitted edits can be reported as having no drift. + +Generating from a clean tree is what makes the field mean what it appears to mean. + + ### What a run says about itself Every `generate` that reaches the generation stage prints one **completeness report** to diff --git a/producers/src/OkfProducer.Cli/OkfgenCli.cs b/producers/src/OkfProducer.Cli/OkfgenCli.cs index 3bc182c7..c20a37ba 100644 --- a/producers/src/OkfProducer.Cli/OkfgenCli.cs +++ b/producers/src/OkfProducer.Cli/OkfgenCli.cs @@ -101,7 +101,9 @@ private static RootCommand BuildRootCommand(ProducerServices services, TextWrite Description = "The git ref --repo-url permalinks are built against. Defaults to the current branch name, never " + "a commit sha -- a sha would rewrite every code concept's `resource` on the next commit. On a " - + "detached HEAD there is no branch name to read, so this becomes required for permalinks.", + + "detached HEAD there is no branch name to read, so this becomes required for permalinks. " + + "The recorded revision names the COMMITTED HEAD, not the working tree: with uncommitted local " + + "edits it names a commit this bundle was not generated from.", }; var checkOption = new Option("--check") { Description = BundleDrift.CheckDescription }; diff --git a/producers/src/OkfProducer.CodeGraph.Roslyn/RoslynResolver.cs b/producers/src/OkfProducer.CodeGraph.Roslyn/RoslynResolver.cs index cddf3a34..954bff6e 100644 --- a/producers/src/OkfProducer.CodeGraph.Roslyn/RoslynResolver.cs +++ b/producers/src/OkfProducer.CodeGraph.Roslyn/RoslynResolver.cs @@ -26,12 +26,21 @@ namespace OkfProducer.CodeGraph.Roslyn; /// /// Degradation. A project that cannot be compiled cleanly is reported unavailable and its /// files are not ed, so 's baseline stands for them -/// untouched. This resolver never resolves from a compilation that has errors. , -/// and are what let a caller distinguish "ran, and -/// resolved nothing" from "could not run" -- the difference between a repository with no internal -/// calls and one whose call graph is only approximate. That distinction is a question a caller can -/// ask, not a pruning gate: see 's own doc comment for why it cannot be one, -/// and for the fact that only reaches the run's report today. +/// untouched. This resolver never resolves from a compilation that has errors. +/// is what lets a caller distinguish "ran, and resolved nothing" from "could not run" -- the +/// difference between a repository with no internal calls and one whose call graph is only +/// approximate -- and it reports that per project, naming each one and why, rather than as a +/// boolean. +/// +/// Two summary properties, IsAvailable and IsComplete, used to sit here. +/// Remediation wave 2b established that nothing in producers/src read either one: the run's +/// report comes from GenerateRun.ReportProjects iterating , and §7.2 had +/// claimed otherwise. That wave chose to correct the claims and keep the properties; this one removed +/// them instead. The reasoning for keeping them was that a host embedding the resolver would ask +/// exactly that question -- but no host does, this is not a published library, and a property whose +/// only readers are the tests asserting on it is a shape this branch has already been burned by. A +/// caller wanting the summary derives it from in one line, which is what the +/// tests now do. /// /// Loud, but never fatal to the run. An unknown LangVersion is refused rather /// than degraded to a preview language version (correction 3) -- but the refusal is scoped to the one @@ -138,64 +147,6 @@ private RoslynResolver( /// public IReadOnlyList Projects { get; } - /// - /// Whether at least one project compiled, i.e. whether this resolver can settle anything at all. - /// - /// Nothing in producers/src reads this, or . Grepped, - /// not assumed: the only readers are this project's tests. See for what - /// the run's report actually feeds from and why that was left as it is. - /// - public bool IsAvailable => Projects.Any(p => p.Availability == RoslynProjectAvailability.Compiled); - - /// - /// Whether this resolver covered the repository completely: at least one project, and every one of - /// them compiled. means some C# was resolved by name alone, so this run's - /// call graph is approximate and an operator should be told. - /// - /// It is not the pruning gate, and an earlier version of this comment said it was. - /// Task 11 settled it against the code: which concepts exist is decided entirely by extraction -- - /// CodeGraphBuilder builds CodeGraph.Symbols from - /// output filtered by , and no resolver contributes a symbol - /// to it. A resolver decides only whether a call site renders as a link or as a code span. So a - /// degraded resolver cannot make a symbol absent, which is the only way an incomplete - /// picture could turn into a wrong deletion. Gating on it would also make pruning dead code on this - /// very repository -- src/OKF4net.Cli uses a source generator and does not compile here, so - /// this property is on an ordinary checkout -- which is the same trap - /// sets, for the same shape of reason. What DOES gate pruning is - /// plus the per-file ; see - /// BundleWriter. - /// - /// - /// The Count > 0 clause is the whole point of this property, not a formality. - /// Projects.All(...) over an empty list is vacuously , so a resolver - /// constructed with no projects at all -- which is precisely the state in which EVERY call in the - /// repository fell back to name matching -- would otherwise report itself complete. That state is - /// reachable rather than theoretical: finding no .csproj in a C# repository is a known gap - /// in this producer, and it yields an empty project list, not an error. - /// - /// - /// - /// What that clause protects is a caller who asks this question. Claiming completeness would tell - /// one the call graph is exact when every edge in it was in fact guessed from a name -- and a wrong - /// ## Calls link reads as confidently as a right one. It forbids nothing, gates nothing, and - /// blocks no deletion; see the paragraph above for why it cannot. - /// - /// - /// It does not feed the run's report, and this comment used to say it did -- as did - /// 7.2. Grepped across producers/src: nothing reads this property or - /// ; the only readers are this project's tests. What the operator - /// actually sees is GenerateRun.ReportProjects iterating and - /// emitting one note per project that did not compile, naming the project, the - /// and the detail. That is strictly more than a single - /// boolean says, which is why the fix was to correct this claim and 7.2's rather than to wire a - /// second, coarser channel alongside a working one. Both properties stay: they are the question a - /// host embedding this resolver would ask, and the fixture in RoslynResolverTests asserts - /// on to prove its own scratch repository really compiled -- a test that - /// would otherwise measure nothing. - /// - public bool IsComplete => - Projects.Count > 0 && Projects.All(p => p.Availability == RoslynProjectAvailability.Compiled); - /// /// Compiles , plus every project they reference that lives under /// , and returns a resolver over whichever of them came out clean. diff --git a/producers/src/OkfProducer.Core/Generation/BundleDrift.cs b/producers/src/OkfProducer.Core/Generation/BundleDrift.cs index 3ce6e4b0..9e6c9e89 100644 --- a/producers/src/OkfProducer.Core/Generation/BundleDrift.cs +++ b/producers/src/OkfProducer.Core/Generation/BundleDrift.cs @@ -237,7 +237,10 @@ public static class BundleDrift + "does not make a link invisible. Where regenerating writes a concept at the link's own path, " + "that path is reported as drift, in a sentence saying a link is what the bundle holds there; " + "where it writes concepts UNDER a linked directory, each of them is reported. Every skipped " - + "link the differences do not already name is reported as a note."; + + "link the differences do not already name is reported as a note. One case a clean result " + + "does NOT cover: `revision` names the committed HEAD, never the working tree, so with " + + "uncommitted local edits both sides carry the same revision and the check can report no " + + "drift for a bundle neither side was generated from."; /// /// Copies the bundle at into a temporary directory, hands that copy diff --git a/producers/tests/OkfProducer.Tests/CodeGraph/RoslynResolverTests.cs b/producers/tests/OkfProducer.Tests/CodeGraph/RoslynResolverTests.cs index c44d607b..27a2fa40 100644 --- a/producers/tests/OkfProducer.Tests/CodeGraph/RoslynResolverTests.cs +++ b/producers/tests/OkfProducer.Tests/CodeGraph/RoslynResolverTests.cs @@ -133,8 +133,8 @@ public void An_unknown_language_version_costs_its_own_project_and_no_other() Assert.Contains("99", stranded.Detail, StringComparison.Ordinal); // The run continued, and said honestly that it is partial. - Assert.True(resolver.IsAvailable, Describe(resolver)); - Assert.False(resolver.IsComplete); + Assert.True(AnyCompiled(resolver), Describe(resolver)); + Assert.False(AllCompiled(resolver)); // The stranded project is left to the baseline; the sound one still resolves exactly. Assert.False(resolver.Owns("stranded/Stranded.cs")); @@ -331,8 +331,8 @@ public void A_project_that_cannot_be_queried_is_reported_unavailable_rather_than var resolver = RoslynResolver.Create(RepoRoot(), [missing]); - Assert.False(resolver.IsAvailable); - Assert.False(resolver.IsComplete); + Assert.False(AnyCompiled(resolver)); + Assert.False(AllCompiled(resolver)); Assert.False(resolver.Owns("src/OKF4net/ConceptId.cs")); var report = Assert.Single(resolver.Projects); Assert.Equal(RoslynProjectAvailability.MsBuildQueryFailed, report.Availability); @@ -356,8 +356,8 @@ public void A_resolver_that_ran_cleanly_says_so_even_when_it_resolves_nothing() // moved: Resolve builds its result by iterating `sites`, so it returns empty for ANY // implementation of it, including a broken one. That assertion could not fail. The three // below can: each names a value this fixture's own restore-and-compile has to have produced. - Assert.True(_scratch.Resolver.IsAvailable); - Assert.True(_scratch.Resolver.IsComplete); + Assert.True(AnyCompiled(_scratch.Resolver)); + Assert.True(AllCompiled(_scratch.Resolver)); Assert.All(_scratch.Resolver.Projects, p => Assert.Equal(RoslynProjectAvailability.Compiled, p.Availability)); } @@ -373,12 +373,12 @@ public void A_resolver_with_no_projects_at_all_is_not_complete() // What that wrong answer costs is a SILENT report, not a deletion. Task 11 settled that this // property is not the pruning gate and cannot be one: no resolver contributes a symbol to // CodeGraph.Symbols, so a degraded resolver can turn a call link into a code span but never - // make a concept absent, and pruning acts on absence. See RoslynResolver.IsComplete's own doc + // make a concept absent, and pruning acts on absence. See this file's AllCompiled doc // comment; pruning gates on RunStatus.TraversalComplete plus the per-file FileStatus. var resolver = RoslynResolver.Create(RepoRoot(), []); - Assert.False(resolver.IsComplete); - Assert.False(resolver.IsAvailable); + Assert.False(AllCompiled(resolver)); + Assert.False(AnyCompiled(resolver)); Assert.Empty(resolver.Projects); Assert.False(resolver.Owns("src/OKF4net/ConceptId.cs")); } @@ -591,7 +591,7 @@ public void A_project_completed_by_a_source_generator_degrades_rather_than_resol var resolver = RoslynResolver.Create(repository.Root, [repository.Project]); - Assert.False(resolver.IsAvailable); + Assert.False(AnyCompiled(resolver)); var report = Assert.Single(resolver.Projects); Assert.Equal(RoslynProjectAvailability.CompilationHadErrors, report.Availability); Assert.False(resolver.Owns("Serialization.cs")); @@ -616,7 +616,7 @@ public void A_failed_project_does_not_retract_correct_edges_in_the_clean_project Assert.Equal(RoslynProjectAvailability.CompilationHadErrors, library.Availability); var application = Assert.Single(resolver.Projects, p => p.ProjectPath == repository.ApplicationProject); Assert.Equal(RoslynProjectAvailability.Compiled, application.Availability); - Assert.False(resolver.IsComplete); + Assert.False(AllCompiled(resolver)); using var extractor = new TreeSitterExtractor(); var extracted = extractor.Extract("app/Program.cs", repository.ApplicationSourceFile, CSharpProfile.Instance, ExtractionLimits.Default); @@ -721,7 +721,7 @@ public void A_call_across_an_unbuilt_project_reference_still_resolves_Exact() var resolver = RoslynResolver.Create(repository.Root, [repository.ApplicationProject]); - Assert.True(resolver.IsComplete, Describe(resolver)); + Assert.True(AllCompiled(resolver), Describe(resolver)); Assert.Equal(2, resolver.Projects.Count); using var extractor = new TreeSitterExtractor(); @@ -762,7 +762,7 @@ public void A_compile_item_over_the_size_cap_is_refused_by_the_roslyn_engine_too Assert.False(capped.Owns("Big.cs")); // Still a cap and not a collapse: the project compiled and its in-bounds file is owned. - Assert.True(capped.IsComplete, Describe(capped)); + Assert.True(AllCompiled(capped), Describe(capped)); Assert.True(capped.Owns("Small.cs"), Describe(capped)); } @@ -960,6 +960,29 @@ private static (int ExitCode, string Output, string Error) Generate(string repoP private static string Describe(RoslynResolver resolver) => string.Join("; ", resolver.Projects.Select(p => $"{Path.GetFileName(p.ProjectPath)}: {p.Availability} {p.Detail}")); + /// + /// Whether at least one project compiled -- "ran, and resolved nothing" as against "could not run". + /// + /// + /// These two predicates were RoslynResolver.IsAvailable and RoslynResolver.IsComplete + /// until this file became their only reader and they were removed. They live here now because that + /// is where they are used, and a derived one-liner over is + /// what any caller wanting the summary would write. + /// + private static bool AnyCompiled(RoslynResolver resolver) => + resolver.Projects.Any(p => p.Availability == RoslynProjectAvailability.Compiled); + + /// + /// Whether the repository was covered completely: at least one project, and every one compiled. + /// + /// The Count > 0 clause is the point, not a formality. All over an empty list + /// is vacuously true, and an empty project list is precisely the state in which EVERY call fell back + /// to name matching -- reachable rather than theoretical, since finding no .csproj yields an + /// empty list and not an error. + /// + private static bool AllCompiled(RoslynResolver resolver) => + resolver.Projects.Count > 0 && resolver.Projects.All(p => p.Availability == RoslynProjectAvailability.Compiled); + /// /// A hand-built for the tests that only exercise /// 's parse options: no MSBuild round trip, no files, nothing that @@ -983,7 +1006,7 @@ private static (int Attached, int Total, int Exact, int Joined, string Unjoined) var resolver = RoslynResolver.Create(repositoryRoot, [projectPath]); Assert.True( - resolver.IsAvailable, + AnyCompiled(resolver), $"no project compiled, so attachment cannot be measured. This test needs a restored repository. {Describe(resolver)}"); using var extractor = new TreeSitterExtractor(); @@ -1091,7 +1114,7 @@ public ScratchProject() Restore(projectPath); Resolver = RoslynResolver.Create(Root, [projectPath]); - Assert.True(Resolver.IsComplete, Describe(Resolver)); + Assert.True(AllCompiled(Resolver), Describe(Resolver)); using var extractor = new TreeSitterExtractor(); var symbols = new List(); diff --git a/producers/tests/OkfProducer.Tests/Generation/CodeConceptGeneratorTests.cs b/producers/tests/OkfProducer.Tests/Generation/CodeConceptGeneratorTests.cs index 83a8b954..079fabc9 100644 --- a/producers/tests/OkfProducer.Tests/Generation/CodeConceptGeneratorTests.cs +++ b/producers/tests/OkfProducer.Tests/Generation/CodeConceptGeneratorTests.cs @@ -301,14 +301,24 @@ public void Signature_bullets_are_ordered_by_declaration_site_not_by_input_order } [Fact] - public void The_registry_spans_the_code_family_as_well_as_packages_and_docs() - { - // §3.4: one registry, one allocation record for the whole run. Being exact about what that - // means, because the tempting claim is false: the four families use disjoint prefixes, so a doc - // titled "overview" lands on `docs/overview` and CANNOT collide with the bare `overview` id -- - // they coexist, which is what this asserts. What the shared registry buys is that `code/` is in - // the same record as the rest (the old Generate-local usedIds never covered it) and that - // `overview` is allocated rather than assumed. + public void A_doc_titled_overview_coexists_with_the_overview_concept() + { + // NAMED FOR WHAT IT PINS, after a review found the old name + // (`The_registry_spans_the_code_family_as_well_as_packages_and_docs`) promising something no + // assertion here could fail on: the graph is empty, so no `code/` id is produced at all and + // reverting the code family to a `Generate`-local `usedIds` leaves both assertions green. + // + // Adding code concepts would not fix that, and the reason is worth stating rather than + // rediscovering. §3.4's "one registry" has no observable consequence through this surface: the + // four families use DISJOINT PREFIXES by construction (`overview`, `docs/`, `packages/`, + // `code/`), so a cross-family collision cannot be built, and a code-local registry would + // disambiguate code ids among themselves exactly as the shared one does. The property is + // structural -- it is why a whole class of collision is impossible -- not behavioural, and a + // test cannot separate it from its absence without changing the id scheme itself. + // + // What this DOES pin, and what a regression could break: a doc titled "overview" lands on + // `docs/overview` and the bare `overview` id is allocated rather than assumed, so the two + // coexist instead of one overwriting the other. var snapshot = new RepositorySnapshot("/repo", "my-repo", [], [new DocFile("O.md", "overview")]); var ids = Ids(new ConceptGenerator().Generate(snapshot, GraphOf(), Options())); From 86253bd1a7d747a158387ec3d7727b096d886a4f Mon Sep 17 00:00:00 2001 From: Julien CHABLE Date: Fri, 4 Sep 2026 15:40:15 +0200 Subject: [PATCH 05/36] fix(producer): separate two declaration shapes the grammar's name field cannot tell apart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Concept ids change for the shapes below, which is what the user arbitrated. D3-2's new test is what covers the run that follows such a change. **D1b-I1 — generic arity, on TYPES only.** `Foo`, `Foo` and `Foo` in one namespace all report `name` as `Foo`, so they collapsed into one symbol group and rendered as overloads of a single member. §3.2's merge rule was written for method overloads, where merging is the point; extending it to unrelated types was never the spec's intent. The suffix is `_N`, not `-N`: the registry already appends `-2` to disambiguate two concepts wanting one id, and a reader must be able to tell "arity 1" from "second thing called Foo". So `foo`, `foo_1`, `foo_2` -- and `foo_1-2` if two `Foo` somehow met. Members are deliberately left alone, generic ones included, and a test says so rather than leaving the asymmetry to look like an oversight. A call site captures its callee as the bare identifier -- `Bar()` yields `Bar` -- so qualifying a generic METHOD would stop every call to it from matching by name. That trades a merged concept for a lost edge, which is the worse of the two. **D1b-I2 — explicit interface implementations.** `public void Bar()` and `void IFoo.Bar()` on one type also both report `Bar`, and collapsed into one concept carrying ONE description -- the first declaration's -- and both signatures, for two deliberately different implementations. The qualified form takes the interface as a dotted prefix (`ifoo.bar`), which is how C# writes it. Name matching no longer reaches the explicit member, and that is the correct outcome rather than a cost: it is not callable as `Bar()` on the type, so a call that used to bind to it was binding to the wrong member. Both were measured against the vendored grammar before being written, not inferred: `type_parameter_list` with N `type_parameter` children, and `explicit_interface_specifier` as a child of `method_declaration`. The whole suite was green before these changes, which is the finding's point -- no test covered either shape. **D3-2 — a rename is covered.** The one destructive interaction no test ran: old id pruned, new id written, in one run. §6.3's "not visited AND not on disk" inference was accepted partly on the claim that it was covered; it was not. Both directions are asserted, and so is the manifest tracking the rename rather than accumulating the old id, which would make the next run treat it as a candidate again. 621 tests green, format clean, no fixture touched. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0166CcRSdoZrL8g1Suruy6Uz --- .../TreeSitterExtractor.cs | 59 +++++++++++++++++- .../CodeGraph/TreeSitterExtractorTests.cs | 60 +++++++++++++++++++ .../Generation/PruningTests.cs | 32 ++++++++++ 3 files changed, 149 insertions(+), 2 deletions(-) diff --git a/producers/src/OkfProducer.CodeGraph.TreeSitter/TreeSitterExtractor.cs b/producers/src/OkfProducer.CodeGraph.TreeSitter/TreeSitterExtractor.cs index f8679c45..a71ddfcd 100644 --- a/producers/src/OkfProducer.CodeGraph.TreeSitter/TreeSitterExtractor.cs +++ b/producers/src/OkfProducer.CodeGraph.TreeSitter/TreeSitterExtractor.cs @@ -1,4 +1,5 @@ // SPDX-License-Identifier: LGPL-3.0-or-later +using System.Globalization; using System.Text; using OkfProducer.Core.CodeGraph; using TreeSitter; @@ -742,9 +743,9 @@ private static List ExtractSymbols( continue; } - var name = match.Captures.First(c => c.Name == "name").Node.Text; - var kind = IsTypeDeclaration(decl.Type) ? SymbolKind.Type : SymbolKind.Member; + var name = QualifyName(match.Captures.First(c => c.Name == "name").Node.Text, decl, kind); + var container = ComputeContainerPath(decl, namespaceContext.NameCovering(decl.StartIndex)); var modifiersText = ComputeModifiersText(decl, kind); var visibility = profile.VisibilityOf(modifiersText, kind); @@ -837,6 +838,60 @@ private static List ExtractCallSites(string source, Tree tree, Query c private static bool IsTypeDeclaration(string nodeType) => Array.IndexOf(TypeDeclarationNodeTypes, nodeType) >= 0; + /// The node holding a generic declaration's type parameters, when it has any. + private const string TypeParameterListNodeType = "type_parameter_list"; + + /// One type parameter inside a . + private const string TypeParameterNodeType = "type_parameter"; + + /// + /// The node naming the interface an explicitly-implemented member belongs to + /// (void IFoo.Bar()), when there is one. + /// + private const string ExplicitInterfaceSpecifierNodeType = "explicit_interface_specifier"; + + /// + /// The name a declaration is known by once two shapes the grammar's name field cannot tell + /// apart are separated. + /// + /// Generic arity, on TYPES only. Foo, Foo<T> and + /// Foo<T, U> in one namespace all report name as Foo, so they collapsed + /// into one symbol group and were rendered as overloads of a single member. §3.2's merge rule was + /// written for method overloads, where merging is the point; extending it to unrelated types was + /// never intended by the spec. The suffix is _N and not -N deliberately: the registry + /// already appends -2 to disambiguate two concepts that want one id, and a reader must be + /// able to tell "arity 1" from "second thing called Foo" at a glance. + /// + /// Members are deliberately left alone, generic ones included. A call site captures its + /// callee as the bare identifier (Bar<T>() yields Bar), so qualifying a generic + /// METHOD would stop every call to it from matching by name -- trading a merged concept for a lost + /// edge, which is the worse of the two. + /// + /// Explicit interface implementations. public void Bar() and + /// void IFoo.Bar() on one type also both report Bar, and collapsed into one concept + /// carrying one description -- the first declaration's -- and both signatures. The qualified form + /// takes the interface as a dotted prefix, which is how C# writes it. Name matching no longer + /// reaches the explicit member, and that is correct rather than a cost: it is not callable as + /// Bar() on the type, so a call that used to bind to it was binding to the wrong member. + /// + private static string QualifyName(string name, Node decl, SymbolKind kind) + { + if (kind == SymbolKind.Type) + { + var arity = decl.Children + .FirstOrDefault(c => c.Type == TypeParameterListNodeType) + ?.Children.Count(c => c.Type == TypeParameterNodeType) ?? 0; + + return arity == 0 ? name : $"{name}_{arity.ToString(CultureInfo.InvariantCulture)}"; + } + + var explicitInterface = decl.Children + .FirstOrDefault(c => c.Type == ExplicitInterfaceSpecifierNodeType) + ?.Text.TrimEnd('.'); + + return string.IsNullOrEmpty(explicitInterface) ? name : $"{explicitInterface}.{name}"; + } + /// /// Builds the dotted N.Outer.Inner path above : every ancestor /// that exposes a name field (a namespace, a type, or -- for a local function -- the diff --git a/producers/tests/OkfProducer.Tests/CodeGraph/TreeSitterExtractorTests.cs b/producers/tests/OkfProducer.Tests/CodeGraph/TreeSitterExtractorTests.cs index fdeb2fdf..2c7789d6 100644 --- a/producers/tests/OkfProducer.Tests/CodeGraph/TreeSitterExtractorTests.cs +++ b/producers/tests/OkfProducer.Tests/CodeGraph/TreeSitterExtractorTests.cs @@ -491,6 +491,66 @@ public class T Assert.Equal(first.Sites.Select(s => s.CalledName), second.Sites.Select(s => s.CalledName)); } + [Fact] + public void Generic_types_of_different_arity_are_distinct_symbols() + { + // `Foo`, `Foo` and `Foo` all report `name` as `Foo`, so they used to collapse into one + // symbol group and render as overloads of a single member -- §3.2's merge rule, written for + // method overloads, silently extended to unrelated types. The suffix is `_N` and not `-N` + // because the registry already appends `-2` to disambiguate two concepts wanting one id, and + // "arity 1" must not read as "second thing called Foo". + var result = ExtractSource(""" + namespace N; + public class Foo { } + public class Foo { } + public class Foo { } + """); + + Assert.Equal( + ["Foo", "Foo_1", "Foo_2"], + result.Symbols.Where(s => s.Kind == SymbolKind.Type).Select(s => s.Name).OrderBy(n => n, StringComparer.Ordinal)); + } + + [Fact] + public void A_generic_method_keeps_its_bare_name_so_calls_still_match_it() + { + // The deliberate asymmetry, and the reason it is not an oversight. A call site captures its + // callee as the bare identifier -- `Bar()` yields `Bar` -- so suffixing a generic METHOD + // would stop every call to it from matching by name. That trades a merged concept for a lost + // edge, which is the worse of the two, so arity is a TYPE rule only. + var result = ExtractSource(""" + namespace N; + public class T { public void Bar() { } } + """); + + Assert.Contains("Bar", result.Symbols.Select(s => s.Name)); + } + + [Fact] + public void An_explicit_interface_implementation_is_named_apart_from_the_public_member() + { + // Both report `name` as `Bar`, so the two collapsed into one concept carrying ONE description + // -- the first declaration's -- and both signatures, for two deliberately different + // implementations. The qualified form takes the interface as a dotted prefix, which is how C# + // writes it. + // + // Name matching no longer reaches the explicit member, and that is the correct outcome rather + // than a cost: it is not callable as `Bar()` on the type, so a call that used to bind to it was + // binding to the wrong member. + var result = ExtractSource(""" + namespace N; + public interface IFoo { void Bar(); } + public class Impl : IFoo + { + public void Bar() { } + void IFoo.Bar() { } + } + """); + + var names = result.Symbols.Where(s => s.Container == "N.Impl").Select(s => s.Name).OrderBy(n => n, StringComparer.Ordinal); + Assert.Equal(["Bar", "IFoo.Bar"], names); + } + private ExtractionResult ExtractSource(string source, string relativePath = "T.cs") { var directory = Directory.CreateTempSubdirectory("okfproducer-treesitter-").FullName; diff --git a/producers/tests/OkfProducer.Tests/Generation/PruningTests.cs b/producers/tests/OkfProducer.Tests/Generation/PruningTests.cs index db1f018d..83de8cdf 100644 --- a/producers/tests/OkfProducer.Tests/Generation/PruningTests.cs +++ b/producers/tests/OkfProducer.Tests/Generation/PruningTests.cs @@ -1748,6 +1748,38 @@ public void The_directory_ladder_stops_at_the_owned_prefix_root_and_not_above_it Assert.True(Directory.Exists(codeDirectory)); } + [Fact] + public void A_rename_prunes_the_old_id_and_writes_the_new_one_in_the_same_run() + { + // The one destructive interaction no test ran. §6.3's "not visited AND not on disk" inference + // was accepted partly on the claim that a rename is covered; it was not. It is also the shape + // this producer causes on itself: any change to the id scheme renames every concept it touches, + // so the run that follows such a change IS this test. + // + // Ordering is what makes it safe and what makes it worth pinning: Reconcile runs AFTER + // CommitStaging, and its candidates exclude this run's own ids -- so the new file is on disk + // before the old one is considered for deletion, and a scheme where the new id sorted before + // the old could not delete the new one by mistake. + using var tmp = new TempDir(); + + WriteRun(tmp, [A], complete: true); + Assert.True(File.Exists(Path.Combine(tmp.Path, "code/csharp/n/t/a.md"))); + + var result = WriteRun(tmp, [B], complete: true); + + // Both directions, because a writer that deleted everything would satisfy the first alone and a + // writer that deleted nothing would satisfy the second alone. + Assert.False(File.Exists(Path.Combine(tmp.Path, "code/csharp/n/t/a.md"))); + Assert.True(File.Exists(Path.Combine(tmp.Path, "code/csharp/n/t/b.md"))); + Assert.Equal([A], result.Pruned.Select(id => id.ToString())); + + // And the manifest tracks the rename rather than accumulating: a manifest still claiming the old + // id would make the NEXT run treat it as a candidate all over again. + var claimed = GenerationManifest.TryRead(tmp.Path)!.ConceptIds.Select(id => id.ToString()).ToList(); + Assert.Contains(B, claimed); + Assert.DoesNotContain(A, claimed); + } + // --------------------------------------------------------------------------------------------- // Helpers. // --------------------------------------------------------------------------------------------- From 4ada5b9909bde8d587769b67ced421b380edbfbb Mon Sep 17 00:00:00 2001 From: Julien CHABLE Date: Fri, 4 Sep 2026 16:25:05 +0200 Subject: [PATCH 06/36] fix(producer): stop a namespace's contents from being emitted inside a same-named type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit D1b-I3, the third residual collision §3.3 did not list. A class `Bar` in namespace `Foo` and a class `Baz` in namespace `Foo.Bar` reduce to the same raw path `[code, csharp, Foo, Bar]` for their parent, so the container was never synthesized -- a group already claimed the path -- and `Baz` registered under the TYPE's id. Measured on exactly the fixture the new test uses: `code/csharp/foo/bar/baz`, a top-level type rendered as a nested one, with the namespace having no concept at all and nothing dangling for `okf validate` to see. **Why the arbitrated fix had to change shape.** The chosen option was "disambiguate the container, like D1a-3". Measuring it first showed that closes only half: the children compute their parent key from their own container string, which still resolves to the type, so a discriminated container would exist and be empty -- a misleading concept added without removing the confusion. Put back to the user, who chose the real fix. **What separates them.** `SymbolFact.Container` is a flat dotted string, so a type nested in `Bar` and a top-level type in `Foo.Bar` both report `"Foo.Bar"`. The extractor knows the difference -- it walks ancestors and sees a namespace node in one case and a type node in the other -- and the flattening threw it away. `ContainerNamespace` now carries the namespace part, and the rule is a depth comparison: a group's parent is a namespace exactly when the parent's depth equals the namespace's. Two of the four rows in the new test have IDENTICAL raw segments and opposite parents, which is what no rule over the string can express -- the second test is that control, and it is the one a container-string fix would break. An `init` property defaulting to null, the same shape as `HeaderEndLine`: every fixture in this solution keeps compiling, `RoslynResolver` is untouched (it builds no `SymbolFact`), and a group with no recorded namespace is left exactly as it was, so the pass is inert rather than guessing. The namespace gets a marked raw path and its own id (`code/csharp/foo/bar-2`); the TYPE keeps the path it had, so no existing type's id moves. The marker is a keying device only: `SegmentName` now strips it everywhere a raw segment becomes something a reader sees. That was already true of a group's segments by accident -- ids and titles come from the `SymbolFact` -- but NOT of a synthesized container, whose id, title and description-chain fact are all derived from the segments, so a marker would have reached output. Verified by mutation: neutralising the new pass fails the collision test and leaves the nesting control green. 623 tests green, format clean, no fixture touched. §3.3 records the collision, the depth table, and what the pass deliberately does not do. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0166CcRSdoZrL8g1Suruy6Uz --- ...26-08-31-okf-producer-code-graph-design.md | 15 +++ .../TreeSitterExtractor.cs | 29 +++++ .../OkfProducer.Core/CodeGraph/SymbolFact.cs | 24 ++++ .../Generation/ConceptGenerator.cs | 108 +++++++++++++++++- .../Generation/CodeConceptGeneratorTests.cs | 64 +++++++++++ 5 files changed, 236 insertions(+), 4 deletions(-) diff --git a/docs/superpowers/specs/2026-08-31-okf-producer-code-graph-design.md b/docs/superpowers/specs/2026-08-31-okf-producer-code-graph-design.md index a03f5518..f09bdca1 100644 --- a/docs/superpowers/specs/2026-08-31-okf-producer-code-graph-design.md +++ b/docs/superpowers/specs/2026-08-31-okf-producer-code-graph-design.md @@ -145,6 +145,21 @@ Effet de bord bienvenu : les `partial class` réparties sur plusieurs fichiers f - **Segments réservés.** `BundleConceptWriter` rejette les concepts nommés `index` ou `log` (ils écraseraient les fichiers propres du bundle) — vérifié dans `src/OKF4net/BundleConceptWriter.cs`. Une propriété nommée `Index` est parfaitement plausible ; on **réutilise** `IsReservedSegment` de `ConceptGenerator.cs` au lieu d'en écrire un second. - **Collision résiduelle** (casse seule — `Parse` vs `parse` — ou type imbriqué homonyme d'un membre) : départage déterministe par **ordre Ordinal du nom d'origine**, le premier garde le slug nu, les suivants prennent `-2`, `-3`. Ordinal **sur le nom** et non sur (fichier, ligne), pour que le départage survive à un déplacement de fichier ou à un décalage de lignes. Mesuré à **0 occurrence** sur ce repo ; la règle existe pour Go et JS, où c'est courant. + +> **Correction apportée à l'implémentation (dépilage des findings Important, 2026-09-04) : une TROISIÈME collision résiduelle existait, non listée ici.** Une classe `Bar` dans le namespace `Foo` et une classe `Baz` dans le namespace `Foo.Bar` se réduisent au même chemin brut `[code, csharp, Foo, Bar]` pour leur parent. Le conteneur n'était donc jamais synthétisé — un groupe réclamait déjà le chemin — et `Baz` s'enregistrait sous l'identifiant du **type**. Mesuré sur cette fixture exacte avant correctif : `code/csharp/foo/bar/baz`, c'est-à-dire un type de premier niveau rendu comme un type imbriqué, le namespace n'ayant aucun concept. +> +> `SymbolFact.Container` ne peut pas trancher : c'est une chaîne pointée aplatie, et une classe imbriquée dans `Bar` comme une classe de premier niveau dans `Foo.Bar` y rapportent toutes deux `"Foo.Bar"`. L'extracteur, lui, connaît la différence — il descend les ancêtres et voit un nœud namespace dans un cas, un nœud type dans l'autre — et l'aplatissement la jetait. +> +> `SymbolFact.ContainerNamespace` (propriété `init`, défaut `null`, même forme que `HeaderEndLine`) porte désormais la partie namespace du conteneur. La règle : **le parent d'un groupe est un namespace exactement quand sa profondeur égale celle du namespace.** Deux lignes du tableau ci-dessous ont des segments identiques et des parents différents, ce qui est précisément ce qu'aucune règle sur la chaîne ne peut exprimer. +> +> | déclaration | segments | profondeur ns | parent | +> |---|---|---|---| +> | `Bar`, type dans ns `Foo` | `[code,csharp,Foo,Bar]` | 3 | namespace `Foo` | +> | `M`, membre de `Bar` | `[code,csharp,Foo,Bar,M]` | 3 | type `Bar` | +> | `Baz`, type dans ns `Foo.Bar` | `[code,csharp,Foo,Bar,Baz]` | 4 | namespace `Foo.Bar` | +> | `Baz`, imbriquée dans `Bar` | `[code,csharp,Foo,Bar,Baz]` | 3 | type `Bar` | +> +> Le namespace reçoit un chemin brut marqué et donc son propre identifiant (`code/csharp/foo/bar-2`) ; **le type garde le chemin qu'il avait**, donc aucun identifiant de type existant ne bouge. Un groupe dont `ContainerNamespace` est `null` — toute fixture de test, tout futur extracteur qui ne l'enregistre pas — est laissé exactement tel quel : la passe est inerte plutôt que devinatrice. - **Profondeur.** Un type devient à la fois le fichier `link-scanner.md` et le dossier `link-scanner/`. C'est légal, et `IndexGenerator` les liste dans deux rubriques distinctes du parent (document / `Subdirectories`). Conséquence assumée : **un `index.md` par dossier de type** (~170 sur ce repo). Sur un projet Java profond (`com/example/…`), les chemins s'allongent — **à surveiller vis-à-vis de `MAX_PATH` sous Windows**. ### 3.4 Un registre d'ids unique diff --git a/producers/src/OkfProducer.CodeGraph.TreeSitter/TreeSitterExtractor.cs b/producers/src/OkfProducer.CodeGraph.TreeSitter/TreeSitterExtractor.cs index a71ddfcd..ab229e94 100644 --- a/producers/src/OkfProducer.CodeGraph.TreeSitter/TreeSitterExtractor.cs +++ b/producers/src/OkfProducer.CodeGraph.TreeSitter/TreeSitterExtractor.cs @@ -777,6 +777,7 @@ private static List ExtractSymbols( // stops short of the declaration it names. Where there is no body at all // (`public record Foo(int X);`), the declaration's own last line is its header's. HeaderEndLine = (bodyStart?.StartPosition.Row ?? decl.EndPosition.Row) + 1, + ContainerNamespace = ComputeContainerNamespace(decl, namespaceContext.NameCovering(decl.StartIndex)), }); } @@ -908,6 +909,34 @@ private static string QualifyName(string name, Node decl, SymbolKind kind) /// reparented one and a doubled segment. /// /// + /// + /// The namespace part of the path builds -- the same walk, + /// keeping only the namespace ancestors -- or when there are none. + /// + /// Always a dotted PREFIX of the container path, because a namespace can only enclose a type + /// and never the reverse, so the generator can compare depths rather than re-parse either string. + /// A block-scoped namespace Foo.Bar { } contributes its whole dotted name as one ancestor, + /// exactly as it does over there, so the two stay in step. + /// + private static string? ComputeContainerNamespace(Node decl, string? fileScopedNamespaceName) + { + var segments = new List(); + for (var current = decl.Parent; current is not null; current = current.Parent) + { + if (current.Type == NamespaceDeclarationNodeType && current.GetChildForField(NameFieldName) is { } nameField) + { + segments.Insert(0, nameField.Text); + } + } + + if (fileScopedNamespaceName is not null) + { + segments.Insert(0, fileScopedNamespaceName); + } + + return segments.Count == 0 ? null : string.Join(".", segments); + } + private static string ComputeContainerPath(Node decl, string? fileScopedNamespaceName) { var segments = new List(); diff --git a/producers/src/OkfProducer.Core/CodeGraph/SymbolFact.cs b/producers/src/OkfProducer.Core/CodeGraph/SymbolFact.cs index d20655fa..e332d364 100644 --- a/producers/src/OkfProducer.Core/CodeGraph/SymbolFact.cs +++ b/producers/src/OkfProducer.Core/CodeGraph/SymbolFact.cs @@ -74,4 +74,28 @@ public sealed record SymbolFact( /// header line recorded, so use the full span". /// public int? HeaderEndLine { get; init; } + + /// + /// The namespace part of -- a dotted prefix of it -- or + /// when the extractor did not record one. + /// + /// What it separates, and why alone cannot. + /// Container is a flat dotted string, so a type nested inside Bar and a top-level + /// type in the namespace Foo.Bar both report "Foo.Bar" -- two structurally different + /// parents spelled identically. The consequence was measured: with a class Bar in namespace + /// Foo and a class Baz in namespace Foo.Bar, Baz was emitted at + /// code/csharp/foo/bar/baz, as though it were nested inside the type. §3.3 enumerated two + /// residual collisions; this was a third. + /// + /// The extractor knows the difference -- it walks ancestors and sees a namespace node in one case + /// and a type node in the other -- and the flattening threw it away. Recording where the namespace + /// STOPS is enough to recover it: a group's parent is a namespace exactly when the parent's depth + /// equals the namespace's. + /// + /// An init property defaulting to for the same reason as + /// : every fixture in this solution constructs a + /// positionally with no syntax tree to read this from, and means "not + /// recorded", which the generator treats exactly as it behaved before this existed. + /// + public string? ContainerNamespace { get; init; } } diff --git a/producers/src/OkfProducer.Core/Generation/ConceptGenerator.cs b/producers/src/OkfProducer.Core/Generation/ConceptGenerator.cs index 3e7ee7dc..c7e5fca4 100644 --- a/producers/src/OkfProducer.Core/Generation/ConceptGenerator.cs +++ b/producers/src/OkfProducer.Core/Generation/ConceptGenerator.cs @@ -537,6 +537,7 @@ private static CodeFamily BuildCodeConcepts( .ToList(); DisambiguateSharedRawPaths(groups); + DisambiguateNamespacesAgainstTypes(groups, language => ProfileFor(language, options, profiles)); // A call site names its caller as (container, name) and its target as (container, name) -- and // CallSite carries no language at all. Both joins below are therefore language-agnostic, which is @@ -1275,7 +1276,7 @@ private static OkfDocument BuildContainerConcept( /// the split direction only and a title is prose, not an id. /// private static string ContainerTitle(string[] segments) => - segments.Length <= MinimumContainerDepth ? segments[^1] : $"{segments[^2]}.{segments[^1]}"; + segments.Length <= MinimumContainerDepth ? SegmentName(segments[^1]) : $"{SegmentName(segments[^2])}.{SegmentName(segments[^1])}"; /// /// A container rendered as a for the description chain only, so a @@ -1297,8 +1298,8 @@ private static string ContainerTitle(string[] segments) => private static SymbolFact ContainerFact(string[] segments) => new( SymbolKind.Namespace, Language: segments[1], - Container: string.Join('.', segments[2..^1]), - Name: segments[^1], + Container: string.Join('.', segments[2..^1].Select(SegmentName)), + Name: SegmentName(segments[^1]), Signature: string.Empty, SymbolVisibility.Public, RelativePath: string.Empty, @@ -1927,7 +1928,7 @@ private static ConceptId RegisterContainerId( if (registeredByRawPath.TryGetValue(RawKey(segments[..^1]), out var parentId)) { - foreach (var candidate in new[] { segments[^1], ContainerToken }) + foreach (var candidate in new[] { SegmentName(segments[^1]), ContainerToken }) { try { @@ -2001,6 +2002,23 @@ private static string[] RawSegments(SymbolFact fact, LanguageProfile profile) /// private static string RawKey(IEnumerable segments) => string.Join(char.MinValue, segments); + /// + /// A raw segment with any suffix cut off -- the name it denotes, + /// as against the key it is. + /// + /// Every place that turns a raw segment back into something a reader sees goes through this, + /// which is what makes the discriminator a pure keying device. That was already true of a GROUP's + /// segments by accident rather than by design: a group's id and title come from its + /// , so nothing read its leaf segment as a name. It is not true of a + /// synthesized container, whose id, title and description-chain fact are all derived from the + /// segments themselves -- so a discriminator on a container segment would have reached output. + /// + private static string SegmentName(string segment) + { + var cut = segment.IndexOf(RawPathDiscriminator, StringComparison.Ordinal); + return cut < 0 ? segment : segment[..cut]; + } + /// /// The character appended to a leaf raw segment to tell two groups apart that /// would otherwise give the same path. Never NUL, which @@ -2044,6 +2062,88 @@ private static string[] RawSegments(SymbolFact fact, LanguageProfile profile) /// where before it was contradictory: both concepts listed the same children while one of them had no /// parent. /// + /// The marker a namespace segment carries when a type of the same name occupies its path. + private const string NamespaceMarker = "ns"; + + /// + /// Separates a namespace from a type that occupies the same raw path, so the namespace's contents + /// stop being emitted as though they were nested inside the type. + /// + /// The shape. A class Bar in namespace Foo, and a class Baz in + /// namespace Foo.Bar. Both reduce to the raw path [code, csharp, Foo, Bar] for their + /// parent, so the container was never synthesized (a group already claimed the path) and + /// Baz registered under the TYPE's id: measured as code/csharp/foo/bar/baz, a + /// top-level type rendered as a nested one, with the namespace having no concept at all. §3.3 + /// enumerated two residual collisions; this was a third, and it had no test. + /// + /// Why a flat container string cannot decide it, and what can. + /// is dotted and flat, so a type nested in Bar and a + /// top-level type in Foo.Bar both report "Foo.Bar". What separates them is + /// : a group's parent is a namespace exactly when the + /// parent's depth equals the namespace's. The four cases, on the fixture above -- + /// Bar (parent depth 3, namespace depth 3, so a namespace), its member M (4 against + /// 3, so a type), Baz in Foo.Bar (4 against 4, a namespace) and a Baz nested + /// inside Bar (4 against 3, a type) -- have two rows whose segments are IDENTICAL and whose + /// parents differ. That is precisely what the depth decides and the string cannot. + /// + /// What it does not do. A group whose is + /// -- every fixture in this solution, and any future extractor that does not + /// record one -- is left exactly as it was, so this pass is inert rather than guessing. And a + /// group never discriminates its own full path, only the namespace prefixes ABOVE it: the type + /// keeps the undecorated path it already had, so no existing type's id moves. + /// + private static void DisambiguateNamespacesAgainstTypes( + List<(SymbolKey Key, IReadOnlyList Declarations, string[] RawSegments)> groups, + Func profileFor) + { + var typePaths = new HashSet(StringComparer.Ordinal); + foreach (var group in groups) + { + if (group.Declarations[0].Kind == SymbolKind.Type) + { + typePaths.Add(RawKey(group.RawSegments)); + } + } + + if (typePaths.Count == 0) + { + return; + } + + for (var i = 0; i < groups.Count; i++) + { + var (key, declarations, segments) = groups[i]; + if (declarations[0].ContainerNamespace is not { } containerNamespace) + { + continue; + } + + // Two for `code` and the language tag, which every raw path starts with. + var namespaceDepth = 2 + profileFor(key.Language).SplitContainer(containerNamespace).Count; + var deepest = Math.Min(namespaceDepth, segments.Length - 1); + + string[]? rewritten = null; + for (var depth = MinimumContainerDepth; depth <= deepest; depth++) + { + // Against the REWRITTEN prefix once a shallower level has been marked: a marked prefix + // no longer matches any type path, which is the point -- one mark separates the whole + // branch below it rather than every level repeating the same separation. + if (!typePaths.Contains(RawKey((rewritten ?? segments)[..depth]))) + { + continue; + } + + rewritten ??= (string[])segments.Clone(); + rewritten[depth - 1] = SegmentName(rewritten[depth - 1]) + RawPathDiscriminator + NamespaceMarker; + } + + if (rewritten is not null) + { + groups[i] = (key, declarations, rewritten); + } + } + } + private static void DisambiguateSharedRawPaths( List<(SymbolKey Key, IReadOnlyList Declarations, string[] RawSegments)> groups) { diff --git a/producers/tests/OkfProducer.Tests/Generation/CodeConceptGeneratorTests.cs b/producers/tests/OkfProducer.Tests/Generation/CodeConceptGeneratorTests.cs index 079fabc9..cfbfb4d2 100644 --- a/producers/tests/OkfProducer.Tests/Generation/CodeConceptGeneratorTests.cs +++ b/producers/tests/OkfProducer.Tests/Generation/CodeConceptGeneratorTests.cs @@ -953,6 +953,70 @@ private static Frontmatter ExistingFrontmatter(string description, string descri private static CodeGraphModel GraphOf(params SymbolFact[] symbols) => new(symbols, [], RunStatus.Complete); + [Fact] + public void A_namespace_and_a_type_of_the_same_name_get_separate_concepts() + { + // A class `Bar` in namespace `Foo`, and a class `Baz` in namespace `Foo.Bar`. Both reduce to + // the raw path [code, csharp, Foo, Bar] for their parent, so the container was never + // synthesized and Baz registered under the TYPE's id. MEASURED before the fix, with exactly + // this fixture: overview, code/csharp/foo, code/csharp/foo/bar, code/csharp/foo/bar/baz, + // code/csharp/foo/bar/m, code/csharp/foo/bar/baz/q -- a top-level type rendered as a nested + // one, and the namespace with no concept at all. + // + // ContainerNamespace is what separates them: the parent is a namespace exactly when its depth + // equals the namespace's. Two of the four facts below have IDENTICAL raw segments and + // different parents, which is what no flat container string can express. + var graph = GraphOf( + new SymbolFact(SymbolKind.Type, "csharp", "Foo", "Bar", "public class Bar", + SymbolVisibility.Public, "src/Bar.cs", 0, 1, 1, 2, null) + { ContainerNamespace = "Foo" }, + new SymbolFact(SymbolKind.Member, "csharp", "Foo.Bar", "M", "public void M()", + SymbolVisibility.Public, "src/Bar.cs", 2, 3, 3, 4, null) + { ContainerNamespace = "Foo" }, + new SymbolFact(SymbolKind.Type, "csharp", "Foo.Bar", "Baz", "public class Baz", + SymbolVisibility.Public, "src/Baz.cs", 0, 1, 1, 2, null) + { ContainerNamespace = "Foo.Bar" }, + new SymbolFact(SymbolKind.Member, "csharp", "Foo.Bar.Baz", "Q", "public void Q()", + SymbolVisibility.Public, "src/Baz.cs", 2, 3, 3, 4, null) + { ContainerNamespace = "Foo.Bar" }); + + var ids = Ids(new ConceptGenerator().Generate(Snapshot(), graph, Options())); + + // The TYPE keeps the path it already had -- only the namespace moves, so no existing type's id + // shifts under this change. + Assert.Contains("code/csharp/foo/bar", ids); + Assert.Contains("code/csharp/foo/bar/m", ids); + + // The namespace is a concept of its own now, and its contents hang off IT. + Assert.Contains("code/csharp/foo/bar-2", ids); + Assert.Contains("code/csharp/foo/bar-2/baz", ids); + Assert.Contains("code/csharp/foo/bar-2/baz/q", ids); + + // The assertion that fails without the fix. + Assert.DoesNotContain("code/csharp/foo/bar/baz", ids); + } + + [Fact] + public void A_type_nested_in_another_type_still_hangs_off_it() + { + // The control for the test above, and the reason the fix is a depth comparison rather than a + // rule about names. `Baz` here has the SAME raw segments as the `Baz` above -- container + // "Foo.Bar", name "Baz" -- and must land in the opposite place, because its namespace stops one + // level higher. A fix that keyed on the container string alone would move this one too. + var graph = GraphOf( + new SymbolFact(SymbolKind.Type, "csharp", "Foo", "Bar", "public class Bar", + SymbolVisibility.Public, "src/Bar.cs", 0, 1, 1, 2, null) + { ContainerNamespace = "Foo" }, + new SymbolFact(SymbolKind.Type, "csharp", "Foo.Bar", "Baz", "public class Baz", + SymbolVisibility.Public, "src/Bar.cs", 2, 3, 3, 4, null) + { ContainerNamespace = "Foo" }); + + var ids = Ids(new ConceptGenerator().Generate(Snapshot(), graph, Options())); + + Assert.Contains("code/csharp/foo/bar/baz", ids); + Assert.DoesNotContain("code/csharp/foo/bar-2", ids); + } + private static SymbolFact Type(string container, string name, string path, string? doc = null) => new(SymbolKind.Type, "csharp", container, name, $"public class {name}", SymbolVisibility.Public, path, 0, 1, 1, 2, doc); From 144722525af22bb13998e5bb8cded5546efda471 Mon Sep 17 00:00:00 2001 From: Julien CHABLE Date: Sun, 6 Sep 2026 08:47:28 +0200 Subject: [PATCH 07/36] feat(producer): lock the engine versions and record them in the artefact INT-I5. Spec 6.2's determinism-versioning mechanism was entirely absent: no `packages.lock.json` anywhere, both engines pinned by a floor `Version=` only, and `generated.by` carrying `okfgen/{version}` alone. Grepping the ledger, the plan and the README for `lock|packages.lock|RestorePackages` found nothing -- an omission rather than a deferral, and it undid 6.2's "a version bump is a deliberate migration, never a drift you suffer". No format decision was needed: 6.2 prescribes the spelling (`okfgen/0.1.0 tree-sitter/x.y.z roslyn/a.b.c`), the surface (`overview`), and the scope of the lock (the `CodeGraph.*` projects). Measured on the fixture repository after the change: by: okfgen/0.1.0 tree-sitter/1.3.0 roslyn/5.3.0-2.26078.5 **Locking and recording answer different halves.** A lock file makes a bump deliberate; it cannot make a diff interpretable. Without the versions in the artefact, a golden that moved says only that something changed. **Each engine names itself.** `OkfProducer.Core` references neither, and should not, so `TreeSitterExtractor.EngineVersion` and `RoslynResolver.EngineVersion` read their own loaded assembly and the CLI -- the one project referencing everything -- collects them. `EngineVersions.Token` lives in Core so the two cannot come to spell a version differently: informational version preferred (that is the package version a reader can look up), anything after `+` dropped (SourceLink puts the commit sha there, which would churn the field on rebuilds that changed no dependency), and `unknown` rather than omission when an assembly declares neither -- a run whose engine version could not be read is a run whose determinism claim is weaker. **Two deliberate scopes.** On `overview` alone, like `at` and for 6.1's reason: the engines are a fact about the run, and repeating them on hundreds of `code/` concepts would rewrite every file on a bump that changed nothing in them. And none at all under `--no-code`, where no engine touched the repository -- the same reason that path passes a null manifest rather than an empty one. Both are pinned by their own test. Verified by mutation: dropping the engine tokens from the emission fails the first test and only it. 626 tests green, format clean, no fixture touched. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0166CcRSdoZrL8g1Suruy6Uz --- producers/src/OkfProducer.Cli/GenerateRun.cs | 8 ++++ .../OkfProducer.CodeGraph.Roslyn.csproj | 4 ++ .../RoslynResolver.cs | 7 ++++ .../packages.lock.json | 39 ++++++++++++++++++ .../OkfProducer.CodeGraph.TreeSitter.csproj | 7 ++++ .../TreeSitterExtractor.cs | 6 +++ .../packages.lock.json | 22 ++++++++++ .../CodeGraph/EngineVersions.cs | 40 +++++++++++++++++++ .../Generation/ConceptGenerator.cs | 13 ++++-- .../Generation/GenerateOptions.cs | 22 ++++++++++ .../Generation/CodeConceptGeneratorTests.cs | 40 +++++++++++++++++++ 11 files changed, 205 insertions(+), 3 deletions(-) create mode 100644 producers/src/OkfProducer.CodeGraph.Roslyn/packages.lock.json create mode 100644 producers/src/OkfProducer.CodeGraph.TreeSitter/packages.lock.json create mode 100644 producers/src/OkfProducer.Core/CodeGraph/EngineVersions.cs diff --git a/producers/src/OkfProducer.Cli/GenerateRun.cs b/producers/src/OkfProducer.Cli/GenerateRun.cs index 084c48ff..d1a0a2ac 100644 --- a/producers/src/OkfProducer.Cli/GenerateRun.cs +++ b/producers/src/OkfProducer.Cli/GenerateRun.cs @@ -238,6 +238,14 @@ public static WriteResult Execute(GenerateRequest request, ProducerServices serv : null, SourceOwnership = ownership, Note = note, + + // §6.2: named only when the code stage actually ran. Under --no-code no engine touched the + // repository, so claiming a tree-sitter and a Roslyn version would attach a determinism + // guarantee to an artefact none of them produced -- the same reason that path passes a null + // manifest rather than an empty one. + EngineVersions = request.NoCode + ? [] + : [TreeSitterExtractor.EngineVersion, RoslynResolver.EngineVersion], }; var concepts = services.Generator.Generate(snapshot, graph, options); diff --git a/producers/src/OkfProducer.CodeGraph.Roslyn/OkfProducer.CodeGraph.Roslyn.csproj b/producers/src/OkfProducer.CodeGraph.Roslyn/OkfProducer.CodeGraph.Roslyn.csproj index 221b8b75..f24e682b 100644 --- a/producers/src/OkfProducer.CodeGraph.Roslyn/OkfProducer.CodeGraph.Roslyn.csproj +++ b/producers/src/OkfProducer.CodeGraph.Roslyn/OkfProducer.CodeGraph.Roslyn.csproj @@ -4,6 +4,10 @@ net10.0 enable enable + + + true diff --git a/producers/src/OkfProducer.CodeGraph.Roslyn/RoslynResolver.cs b/producers/src/OkfProducer.CodeGraph.Roslyn/RoslynResolver.cs index 954bff6e..ffa741f0 100644 --- a/producers/src/OkfProducer.CodeGraph.Roslyn/RoslynResolver.cs +++ b/producers/src/OkfProducer.CodeGraph.Roslyn/RoslynResolver.cs @@ -147,6 +147,13 @@ private RoslynResolver( /// public IReadOnlyList Projects { get; } + /// + /// This engine's §6.2 token for overview's generated.by, read from the compiler + /// assembly this resolver actually loads rather than from a version string written by hand. + /// + public static string EngineVersion { get; } = + EngineVersions.Token("roslyn", typeof(CSharpCompilation).Assembly); + /// /// Compiles , plus every project they reference that lives under /// , and returns a resolver over whichever of them came out clean. diff --git a/producers/src/OkfProducer.CodeGraph.Roslyn/packages.lock.json b/producers/src/OkfProducer.CodeGraph.Roslyn/packages.lock.json new file mode 100644 index 00000000..f0e69f22 --- /dev/null +++ b/producers/src/OkfProducer.CodeGraph.Roslyn/packages.lock.json @@ -0,0 +1,39 @@ +{ + "version": 1, + "dependencies": { + "net10.0": { + "Microsoft.CodeAnalysis.CSharp": { + "type": "Direct", + "requested": "[5.3.0, )", + "resolved": "5.3.0", + "contentHash": "SQFNGQF4f7UfDXKxMzzGNMr3fjrPDIjLfmRvvVgDCw+dyvEHDaRfHuKA5q0Pr0/JW0Gcw89TxrxrS/MjwBvluQ==", + "dependencies": { + "Microsoft.CodeAnalysis.Analyzers": "5.3.0-2.25625.1", + "Microsoft.CodeAnalysis.Common": "[5.3.0]" + } + }, + "Microsoft.CodeAnalysis.Analyzers": { + "type": "Transitive", + "resolved": "5.3.0-2.25625.1", + "contentHash": "4Yhh2fnu3G+J0J1lDc8WZVgMjgbynSeTfkl5IFJMFrmiIO0sc7Tjx+f3sFVV8Sd35PrIUWfof0RWc3lAMl7Azg==" + }, + "Microsoft.CodeAnalysis.Common": { + "type": "Transitive", + "resolved": "5.3.0", + "contentHash": "uC0qk3jzTQY7i90ehfnCqaOZpBUGJyPMiHJ3c0jOb8yaPBjWzIhVdNxPbeVzI74DB0C+YgBKPLqUkgFZzua5Mg==", + "dependencies": { + "Microsoft.CodeAnalysis.Analyzers": "5.3.0-2.25625.1" + } + }, + "okf4net": { + "type": "Project" + }, + "okfproducer.core": { + "type": "Project", + "dependencies": { + "OKF4net": "[0.5.0, )" + } + } + } + } +} \ No newline at end of file diff --git a/producers/src/OkfProducer.CodeGraph.TreeSitter/OkfProducer.CodeGraph.TreeSitter.csproj b/producers/src/OkfProducer.CodeGraph.TreeSitter/OkfProducer.CodeGraph.TreeSitter.csproj index d8fba79d..a750aee0 100644 --- a/producers/src/OkfProducer.CodeGraph.TreeSitter/OkfProducer.CodeGraph.TreeSitter.csproj +++ b/producers/src/OkfProducer.CodeGraph.TreeSitter/OkfProducer.CodeGraph.TreeSitter.csproj @@ -4,6 +4,13 @@ net10.0 enable enable + + + true diff --git a/producers/src/OkfProducer.CodeGraph.TreeSitter/TreeSitterExtractor.cs b/producers/src/OkfProducer.CodeGraph.TreeSitter/TreeSitterExtractor.cs index ab229e94..bd28bd83 100644 --- a/producers/src/OkfProducer.CodeGraph.TreeSitter/TreeSitterExtractor.cs +++ b/producers/src/OkfProducer.CodeGraph.TreeSitter/TreeSitterExtractor.cs @@ -51,6 +51,12 @@ private static byte[] ReadFully(Stream stream, long expectedLength) /// public TreeSitterExtractor(IFileSystemReader? reader = null) => _reader = reader ?? SystemFileReader.Instance; + /// + /// This engine's §6.2 token for overview's generated.by, read from the binding + /// assembly this extractor actually loads rather than from a version string written by hand. + /// + public static string EngineVersion { get; } = Core.CodeGraph.EngineVersions.Token("tree-sitter", typeof(Parser).Assembly); + private const string CommentNodeType = "comment"; private const string FileScopedNamespaceNodeType = "file_scoped_namespace_declaration"; private const string NamespaceDeclarationNodeType = "namespace_declaration"; diff --git a/producers/src/OkfProducer.CodeGraph.TreeSitter/packages.lock.json b/producers/src/OkfProducer.CodeGraph.TreeSitter/packages.lock.json new file mode 100644 index 00000000..665578da --- /dev/null +++ b/producers/src/OkfProducer.CodeGraph.TreeSitter/packages.lock.json @@ -0,0 +1,22 @@ +{ + "version": 1, + "dependencies": { + "net10.0": { + "TreeSitter.DotNet": { + "type": "Direct", + "requested": "[1.3.0, )", + "resolved": "1.3.0", + "contentHash": "VFl5j5UguKI3BvdQ/bbHDrDP1FG7lkEjv9LZCQ01bPe4NuAiXQVnEfuFRWdJ8TA9NmY/D7qqGkj8CuD3ptiDDw==" + }, + "okf4net": { + "type": "Project" + }, + "okfproducer.core": { + "type": "Project", + "dependencies": { + "OKF4net": "[0.5.0, )" + } + } + } + } +} \ No newline at end of file diff --git a/producers/src/OkfProducer.Core/CodeGraph/EngineVersions.cs b/producers/src/OkfProducer.Core/CodeGraph/EngineVersions.cs new file mode 100644 index 00000000..6182a9b4 --- /dev/null +++ b/producers/src/OkfProducer.Core/CodeGraph/EngineVersions.cs @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: LGPL-3.0-or-later +using System.Reflection; + +namespace OkfProducer.Core.CodeGraph; + +/// +/// Builds the name/version tokens §6.2 puts in overview's generated.by. +/// +/// Here rather than in either engine project so the two cannot come to spell a version +/// differently, and so OkfProducer.Core -- which references neither engine -- can define the +/// shape without knowing who fills it. Each engine names ITSELF by handing over its own assembly; +/// the composition root collects the tokens and passes them to GenerateOptions. +/// +public static class EngineVersions +{ + /// + /// joined to 's version by /. + /// + /// The informational version is preferred because that is the NuGet package version a reader + /// can look up, where the assembly version is often frozen across a package's whole major line. + /// Anything after a + is dropped: SourceLink appends the commit sha there, which would put + /// a 40-character hash into every regenerated bundle and make the field churn on rebuilds that + /// changed no dependency. + /// + /// An assembly that declares neither falls back to unknown rather than being omitted: + /// a run whose engine version could not be read is a run whose determinism claim is weaker, and + /// saying so is the point of the field. + /// + public static string Token(string name, Assembly assembly) + { + var informational = assembly.GetCustomAttribute()?.InformationalVersion; + if (!string.IsNullOrWhiteSpace(informational)) + { + var plus = informational.IndexOf('+', StringComparison.Ordinal); + return $"{name}/{(plus < 0 ? informational : informational[..plus])}"; + } + + return $"{name}/{assembly.GetName().Version?.ToString(3) ?? "unknown"}"; + } +} diff --git a/producers/src/OkfProducer.Core/Generation/ConceptGenerator.cs b/producers/src/OkfProducer.Core/Generation/ConceptGenerator.cs index c7e5fca4..55d0ff57 100644 --- a/producers/src/OkfProducer.Core/Generation/ConceptGenerator.cs +++ b/producers/src/OkfProducer.Core/Generation/ConceptGenerator.cs @@ -151,7 +151,7 @@ public IReadOnlyList Generate(RepositorySnapshot snapshot, Cod var revision = GitRevision.HeadSha(snapshot.RepoPath); results.Add(new GeneratedConcept( overviewId, - BuildOverview(snapshot, overviewChildren, generatedAt, revision, options.ExistingFrontmatter?.Invoke(overviewId)))); + BuildOverview(snapshot, overviewChildren, generatedAt, revision, options.ExistingFrontmatter?.Invoke(overviewId), options.EngineVersions))); foreach (var (id, manifest) in packages) { @@ -249,7 +249,8 @@ private static OkfDocument BuildOverview( IReadOnlyList children, string generatedAt, string? revision, - Frontmatter? existing) + Frontmatter? existing, + IReadOnlyList engineVersions) { var derived = snapshot.Packages.Count switch { @@ -279,8 +280,14 @@ private static OkfDocument BuildOverview( // generated in one pass, and a per-concept timestamp would store the same fact hundreds of // times over, rewriting every file's `generated.at` on every regeneration regardless of what in // the code actually changed. + // §6.2: the extraction engines are named here and nowhere else in the bundle. Determinism holds + // at a FIXED extractor version, not absolutely -- a grammar or Roslyn bump can move symbols, + // spans and descriptions over unchanged source -- so a golden that moved is uninterpretable + // unless the artefact says which engines produced it. A run given none (every fixture, and + // --no-code) emits the producer token alone. var generated = new YamlMapping(); - generated.Insert("by", new YamlString(ProducerActor)); + generated.Insert("by", new YamlString( + engineVersions.Count == 0 ? ProducerActor : $"{ProducerActor} {string.Join(' ', engineVersions)}")); generated.Insert("at", new YamlString(generatedAt)); builder = builder.Extension("generated", generated); diff --git a/producers/src/OkfProducer.Core/Generation/GenerateOptions.cs b/producers/src/OkfProducer.Core/Generation/GenerateOptions.cs index de3ba56f..462381dd 100644 --- a/producers/src/OkfProducer.Core/Generation/GenerateOptions.cs +++ b/producers/src/OkfProducer.Core/Generation/GenerateOptions.cs @@ -103,6 +103,28 @@ public sealed record GenerateOptions /// public Action? Note { get; init; } + /// + /// The extraction engines this run used, each already spelled name/version, appended to + /// overview's generated.by after the producer's own token -- §6.2's + /// okfgen/0.1.0 tree-sitter/x.y.z roslyn/a.b.c. + /// + /// Why the artefact carries them. §6.2's determinism holds at a fixed extractor + /// version, not absolutely: a tree-sitter grammar or Roslyn bump can change symbols, spans or + /// descriptions over unchanged source. Locking the versions (packages.lock.json) makes the + /// bump deliberate; recording them makes a diff INTERPRETABLE, which is the half a lock file + /// cannot supply. Without them, a golden that moved says only that something changed. + /// + /// Supplied by the composition root, not read here. OkfProducer.Core references + /// neither engine, and should not: each engine names its own version, and the CLI -- the one + /// project that references everything -- passes them through. A run with none (every test fixture, + /// and --no-code) emits the producer token alone, exactly as before this existed. + /// + /// On overview alone, like at and for the same reason (§6.1): the + /// engines are a fact about the run, and repeating them on every one of hundreds of code/ + /// concepts would rewrite every file on a version bump that changed nothing else. + /// + public IReadOnlyList EngineVersions { get; init; } = []; + /// /// Whether is something §4.3's resource permalink can actually /// be built from -- present, absolute, and http or https -- handing back the parsed diff --git a/producers/tests/OkfProducer.Tests/Generation/CodeConceptGeneratorTests.cs b/producers/tests/OkfProducer.Tests/Generation/CodeConceptGeneratorTests.cs index cfbfb4d2..b5f7dace 100644 --- a/producers/tests/OkfProducer.Tests/Generation/CodeConceptGeneratorTests.cs +++ b/producers/tests/OkfProducer.Tests/Generation/CodeConceptGeneratorTests.cs @@ -890,6 +890,46 @@ public void Without_a_code_graph_the_output_is_unchanged() Assert.Equal(["overview"], ids); } + [Fact] + public void The_engine_versions_reach_overviews_generated_by() + { + // Spec 6.2: determinism holds at a FIXED extractor version, not absolutely, so the artefact has + // to say which engines produced it -- a golden that moved is otherwise uninterpretable, and a + // lock file pins the versions without recording them. The shape is the spec's own: + // `okfgen/0.1.0 tree-sitter/x.y.z roslyn/a.b.c`. + var options = Options() with { EngineVersions = ["tree-sitter/1.3.0", "roslyn/5.3.0"] }; + + var by = Single(new ConceptGenerator().Generate(Snapshot(), GraphOf(), options), "overview") + .Document.Frontmatter.Get("generated")?.AsMapping()?.Get("by")?.AsDisplayString(); + + Assert.Equal($"{ConceptGenerator.ProducerActor} tree-sitter/1.3.0 roslyn/5.3.0", by); + } + + [Fact] + public void With_no_engine_versions_overview_carries_the_producer_token_alone() + { + // The --no-code path, and every caller that supplies none. Emitting a trailing space, or engine + // tokens for engines that never ran, would attach a determinism claim to an artefact no engine + // produced. + var by = Single(Generate(), "overview") + .Document.Frontmatter.Get("generated")?.AsMapping()?.Get("by")?.AsDisplayString(); + + Assert.Equal(ConceptGenerator.ProducerActor, by); + } + + [Fact] + public void A_code_concept_does_not_repeat_the_engine_versions() + { + // On `overview` alone, like `at` and for the same reason (6.1): the engines are a fact about the + // RUN, and repeating them on hundreds of `code/` concepts would rewrite every file on a version + // bump that changed nothing else in them. + var options = Options() with { EngineVersions = ["tree-sitter/1.3.0", "roslyn/5.3.0"] }; + var by = Single(new ConceptGenerator().Generate(Snapshot(), Graph(), options), "code/csharp/n/scanner/scan") + .Document.Frontmatter.Get("generated")?.AsMapping()?.Get("by")?.AsDisplayString(); + + Assert.Equal(ConceptGenerator.ProducerActor, by); + } + // -- fixture ---------------------------------------------------------------------------------- private static IReadOnlyList Generate( From fb7a49e0554a6815a4c2bae60144b3d4384f3a71 Mon Sep 17 00:00:00 2001 From: Julien CHABLE Date: Sun, 6 Sep 2026 09:48:08 +0200 Subject: [PATCH 08/36] test(producer): give the golden a fixture that can fail, and fix the two defects it found F-2, F-4, F-1 and D4b-1's last surface. The golden was captured by a reduced pipeline over a fixture holding three classes and their methods: no interface, struct, record or enum, no constructor, event, field or local function, no nested or generic type, no explicit interface implementation, no block-scoped namespace, no non-public visibility, and no type whose header spans more than one line. Deleting the whole escaping layer would not have moved one golden byte. **F-2, the pipeline half.** `ProducerFixture` never set `SourceOwnership`, so the package -> namespace link was structurally absent and the whole `code/` family was an orphan in the captured bundle -- 0 of 12 concepts reachable from `overview`, measured. The map is now supplied directly rather than through MSBuild, and the reason is recorded where the code is: the fixture repository is never restored (committing an `obj/` would be machine-specific), so `dotnet msbuild` fails on it with NETSDK1004 and Roslyn compiles nothing. Composing the real query would add a network- and SDK-dependent restore to the one test that is fast and offline, and still capture zero `Exact` edges unless it succeeded. Arbitrated with the user. `Exact` edges therefore stay out of the golden, deliberately and in writing; `RoslynResolverTests` covers that engine against repositories it restores itself. **F-4.** No fixture type had a multi-line header, so R48's header cap -- the churn bound the whole id scheme rests on -- would have produced a byte-identical golden if it returned `StartLine + 1`. `Boxed`'s header spans three lines now, and that substitution fails the golden. **F-1.** `fixtures/README.md` said the regeneration command "asserts against what it just wrote". It does the opposite: update mode refuses to assert, because the expected side is produced by the same harness as the actual side, and a tautology reported green is how a stale variable disarms a golden. The README now says the command exits RED by design and that the procedure is two runs. **D4b-1's fourth surface.** The dirty-tree caveat now reaches the `overview` concept, where a reader of the bundle is. Pinned in `DeterminismTests` rather than the golden, and that is not a workaround: the golden is captured outside git, so it has no `revision` and correctly carries no caveat. Both directions are asserted. ## The two defects the enriched fixture found immediately Both are mine, from the batch that separated generic arity and explicit interface implementations, and neither had a test. **Members of a generic type all landed under the first arity.** `Holder`, `Holder` and `Holder` became three type concepts while `Count`, `Value` and `Find` all registered under `holder`, because a member's container path read the ancestor's bare `name` field. Ancestors now go through the same qualification the declaration itself gets. **That fix then desynchronised the two engines**, which is worse than the bug it fixed: `RoslynResolver.ContainerPathOf` exists precisely to spell a container the way the extractor does (C1-1), and it still used the bare identifier token. A spelling only one side knows makes `CodeGraphBuilder` overwrite the baseline with a non-joining `Exact` and then degrade it -- the graph left strictly worse than not running the resolver at all. `DeclaredName` is now the single place that rule lives on the Roslyn side, used for both a leaf name and every ancestor segment. The existing explicit-implementation test caught the drift and is what pins the agreement. ## One register claim measured false D1b-I2 said a public `Bar()` and an explicit `IFoo.Bar()` "collapse into one concept, one description, both signatures". Not reachable: an explicit interface implementation carries no access modifier, so it is Private and out of scope under every flag, filtered before `ConceptGenerator` groups anything. The golden confirms it -- the fixture's `IEquatable.Equals` produces no concept -- and the inventory test now asserts that absence. The separation is still right one layer down, where two different members shared one name. 626 tests green, format clean. The golden grew from 15 concepts to 37, absorbing the id changes and the `generated.by` engine tokens in a single reviewed regeneration. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0166CcRSdoZrL8g1Suruy6Uz --- .../RoslynResolver.cs | 99 +++++++++---- .../TreeSitterExtractor.cs | 10 +- .../Generation/ConceptGenerator.cs | 13 ++ .../CodeGraph/RoslynResolverTests.cs | 12 +- .../CodeGraph/TreeSitterExtractorTests.cs | 21 ++- .../Generation/CheckTests.cs | 46 +++++- .../Generation/DeterminismTests.cs | 22 +++ .../Generation/ProducerFixture.cs | 29 ++++ .../OkfProducer.Tests/fixtures/README.md | 13 +- .../fixtures/fixture-repo/src/Shapes.cs | 118 +++++++++++++++ .../fixtures/golden/.okfgen-manifest.json | 134 ++++++++++++++++++ .../fixtures/golden/code/csharp/index.md | 2 +- .../fixtures/golden/code/csharp/n.md | 1 + .../fixtures/golden/code/csharp/n/index.md | 2 + .../fixtures/golden/code/csharp/n/shapes.md | 27 ++++ .../golden/code/csharp/n/shapes/boxed.md | 28 ++++ .../code/csharp/n/shapes/boxed/boxed.md | 21 +++ .../code/csharp/n/shapes/boxed/builder.md | 25 ++++ .../csharp/n/shapes/boxed/builder/build.md | 21 +++ .../csharp/n/shapes/boxed/builder/index.md | 3 + .../code/csharp/n/shapes/boxed/changed.md | 21 +++ .../code/csharp/n/shapes/boxed/index.md | 13 ++ .../code/csharp/n/shapes/boxed/render.md | 25 ++++ .../golden/code/csharp/n/shapes/corner.md | 21 +++ .../golden/code/csharp/n/shapes/hidden.md | 19 +++ .../code/csharp/n/shapes/hidden/index.md | 3 + .../code/csharp/n/shapes/hidden/never.md | 21 +++ .../golden/code/csharp/n/shapes/holder.md | 25 ++++ .../code/csharp/n/shapes/holder/count.md | 21 +++ .../code/csharp/n/shapes/holder/index.md | 3 + .../golden/code/csharp/n/shapes/holder_1.md | 25 ++++ .../code/csharp/n/shapes/holder_1/index.md | 3 + .../code/csharp/n/shapes/holder_1/value.md | 21 +++ .../golden/code/csharp/n/shapes/holder_2.md | 25 ++++ .../code/csharp/n/shapes/holder_2/find.md | 21 +++ .../code/csharp/n/shapes/holder_2/index.md | 3 + .../golden/code/csharp/n/shapes/i-shape.md | 25 ++++ .../code/csharp/n/shapes/i-shape/index.md | 3 + .../code/csharp/n/shapes/i-shape/render.md | 21 +++ .../golden/code/csharp/n/shapes/index.md | 24 ++++ .../golden/code/csharp/n/shapes/point.md | 26 ++++ .../code/csharp/n/shapes/point/index.md | 4 + .../golden/code/csharp/n/shapes/point/x.md | 21 +++ .../golden/code/csharp/n/shapes/point/y.md | 21 +++ .../golden/code/csharp/n/shapes/size.md | 21 +++ .../fixtures/golden/overview.md | 4 +- .../fixtures/golden/packages/fixture.md | 4 + 47 files changed, 1038 insertions(+), 53 deletions(-) create mode 100644 producers/tests/OkfProducer.Tests/fixtures/fixture-repo/src/Shapes.cs create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/boxed.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/builder.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/builder/build.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/builder/index.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/changed.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/index.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/render.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/corner.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/hidden.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/hidden/index.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/hidden/never.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder/count.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder/index.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_1.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_1/index.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_1/value.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_2.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_2/find.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_2/index.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/i-shape.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/i-shape/index.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/i-shape/render.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/index.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/point.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/point/index.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/point/x.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/point/y.md create mode 100644 producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/size.md diff --git a/producers/src/OkfProducer.CodeGraph.Roslyn/RoslynResolver.cs b/producers/src/OkfProducer.CodeGraph.Roslyn/RoslynResolver.cs index ffa741f0..da7a4c8f 100644 --- a/producers/src/OkfProducer.CodeGraph.Roslyn/RoslynResolver.cs +++ b/producers/src/OkfProducer.CodeGraph.Roslyn/RoslynResolver.cs @@ -1,4 +1,5 @@ // SPDX-License-Identifier: LGPL-3.0-or-later +using System.Globalization; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -754,20 +755,12 @@ private static string ContainerPathFromSyntax(SyntaxNode declaration) for (var current = declaration.Parent; current is not null; current = current.Parent) { - var segment = current switch - { - BaseNamespaceDeclarationSyntax ns => ns.Name.ToString(), - BaseTypeDeclarationSyntax type => type.Identifier.Text, - DelegateDeclarationSyntax dele => dele.Identifier.Text, - MethodDeclarationSyntax method => method.Identifier.Text, - ConstructorDeclarationSyntax constructor => constructor.Identifier.Text, - DestructorDeclarationSyntax destructor => destructor.Identifier.Text, - PropertyDeclarationSyntax property => property.Identifier.Text, - EventDeclarationSyntax @event => @event.Identifier.Text, - LocalFunctionStatementSyntax local => local.Identifier.Text, - VariableDeclaratorSyntax declarator => declarator.Identifier.Text, - _ => string.Empty, - }; + // Through DeclaredName for everything but a namespace, so an ancestor is spelled here + // exactly as that ancestor's own concept is. Spelling it with the bare identifier token + // instead was measured to put every member of `Holder` under `Holder`'s container. + var segment = current is BaseNamespaceDeclarationSyntax ns + ? ns.Name.ToString() + : DeclaredName(current); if (segment.Length > 0) { @@ -830,6 +823,66 @@ private static string ContainerPathFromSymbols(ISymbol symbol) /// symbol with no source declaration to read. /// /// + /// + /// The name TreeSitterExtractor gives this declaration -- the ONE place the two engines' + /// spelling rule lives on this side, used both for a leaf name and for every ancestor segment of a + /// container path. + /// + /// Why it is not just the identifier token. Two shapes need more than the token, and + /// both were measured to drift when only one side knew: a generic TYPE carries its arity + /// (Holder, Holder_1, Holder_2, since the grammar's name field is + /// Holder for all three), and a member implementing an interface explicitly carries the + /// interface as a dotted prefix (IShape.Draw), because it is a different member from a + /// public Draw and not callable as one. + /// + /// Arity is a TYPE rule only: a call site captures its callee as the bare identifier, so + /// qualifying a generic METHOD would stop every call to it from joining. Delegates take no arity + /// either, because the extractor classes them as members. + /// + /// Getting this wrong is not a missed link but a WRONG one, which is the outcome §2.1's + /// chaining exists to make impossible: CodeGraphBuilder overwrites the baseline's verdict + /// with the exact one and only then degrades a non-joining Exact, so a spelling only this + /// side knows leaves the graph strictly worse than not running the resolver at all. + /// + private static string DeclaredName(SyntaxNode declaration) + { + var identifier = declaration switch + { + BaseTypeDeclarationSyntax type => type.Identifier, + DelegateDeclarationSyntax dele => dele.Identifier, + MethodDeclarationSyntax method => method.Identifier, + ConstructorDeclarationSyntax constructor => constructor.Identifier, + DestructorDeclarationSyntax destructor => destructor.Identifier, + PropertyDeclarationSyntax property => property.Identifier, + EventDeclarationSyntax @event => @event.Identifier, + LocalFunctionStatementSyntax local => local.Identifier, + VariableDeclaratorSyntax declarator => declarator.Identifier, + _ => default, + }; + + if (identifier.Text.Length == 0) + { + return string.Empty; + } + + if (declaration is TypeDeclarationSyntax { TypeParameterList.Parameters.Count: > 0 } generic) + { + return $"{identifier.Text}_{generic.TypeParameterList!.Parameters.Count.ToString(CultureInfo.InvariantCulture)}"; + } + + var explicitInterface = declaration switch + { + MethodDeclarationSyntax method => method.ExplicitInterfaceSpecifier, + PropertyDeclarationSyntax property => property.ExplicitInterfaceSpecifier, + EventDeclarationSyntax @event => @event.ExplicitInterfaceSpecifier, + _ => null, + }; + + return explicitInterface is null + ? identifier.Text + : $"{explicitInterface.Name}.{identifier.Text}"; + } + private static string SimpleNameOf(ISymbol symbol) { foreach (var reference in symbol.DeclaringSyntaxReferences) @@ -837,23 +890,9 @@ private static string SimpleNameOf(ISymbol symbol) // The node kinds here mirror CSharpProfile.DeclarationQuery's, deliberately: a declaration // this producer does not extract has no SymbolFact to join anyway, so there is nothing to // match its spelling against. - var identifier = reference.GetSyntax() switch - { - BaseTypeDeclarationSyntax type => type.Identifier, - DelegateDeclarationSyntax dele => dele.Identifier, - MethodDeclarationSyntax method => method.Identifier, - ConstructorDeclarationSyntax constructor => constructor.Identifier, - DestructorDeclarationSyntax destructor => destructor.Identifier, - PropertyDeclarationSyntax property => property.Identifier, - EventDeclarationSyntax @event => @event.Identifier, - LocalFunctionStatementSyntax local => local.Identifier, - VariableDeclaratorSyntax declarator => declarator.Identifier, - _ => default, - }; - - if (identifier.Text.Length > 0) + if (DeclaredName(reference.GetSyntax()) is { Length: > 0 } declared) { - return identifier.Text; + return declared; } } diff --git a/producers/src/OkfProducer.CodeGraph.TreeSitter/TreeSitterExtractor.cs b/producers/src/OkfProducer.CodeGraph.TreeSitter/TreeSitterExtractor.cs index bd28bd83..b5dcce15 100644 --- a/producers/src/OkfProducer.CodeGraph.TreeSitter/TreeSitterExtractor.cs +++ b/producers/src/OkfProducer.CodeGraph.TreeSitter/TreeSitterExtractor.cs @@ -952,7 +952,15 @@ private static string ComputeContainerPath(Node decl, string? fileScopedNamespac var nameField = current.Type == FileScopedNamespaceNodeType ? null : current.GetChildForField(NameFieldName); if (nameField is not null) { - segments.Insert(0, nameField.Text); + // Through the SAME qualification the declaration itself gets, or the container path + // would spell an ancestor differently from that ancestor's own concept. Measured on + // the enriched golden fixture before this line existed: `Holder`, `Holder` and + // `Holder` became three type concepts, and all of their members landed under the + // first one, because a member's container read the ancestor's bare `name` field. + segments.Insert(0, QualifyName( + nameField.Text, + current, + IsTypeDeclaration(current.Type) ? SymbolKind.Type : SymbolKind.Member)); } current = current.Parent; diff --git a/producers/src/OkfProducer.Core/Generation/ConceptGenerator.cs b/producers/src/OkfProducer.Core/Generation/ConceptGenerator.cs index 55d0ff57..1b055477 100644 --- a/producers/src/OkfProducer.Core/Generation/ConceptGenerator.cs +++ b/producers/src/OkfProducer.Core/Generation/ConceptGenerator.cs @@ -264,6 +264,19 @@ private static OkfDocument BuildOverview( var body = new StringBuilder(); body.Append("# ").Append(LiftedBodyText(snapshot.RepoName)).Append("\n\n") .Append(LiftedBodyParagraph(description, preserved)).Append('\n'); + + // The dirty-tree caveat, where a READER of the bundle is rather than only in a code comment. + // `revision` names the committed HEAD and never the working tree, so with uncommitted edits at + // generation time it names a commit this bundle was not built from -- and `--check` cannot see + // the difference either, since both sides carry the same revision. Emitted only when there is a + // revision to qualify: outside a git repository the field is absent and there is nothing to + // warn about. + if (revision is { Length: > 0 }) + { + body.Append("\nThe `revision` above names the committed HEAD, not the working tree: if the repository had " + + "uncommitted changes when this bundle was generated, that commit is not what it was generated from.\n"); + } + AppendContains(body, children); var builder = OkfDocumentBuilder diff --git a/producers/tests/OkfProducer.Tests/CodeGraph/RoslynResolverTests.cs b/producers/tests/OkfProducer.Tests/CodeGraph/RoslynResolverTests.cs index 27a2fa40..2ca4dc37 100644 --- a/producers/tests/OkfProducer.Tests/CodeGraph/RoslynResolverTests.cs +++ b/producers/tests/OkfProducer.Tests/CodeGraph/RoslynResolverTests.cs @@ -473,7 +473,7 @@ public class @class // The other half of the same defect, and it needs no verbatim identifier at all: Roslyn mangles // an explicit interface implementation's name to its qualified form (Explicitly.IShape.Draw), so // a local function declared inside one reports a container carrying two extra dots -- and - // therefore two extra SEGMENTS -- that the extractor's `Explicitly.Square.Draw` does not have. + // therefore two extra SEGMENTS -- that the extractor's `Explicitly.Square.IShape.Draw` does not have. public const string ExplicitImplementationSource = """ namespace Explicitly; public interface IShape { void Draw(); } @@ -515,16 +515,22 @@ public void A_verbatim_identifier_keeps_its_at_sign_in_the_container_as_well_as_ [Fact] public void An_explicit_interface_implementation_contributes_the_segment_source_spells() { + // The segment reads `IShape.Draw` rather than `Draw` since an explicitly implemented member is + // named apart from a public one of the same name (D1b-I2). What this test pins is unchanged and + // is the point: BOTH sides say the same thing. It is what caught the drift when only the + // extractor learnt the rule -- an ancestor spelled one way here and another way there makes + // `CodeGraphBuilder` overwrite the baseline with a non-joining `Exact` and then degrade it, + // leaving the graph strictly worse than not running the resolver at all. var site = Assert.Single(_scratch.SitesIn("ExplicitImplementation.cs"), s => s.CalledName == "Nested"); var baseline = Assert.Single(new NameMatchResolver().Resolve([site], _scratch.Symbols)); Assert.Equal(EdgeConfidence.ByName, baseline.Confidence); - Assert.Equal("Explicitly.Square.Draw", baseline.TargetContainer); + Assert.Equal("Explicitly.Square.IShape.Draw", baseline.TargetContainer); var edge = Assert.Single(_scratch.Resolver.Resolve([site], _scratch.Symbols)); Assert.Equal(EdgeConfidence.Exact, edge.Confidence); - Assert.Equal("Explicitly.Square.Draw", edge.TargetContainer); + Assert.Equal("Explicitly.Square.IShape.Draw", edge.TargetContainer); Assert.Contains(_scratch.Symbols, s => s.Container == edge.TargetContainer && s.Name == edge.TargetName); } diff --git a/producers/tests/OkfProducer.Tests/CodeGraph/TreeSitterExtractorTests.cs b/producers/tests/OkfProducer.Tests/CodeGraph/TreeSitterExtractorTests.cs index 2c7789d6..3ffc70c1 100644 --- a/producers/tests/OkfProducer.Tests/CodeGraph/TreeSitterExtractorTests.cs +++ b/producers/tests/OkfProducer.Tests/CodeGraph/TreeSitterExtractorTests.cs @@ -529,14 +529,21 @@ public class T { public void Bar() { } } [Fact] public void An_explicit_interface_implementation_is_named_apart_from_the_public_member() { - // Both report `name` as `Bar`, so the two collapsed into one concept carrying ONE description - // -- the first declaration's -- and both signatures, for two deliberately different - // implementations. The qualified form takes the interface as a dotted prefix, which is how C# - // writes it. + // Both report `name` as `Bar`, so at THIS layer the two were one symbol. The qualified form + // takes the interface as a dotted prefix, which is how C# writes it. // - // Name matching no longer reaches the explicit member, and that is the correct outcome rather - // than a cost: it is not callable as `Bar()` on the type, so a call that used to bind to it was - // binding to the wrong member. + // What that does NOT do, measured rather than assumed after the register claimed otherwise: + // it does not split a merged CONCEPT, because there was never a merged concept to split. An + // explicit interface implementation carries no access modifier, so `VisibilityOf` classes it + // Private, and Private is out of scope under every flag -- `--include-internal` included. It is + // filtered before ConceptGenerator ever groups anything. The register's D1b-I2 said the two + // collapsed into "one concept, one description, both signatures"; that outcome is not + // reachable through the shipped pipeline, and the golden confirms it -- the fixture's explicit + // `IEquatable.Equals` produces no concept at all. + // + // The fix is still right, one layer down: two different members sharing one name is wrong for + // any consumer reading SymbolFacts before the scope filter, and name matching would otherwise + // bind a call to `Bar()` -- which cannot reach the explicit member on the type -- to it. var result = ExtractSource(""" namespace N; public interface IFoo { void Bar(); } diff --git a/producers/tests/OkfProducer.Tests/Generation/CheckTests.cs b/producers/tests/OkfProducer.Tests/Generation/CheckTests.cs index 576ffdc9..f719edcd 100644 --- a/producers/tests/OkfProducer.Tests/Generation/CheckTests.cs +++ b/producers/tests/OkfProducer.Tests/Generation/CheckTests.cs @@ -498,7 +498,7 @@ public void A_real_regeneration_reports_how_many_concepts_it_wrote() // The_golden_bundle_holds_one_occurrence_of_each_shape. using var workspace = ProducerFixture.CopyRepoOutsideGit(); - Assert.Equal(15, RunCheck(workspace, ProducerFixture.GoldenBundle).ConceptsRegenerated); + Assert.Equal(37, RunCheck(workspace, ProducerFixture.GoldenBundle).ConceptsRegenerated); } [Fact] @@ -544,9 +544,11 @@ public void The_golden_bundle_validates_with_no_error_and_only_the_warnings_we_k // the kinds we have accepted, so a NEW kind cannot hide inside an unchanged total. One kind is // left, a §4.3 consequence: // - // * three "missing recommended frontmatter field `resource`" -- `overview` and the two - // container concepts. A container is not declared in one file, so there is no line span to - // build a permalink from, and §4.3 admits only a URL there. + // * five "missing recommended frontmatter field `resource`" -- `overview` and the four + // container concepts (`n`, `n/sub`, `n/shapes`, and `n/shapes/hidden`, the last being the + // internal type whose own symbol is out of scope while its public member is not). A + // container is not declared in one file, so there is no line span to build a permalink + // from, and §4.3 admits only a URL there. // // The four "frontmatter path ... not found" this fixture used to carry are gone. `packages/*` // and `docs/*` wrote a repo-relative `resource` AND repeated it in a one-entry `sources` block, @@ -562,7 +564,7 @@ public void The_golden_bundle_validates_with_no_error_and_only_the_warnings_we_k Assert.True(outcome.IsConformant, string.Join("\n", outcome.DiagnosticLines)); Assert.DoesNotContain(outcome.DiagnosticLines, line => line.Contains("BrokenLink", StringComparison.Ordinal)); - Assert.Equal(3, outcome.WarningCount); + Assert.Equal(5, outcome.WarningCount); Assert.All( outcome.DiagnosticLines.Where(line => line.StartsWith("[warning]", StringComparison.Ordinal)), line => Assert.True( @@ -588,7 +590,7 @@ public void The_golden_bundle_holds_one_occurrence_of_each_shape() .OrderBy(id => id, StringComparer.Ordinal) .ToList(); - Assert.InRange(concepts.Count, 15, 20); + Assert.InRange(concepts.Count, 35, 40); Assert.Equal( [ @@ -601,6 +603,28 @@ public void The_golden_bundle_holds_one_occurrence_of_each_shape() "code/csharp/n/scanner/normalize", // the one resolved call target "code/csharp/n/scanner/root", "code/csharp/n/scanner/scan", + "code/csharp/n/shapes", // a BLOCK-scoped namespace, where every other is file-scoped + "code/csharp/n/shapes/boxed", // the one type whose header spans three lines + "code/csharp/n/shapes/boxed/boxed", // a constructor + "code/csharp/n/shapes/boxed/builder", // a type nested in a type + "code/csharp/n/shapes/boxed/builder/build", + "code/csharp/n/shapes/boxed/changed", // an event + "code/csharp/n/shapes/boxed/render", // whose doc comment carries [brackets], a backtick and a tag + "code/csharp/n/shapes/corner", // an enum, whose members are deliberately not concepts + "code/csharp/n/shapes/hidden", // an INTERNAL type: out of scope, so only a container survives + "code/csharp/n/shapes/hidden/never", // its public member, which scope does not filter + "code/csharp/n/shapes/holder", // three types differing only by arity (D1b-I1) -- + "code/csharp/n/shapes/holder/count", // and each one's members under IT, not under the first + "code/csharp/n/shapes/holder_1", + "code/csharp/n/shapes/holder_1/value", + "code/csharp/n/shapes/holder_2", + "code/csharp/n/shapes/holder_2/find", + "code/csharp/n/shapes/i-shape", // an interface + "code/csharp/n/shapes/i-shape/render", + "code/csharp/n/shapes/point", // a struct + "code/csharp/n/shapes/point/x", // two declarators on one field, which nothing else reaches + "code/csharp/n/shapes/point/y", + "code/csharp/n/shapes/size", // a positional record, with no body at all "code/csharp/n/sub", // a nested container "code/csharp/n/sub/formatter", "code/csharp/n/sub/formatter/format", @@ -613,6 +637,16 @@ public void The_golden_bundle_holds_one_occurrence_of_each_shape() // The private member `Scanner.Cache` is in the fixture source and must NOT be here (§5.4). Assert.DoesNotContain("code/csharp/n/scanner/cache", concepts); + // Nor is `Boxed`'s explicit `IEquatable.Equals`, and for a reason worth pinning: an + // explicit interface implementation carries no access modifier, so it is Private and out of + // scope under every flag. The register's D1b-I2 claimed the two collapsed into one concept + // with both signatures; measured here, the explicit one produces no concept at all. + Assert.DoesNotContain(concepts, id => id.Contains("equals", StringComparison.Ordinal)); + + // The local function inside `Render` is Private too, so it is not a concept either. + Assert.DoesNotContain(concepts, id => id.Contains("compose", StringComparison.Ordinal)); + Assert.DoesNotContain("code/csharp/n/scanner/cache", concepts); + var register = File.ReadAllText(Path.Combine(ProducerFixture.GoldenBundle, "code/csharp/n/registry/register.md")); var count = File.ReadAllText(Path.Combine(ProducerFixture.GoldenBundle, "code/csharp/n/registry/count.md")); diff --git a/producers/tests/OkfProducer.Tests/Generation/DeterminismTests.cs b/producers/tests/OkfProducer.Tests/Generation/DeterminismTests.cs index 029d132e..500846ee 100644 --- a/producers/tests/OkfProducer.Tests/Generation/DeterminismTests.cs +++ b/producers/tests/OkfProducer.Tests/Generation/DeterminismTests.cs @@ -133,6 +133,20 @@ public void Only_overview_carries_at_and_revision() // `by` alone survives on a code concept -- but not on every family: packages/* and docs/* // carry no `generated` block at all, only code and container concepts do. Assert.NotNull(Single(concepts, "code/csharp/n/scanner/scan").Document.Frontmatter.Get("generated")?.AsMapping()?.Get("by")); + + // And the caveat that qualifies `revision` travels with it, in the body where a reader of the + // bundle is. It said this only in a code comment on GitRevision until now: `revision` names the + // committed HEAD, never the working tree, so with uncommitted edits it names a commit the + // bundle was not generated from -- and `--check` cannot see the difference either, both sides + // carrying the same revision. + // + // Asserted HERE rather than against the golden on purpose: the golden is captured outside git, + // so it has no `revision` and correctly carries no caveat. This is the only fixture in the + // solution where the sentence can appear at all, which is also why its absence went unnoticed. + Assert.Contains( + "names the committed HEAD, not the working tree", + Single(concepts, "overview").Document.Body, + StringComparison.Ordinal); } [Fact] @@ -171,6 +185,14 @@ public void Outside_a_git_repository_the_wall_clock_stands_in_and_no_revision_is Assert.NotNull(overview.Document.Frontmatter.GeneratedAt); Assert.EndsWith("Z", overview.Document.Frontmatter.GeneratedAt, StringComparison.Ordinal); Assert.Null(overview.Document.Frontmatter.Get("revision")); + + // And no caveat about a field that is not there. The sentence qualifies `revision`; emitting it + // unconditionally would have every bundle generated outside git warn about a value it does not + // carry, which is how a warning stops being read. + Assert.DoesNotContain( + "names the committed HEAD, not the working tree", + overview.Document.Body, + StringComparison.Ordinal); } // -- fixture ---------------------------------------------------------------------------------- diff --git a/producers/tests/OkfProducer.Tests/Generation/ProducerFixture.cs b/producers/tests/OkfProducer.Tests/Generation/ProducerFixture.cs index 13c0d38e..d37d4036 100644 --- a/producers/tests/OkfProducer.Tests/Generation/ProducerFixture.cs +++ b/producers/tests/OkfProducer.Tests/Generation/ProducerFixture.cs @@ -70,11 +70,40 @@ public static RunOutcome Run(string repoPath, string outPath) var graph = new CodeGraphBuilder(extractor, [CSharpProfile.Instance], [new NameMatchResolver()]) .Build(snapshot, ExtractionLimits.Default, ScopeOptions.Default); + // Supplied by hand rather than by MSBuild, and that is a deliberate, recorded limit on what the + // golden covers. The fixture repository is never restored -- committing an `obj/` would be + // machine-specific -- so `dotnet msbuild` fails on it (NETSDK1004: no project.assets.json) and + // Roslyn compiles nothing. Composing the real query would add a network- and SDK-dependent + // restore to the one test that is fast and offline, and would still capture zero `Exact` edges + // unless that restore succeeded. + // + // What supplying the map directly RECOVERS, and what the golden had been captured without: the + // package -> namespace containment link, whose absence made the whole `code/` family an orphan + // in the captured bundle -- 0 of 12 concepts reachable from `overview`, measured -- plus + // `## Also compiled by` and `## Target frameworks`. + // + // What stays absent, on purpose: every `Exact` edge. `RoslynResolverTests` exercises that + // engine directly, against repositories it restores itself. + var sources = Directory + .EnumerateFiles(Path.Combine(repoPath, "src"), "*.cs", SearchOption.AllDirectories) + .OrderBy(path => path, StringComparer.Ordinal) + .ToList(); + var ownership = SourceOwnershipMap.From( + repoPath, + [new ProjectCompileItems(Path.Combine(repoPath, "src", "Fixture.csproj"), "net10.0", sources)]); + var options = new GenerateOptions { RepoUrl = RepoUrl, Rev = Rev, Profiles = [CSharpProfile.Instance], + SourceOwnership = ownership, + + // Tree-sitter alone, because tree-sitter alone ran: naming a Roslyn version here would + // attach a determinism claim to an engine this capture never invoked. Bumping the + // TreeSitter.DotNet package therefore rewrites `overview`, which is exactly the reviewed + // migration §6.2 asks for rather than a drift. + EngineVersions = [TreeSitterExtractor.EngineVersion], // The line that makes a manual description survive a regeneration -- and therefore the // line without which --check would report every hand-edited concept as drift for ever. diff --git a/producers/tests/OkfProducer.Tests/fixtures/README.md b/producers/tests/OkfProducer.Tests/fixtures/README.md index 202f8d21..306986a7 100644 --- a/producers/tests/OkfProducer.Tests/fixtures/README.md +++ b/producers/tests/OkfProducer.Tests/fixtures/README.md @@ -28,9 +28,16 @@ an unintended one loud, so read the diff before you accept it. OKFGEN_UPDATE_GOLDEN=1 dotnet test producers/OkfProducer.sln --filter "FullyQualifiedName~CheckTests.Check_passes_on_an_unchanged_bundle" ``` -That rewrites `golden/` from scratch (it is machine output, so it is captured, never merged into) and -then asserts against what it just wrote. Review `git diff producers/tests/OkfProducer.Tests/fixtures/golden` -and commit it with the change that caused it. +**That command exits RED, by design, and a green run would be the bug.** It rewrites `golden/` from +scratch (it is machine output, so it is captured, never merged into) and then *refuses to assert*, +failing with a notice saying so. The reason is worth stating: in update mode the expected side is +produced by the very harness that produced the actual side, so a comparison between them is a +tautology — and a tautology reported green is exactly how a stale variable left in someone's shell +disarms a golden test without anyone noticing. + +So the procedure is two runs, not one. Rewrite, review +`git diff producers/tests/OkfProducer.Tests/fixtures/golden`, then **re-run without the variable** to +actually check it, and commit the diff with the change that caused it. **Two intentional changes will rewrite the whole golden**, and neither is drift: diff --git a/producers/tests/OkfProducer.Tests/fixtures/fixture-repo/src/Shapes.cs b/producers/tests/OkfProducer.Tests/fixtures/fixture-repo/src/Shapes.cs new file mode 100644 index 00000000..15100dfc --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/fixture-repo/src/Shapes.cs @@ -0,0 +1,118 @@ +// Declaration shapes the golden did not cover. Before this file, the captured bundle held three +// classes and their methods and nothing else: no interface, struct, record or enum, no constructor, +// event, field or local function, no nested or generic type, no explicit interface implementation, +// no block-scoped namespace, no non-public visibility, and no type whose header spans more than one +// line. Deleting the whole escaping layer, or returning `StartLine + 1` from the header cap, would +// not have moved one golden byte. +namespace N.Shapes +{ + /// A contract with one member, so an interface_declaration reaches the golden. + public interface IShape + { + /// Renders the shape. + string Render(); + } + + /// An enum, whose members are deliberately NOT emitted as concepts of their own. + public enum Corner + { + Sharp, + Round, + } + + /// A struct, so the value-type arm of the declaration query is exercised. + public struct Point + { + /// Two declarators on one field, which is the rule no fixture reached. + public int X, Y; + } + + /// A positional record, whose header carries no body at all. + public record Size(int Width, int Height); + + /// + /// A type whose header spans three lines, which nothing else here does. The header cap (R48) is + /// the churn bound the whole id scheme rests on, and with every fixture type putting { on + /// the next line it would produce a byte-identical golden if it simply returned + /// StartLine + 1. This declaration is what makes that substitution visible. + /// + public sealed class Boxed + : IShape, + System.IEquatable + { + /// Raised when the box changes. An event_field_declaration. + public event System.Action? Changed; + + private readonly Corner _corner; + + /// A constructor, which is its own declaration kind. + public Boxed(Corner corner) + { + _corner = corner; + } + + /// + /// Renders the box. The doc comment carries and markdown-significant + /// characters -- [brackets], a `backtick` and a <tag> -- so the neutralisation layer is + /// exercised by the golden rather than only by unit tests. + /// + public string Render() + { + string Compose(string prefix) => prefix + _corner; + + return Compose("box:"); + } + + /// Compares two boxes. Explicitly implemented, so it is a distinct concept from any public Equals. + bool System.IEquatable.Equals(Boxed? other) + { + return other is not null && other._corner == _corner; + } + + /// A nested type, so the containment spine has a type inside a type to describe. + public sealed class Builder + { + /// Builds a box. + public Boxed Build(Corner corner) + { + return new Boxed(corner); + } + } + } + + /// Non-generic, and the first of three declarations that differ only by arity. + public class Holder + { + /// Held count. + public int Count() + { + return 0; + } + } + + /// Arity one. A separate concept from , which it was not before. + public class Holder + { + /// The held value. + public T? Value { get; set; } + } + + /// Arity two. + public class Holder + { + /// Looks the value up. + public TValue? Find(TKey key) + { + return default; + } + } + + /// Internal rather than public, so scope filtering has something to filter here. + internal class Hidden + { + /// Not in scope by default. + public void Never() + { + } + } +} diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/.okfgen-manifest.json b/producers/tests/OkfProducer.Tests/fixtures/golden/.okfgen-manifest.json index f969aabc..fdd6792f 100644 --- a/producers/tests/OkfProducer.Tests/fixtures/golden/.okfgen-manifest.json +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/.okfgen-manifest.json @@ -8,6 +8,7 @@ "extractedFiles": [ "src/Registry.cs", "src/Scanner.cs", + "src/Shapes.cs", "src/Sub/Formatter.cs" ], "concepts": [ @@ -16,6 +17,7 @@ "sources": [ "src/Registry.cs", "src/Scanner.cs", + "src/Shapes.cs", "src/Sub/Formatter.cs" ] }, @@ -67,6 +69,138 @@ "src/Scanner.cs" ] }, + { + "id": "code/csharp/n/shapes", + "sources": [ + "src/Shapes.cs" + ] + }, + { + "id": "code/csharp/n/shapes/boxed", + "sources": [ + "src/Shapes.cs" + ] + }, + { + "id": "code/csharp/n/shapes/boxed/boxed", + "sources": [ + "src/Shapes.cs" + ] + }, + { + "id": "code/csharp/n/shapes/boxed/builder", + "sources": [ + "src/Shapes.cs" + ] + }, + { + "id": "code/csharp/n/shapes/boxed/builder/build", + "sources": [ + "src/Shapes.cs" + ] + }, + { + "id": "code/csharp/n/shapes/boxed/changed", + "sources": [ + "src/Shapes.cs" + ] + }, + { + "id": "code/csharp/n/shapes/boxed/render", + "sources": [ + "src/Shapes.cs" + ] + }, + { + "id": "code/csharp/n/shapes/corner", + "sources": [ + "src/Shapes.cs" + ] + }, + { + "id": "code/csharp/n/shapes/hidden", + "sources": [ + "src/Shapes.cs" + ] + }, + { + "id": "code/csharp/n/shapes/hidden/never", + "sources": [ + "src/Shapes.cs" + ] + }, + { + "id": "code/csharp/n/shapes/holder", + "sources": [ + "src/Shapes.cs" + ] + }, + { + "id": "code/csharp/n/shapes/holder/count", + "sources": [ + "src/Shapes.cs" + ] + }, + { + "id": "code/csharp/n/shapes/holder_1", + "sources": [ + "src/Shapes.cs" + ] + }, + { + "id": "code/csharp/n/shapes/holder_1/value", + "sources": [ + "src/Shapes.cs" + ] + }, + { + "id": "code/csharp/n/shapes/holder_2", + "sources": [ + "src/Shapes.cs" + ] + }, + { + "id": "code/csharp/n/shapes/holder_2/find", + "sources": [ + "src/Shapes.cs" + ] + }, + { + "id": "code/csharp/n/shapes/i-shape", + "sources": [ + "src/Shapes.cs" + ] + }, + { + "id": "code/csharp/n/shapes/i-shape/render", + "sources": [ + "src/Shapes.cs" + ] + }, + { + "id": "code/csharp/n/shapes/point", + "sources": [ + "src/Shapes.cs" + ] + }, + { + "id": "code/csharp/n/shapes/point/x", + "sources": [ + "src/Shapes.cs" + ] + }, + { + "id": "code/csharp/n/shapes/point/y", + "sources": [ + "src/Shapes.cs" + ] + }, + { + "id": "code/csharp/n/shapes/size", + "sources": [ + "src/Shapes.cs" + ] + }, { "id": "code/csharp/n/sub", "sources": [ diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/index.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/index.md index 8346bc6e..4110fcb3 100644 --- a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/index.md +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/index.md @@ -4,4 +4,4 @@ # Subdirectories -* [n](n/index.md) - Contains 6: registry, N.Registry, scanner, N.Scanner, sub, N.Sub. +* [n](n/index.md) - Contains 8: registry, N.Registry, scanner, N.Scanner, shapes, N.Shapes, sub, N.Sub. diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n.md index d14d17b5..040a5e40 100644 --- a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n.md +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n.md @@ -18,4 +18,5 @@ N, a top-level code container. - [N.Registry](/code/csharp/n/registry) - [N.Scanner](/code/csharp/n/scanner) +- [N.Shapes](/code/csharp/n/shapes) - [N.Sub](/code/csharp/n/sub) diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/index.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/index.md index d1e8851a..d481b057 100644 --- a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/index.md +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/index.md @@ -1,5 +1,6 @@ # C# Container +* [N.Shapes](shapes.md) - Shapes, a code container in N. * [N.Sub](sub.md) - Sub, a code container in N. # C# Type @@ -11,4 +12,5 @@ * [registry](registry/index.md) - Contains 2: Registry.Count, Registry.Register. * [scanner](scanner/index.md) - Contains 4: Scanner.Gone, Scanner.Normalize, Scanner.Root, Scanner.Scan. +* [shapes](shapes/index.md) - Contains 16: boxed, Shapes.Boxed, Shapes.Corner, hidden, Shapes.Hidden, holder, Shapes.Holder, holder_1, Shapes.Holder_1, holder_2, Shapes.Holder_2, i-shape, Shapes.IShape, point, Shapes.Point, Shapes.Size. * [sub](sub/index.md) - Contains 2: formatter, Sub.Formatter. diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes.md new file mode 100644 index 00000000..83e1db5e --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes.md @@ -0,0 +1,27 @@ +--- +type: C# Container +title: N.Shapes +description: Shapes, a code container in N. +tags: + - csharp + - container +description_source: generated +generated: + by: okfgen/0.1.0 +--- + +# N.Shapes + +Shapes, a code container in N. + +## Contains + +- [Shapes.Boxed](/code/csharp/n/shapes/boxed) +- [Shapes.Corner](/code/csharp/n/shapes/corner) +- [Shapes.Hidden](/code/csharp/n/shapes/hidden) +- [Shapes.Holder](/code/csharp/n/shapes/holder) +- [Shapes.Holder_1](/code/csharp/n/shapes/holder_1) +- [Shapes.Holder_2](/code/csharp/n/shapes/holder_2) +- [Shapes.IShape](/code/csharp/n/shapes/i-shape) +- [Shapes.Point](/code/csharp/n/shapes/point) +- [Shapes.Size](/code/csharp/n/shapes/size) diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed.md new file mode 100644 index 00000000..b3dae92e --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed.md @@ -0,0 +1,28 @@ +--- +type: C# Type +title: Shapes.Boxed +description: A type whose header spans three lines, which nothing else here does. The header cap (R48) is the churn bound the whole id scheme rests on, and with every fixture type putting { on the next line it would produce a byte-identical golden if it simply returned StartLine + 1. This declaration is what makes that substitution visible. +resource: https://example.com/acme/fixture/blob/main/src/Shapes.cs#L39-L42 +tags: + - csharp + - type + - public +description_source: doc-comment +generated: + by: okfgen/0.1.0 +--- + +# Shapes.Boxed + +A type whose header spans three lines, which nothing else here does. The header cap (R48) is the churn bound the whole id scheme rests on, and with every fixture type putting { on the next line it would produce a byte-identical golden if it simply returned StartLine + 1. This declaration is what makes that substitution visible. + +## Signatures + +- `public sealed class Boxed : IShape, System.IEquatable` — `src/Shapes.cs#L39-L42` + +## Contains + +- [Boxed.Boxed](/code/csharp/n/shapes/boxed/boxed) +- [Boxed.Builder](/code/csharp/n/shapes/boxed/builder) +- [Boxed.Changed](/code/csharp/n/shapes/boxed/changed) +- [Boxed.Render](/code/csharp/n/shapes/boxed/render) diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/boxed.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/boxed.md new file mode 100644 index 00000000..49e00ec4 --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/boxed.md @@ -0,0 +1,21 @@ +--- +type: C# Member +title: Boxed.Boxed +description: A constructor, which is its own declaration kind. +resource: https://example.com/acme/fixture/blob/main/src/Shapes.cs#L49-L52 +tags: + - csharp + - member + - public +description_source: doc-comment +generated: + by: okfgen/0.1.0 +--- + +# Boxed.Boxed + +A constructor, which is its own declaration kind. + +## Signatures + +- `public Boxed(Corner corner)` — `src/Shapes.cs#L49-L52` diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/builder.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/builder.md new file mode 100644 index 00000000..396b54d3 --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/builder.md @@ -0,0 +1,25 @@ +--- +type: C# Type +title: Boxed.Builder +description: A nested type, so the containment spine has a type inside a type to describe. +resource: https://example.com/acme/fixture/blob/main/src/Shapes.cs#L73-L74 +tags: + - csharp + - type + - public +description_source: doc-comment +generated: + by: okfgen/0.1.0 +--- + +# Boxed.Builder + +A nested type, so the containment spine has a type inside a type to describe. + +## Signatures + +- `public sealed class Builder` — `src/Shapes.cs#L73-L74` + +## Contains + +- [Builder.Build](/code/csharp/n/shapes/boxed/builder/build) diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/builder/build.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/builder/build.md new file mode 100644 index 00000000..d833a6be --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/builder/build.md @@ -0,0 +1,21 @@ +--- +type: C# Member +title: Builder.Build +description: Builds a box. +resource: https://example.com/acme/fixture/blob/main/src/Shapes.cs#L76-L79 +tags: + - csharp + - member + - public +description_source: doc-comment +generated: + by: okfgen/0.1.0 +--- + +# Builder.Build + +Builds a box. + +## Signatures + +- `public Boxed Build(Corner corner)` — `src/Shapes.cs#L76-L79` diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/builder/index.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/builder/index.md new file mode 100644 index 00000000..d6cd5c63 --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/builder/index.md @@ -0,0 +1,3 @@ +# C# Member + +* [Builder.Build](build.md) - Builds a box. diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/changed.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/changed.md new file mode 100644 index 00000000..598bb83b --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/changed.md @@ -0,0 +1,21 @@ +--- +type: C# Member +title: Boxed.Changed +description: Raised when the box changes. An event_field_declaration. +resource: https://example.com/acme/fixture/blob/main/src/Shapes.cs#L44 +tags: + - csharp + - member + - public +description_source: doc-comment +generated: + by: okfgen/0.1.0 +--- + +# Boxed.Changed + +Raised when the box changes. An event_field_declaration. + +## Signatures + +- `public event System.Action? Changed` — `src/Shapes.cs#L44` diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/index.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/index.md new file mode 100644 index 00000000..5f5d4a28 --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/index.md @@ -0,0 +1,13 @@ +# C# Member + +* [Boxed.Boxed](boxed.md) - A constructor, which is its own declaration kind. +* [Boxed.Changed](changed.md) - Raised when the box changes. An event_field_declaration. +* [Boxed.Render](render.md) - Renders the box. The doc comment carries Corner and markdown-significant characters -- [brackets], a `backtick` and a -- so the neutralisation layer is exercised by the golden rather than only by unit tests. + +# C# Type + +* [Boxed.Builder](builder.md) - A nested type, so the containment spine has a type inside a type to describe. + +# Subdirectories + +* [builder](builder/index.md) - Builds a box. diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/render.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/render.md new file mode 100644 index 00000000..165f10c1 --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/boxed/render.md @@ -0,0 +1,25 @@ +--- +type: C# Member +title: Boxed.Render +description: Renders the box. The doc comment carries Corner and markdown-significant characters -- [brackets], a `backtick` and a -- so the neutralisation layer is exercised by the golden rather than only by unit tests. +resource: https://example.com/acme/fixture/blob/main/src/Shapes.cs#L59-L64 +tags: + - csharp + - member + - public +description_source: doc-comment +generated: + by: okfgen/0.1.0 +--- + +# Boxed.Render + +Renders the box. The doc comment carries Corner and markdown-significant characters -- [brackets\], a `backtick` and a -- so the neutralisation layer is exercised by the golden rather than only by unit tests. + +## Signatures + +- `public string Render()` — `src/Shapes.cs#L59-L64` + +## Calls (unresolved) + +- `Compose` diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/corner.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/corner.md new file mode 100644 index 00000000..f5d05cec --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/corner.md @@ -0,0 +1,21 @@ +--- +type: C# Type +title: Shapes.Corner +description: An enum, whose members are deliberately NOT emitted as concepts of their own. +resource: https://example.com/acme/fixture/blob/main/src/Shapes.cs#L17-L18 +tags: + - csharp + - type + - public +description_source: doc-comment +generated: + by: okfgen/0.1.0 +--- + +# Shapes.Corner + +An enum, whose members are deliberately NOT emitted as concepts of their own. + +## Signatures + +- `public enum Corner` — `src/Shapes.cs#L17-L18` diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/hidden.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/hidden.md new file mode 100644 index 00000000..5b26bc15 --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/hidden.md @@ -0,0 +1,19 @@ +--- +type: C# Container +title: Shapes.Hidden +description: Hidden, a code container in N.Shapes. +tags: + - csharp + - container +description_source: generated +generated: + by: okfgen/0.1.0 +--- + +# Shapes.Hidden + +Hidden, a code container in N.Shapes. + +## Contains + +- [Hidden.Never](/code/csharp/n/shapes/hidden/never) diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/hidden/index.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/hidden/index.md new file mode 100644 index 00000000..f0edac1e --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/hidden/index.md @@ -0,0 +1,3 @@ +# C# Member + +* [Hidden.Never](never.md) - Not in scope by default. diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/hidden/never.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/hidden/never.md new file mode 100644 index 00000000..1040343a --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/hidden/never.md @@ -0,0 +1,21 @@ +--- +type: C# Member +title: Hidden.Never +description: Not in scope by default. +resource: https://example.com/acme/fixture/blob/main/src/Shapes.cs#L114-L116 +tags: + - csharp + - member + - public +description_source: doc-comment +generated: + by: okfgen/0.1.0 +--- + +# Hidden.Never + +Not in scope by default. + +## Signatures + +- `public void Never()` — `src/Shapes.cs#L114-L116` diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder.md new file mode 100644 index 00000000..d4fd61c3 --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder.md @@ -0,0 +1,25 @@ +--- +type: C# Type +title: Shapes.Holder +description: Non-generic, and the first of three declarations that differ only by arity. +resource: https://example.com/acme/fixture/blob/main/src/Shapes.cs#L84-L85 +tags: + - csharp + - type + - public +description_source: doc-comment +generated: + by: okfgen/0.1.0 +--- + +# Shapes.Holder + +Non-generic, and the first of three declarations that differ only by arity. + +## Signatures + +- `public class Holder` — `src/Shapes.cs#L84-L85` + +## Contains + +- [Holder.Count](/code/csharp/n/shapes/holder/count) diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder/count.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder/count.md new file mode 100644 index 00000000..fecf3f03 --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder/count.md @@ -0,0 +1,21 @@ +--- +type: C# Member +title: Holder.Count +description: Held count. +resource: https://example.com/acme/fixture/blob/main/src/Shapes.cs#L87-L90 +tags: + - csharp + - member + - public +description_source: doc-comment +generated: + by: okfgen/0.1.0 +--- + +# Holder.Count + +Held count. + +## Signatures + +- `public int Count()` — `src/Shapes.cs#L87-L90` diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder/index.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder/index.md new file mode 100644 index 00000000..315e7ad6 --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder/index.md @@ -0,0 +1,3 @@ +# C# Member + +* [Holder.Count](count.md) - Held count. diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_1.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_1.md new file mode 100644 index 00000000..cf55d0a4 --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_1.md @@ -0,0 +1,25 @@ +--- +type: C# Type +title: Shapes.Holder_1 +description: Arity one. A separate concept from Holder, which it was not before. +resource: https://example.com/acme/fixture/blob/main/src/Shapes.cs#L94-L95 +tags: + - csharp + - type + - public +description_source: doc-comment +generated: + by: okfgen/0.1.0 +--- + +# Shapes.Holder_1 + +Arity one. A separate concept from Holder, which it was not before. + +## Signatures + +- `public class Holder` — `src/Shapes.cs#L94-L95` + +## Contains + +- [Holder_1.Value](/code/csharp/n/shapes/holder_1/value) diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_1/index.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_1/index.md new file mode 100644 index 00000000..209a71f5 --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_1/index.md @@ -0,0 +1,3 @@ +# C# Member + +* [Holder_1.Value](value.md) - The held value. diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_1/value.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_1/value.md new file mode 100644 index 00000000..8220318b --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_1/value.md @@ -0,0 +1,21 @@ +--- +type: C# Member +title: Holder_1.Value +description: The held value. +resource: https://example.com/acme/fixture/blob/main/src/Shapes.cs#L97 +tags: + - csharp + - member + - public +description_source: doc-comment +generated: + by: okfgen/0.1.0 +--- + +# Holder_1.Value + +The held value. + +## Signatures + +- `public T? Value` — `src/Shapes.cs#L97` diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_2.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_2.md new file mode 100644 index 00000000..287a9bf0 --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_2.md @@ -0,0 +1,25 @@ +--- +type: C# Type +title: Shapes.Holder_2 +description: Arity two. +resource: https://example.com/acme/fixture/blob/main/src/Shapes.cs#L101-L102 +tags: + - csharp + - type + - public +description_source: doc-comment +generated: + by: okfgen/0.1.0 +--- + +# Shapes.Holder_2 + +Arity two. + +## Signatures + +- `public class Holder` — `src/Shapes.cs#L101-L102` + +## Contains + +- [Holder_2.Find](/code/csharp/n/shapes/holder_2/find) diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_2/find.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_2/find.md new file mode 100644 index 00000000..de0aeb13 --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_2/find.md @@ -0,0 +1,21 @@ +--- +type: C# Member +title: Holder_2.Find +description: Looks the value up. +resource: https://example.com/acme/fixture/blob/main/src/Shapes.cs#L104-L107 +tags: + - csharp + - member + - public +description_source: doc-comment +generated: + by: okfgen/0.1.0 +--- + +# Holder_2.Find + +Looks the value up. + +## Signatures + +- `public TValue? Find(TKey key)` — `src/Shapes.cs#L104-L107` diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_2/index.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_2/index.md new file mode 100644 index 00000000..381ce917 --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/holder_2/index.md @@ -0,0 +1,3 @@ +# C# Member + +* [Holder_2.Find](find.md) - Looks the value up. diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/i-shape.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/i-shape.md new file mode 100644 index 00000000..b91d6367 --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/i-shape.md @@ -0,0 +1,25 @@ +--- +type: C# Type +title: Shapes.IShape +description: A contract with one member, so an interface_declaration reaches the golden. +resource: https://example.com/acme/fixture/blob/main/src/Shapes.cs#L10-L11 +tags: + - csharp + - type + - public +description_source: doc-comment +generated: + by: okfgen/0.1.0 +--- + +# Shapes.IShape + +A contract with one member, so an interface_declaration reaches the golden. + +## Signatures + +- `public interface IShape` — `src/Shapes.cs#L10-L11` + +## Contains + +- [IShape.Render](/code/csharp/n/shapes/i-shape/render) diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/i-shape/index.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/i-shape/index.md new file mode 100644 index 00000000..e2d6819c --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/i-shape/index.md @@ -0,0 +1,3 @@ +# C# Member + +* [IShape.Render](render.md) - Renders the shape. diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/i-shape/render.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/i-shape/render.md new file mode 100644 index 00000000..ecd8c532 --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/i-shape/render.md @@ -0,0 +1,21 @@ +--- +type: C# Member +title: IShape.Render +description: Renders the shape. +resource: https://example.com/acme/fixture/blob/main/src/Shapes.cs#L13 +tags: + - csharp + - member + - public +description_source: doc-comment +generated: + by: okfgen/0.1.0 +--- + +# IShape.Render + +Renders the shape. + +## Signatures + +- `string Render()` — `src/Shapes.cs#L13` diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/index.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/index.md new file mode 100644 index 00000000..95d3b268 --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/index.md @@ -0,0 +1,24 @@ +# C# Container + +* [Shapes.Hidden](hidden.md) - Hidden, a code container in N.Shapes. + +# C# Type + +* [Shapes.Boxed](boxed.md) - A type whose header spans three lines, which nothing else here does. The header cap (R48) is the churn bound the whole id scheme rests on, and with every fixture type putting { on the next line it would produce a byte-identical golden if it simply returned StartLine + 1. This declaration is what makes that substitution visible. +* [Shapes.Corner](corner.md) - An enum, whose members are deliberately NOT emitted as concepts of their own. +* [Shapes.Holder](holder.md) - Non-generic, and the first of three declarations that differ only by arity. +* [Shapes.Holder_1](holder_1.md) - Arity one. A separate concept from Holder, which it was not before. +* [Shapes.Holder_2](holder_2.md) - Arity two. +* [Shapes.IShape](i-shape.md) - A contract with one member, so an interface_declaration reaches the golden. +* [Shapes.Point](point.md) - A struct, so the value-type arm of the declaration query is exercised. +* [Shapes.Size](size.md) - A positional record, whose header carries no body at all. + +# Subdirectories + +* [boxed](boxed/index.md) - Contains 5: Boxed.Boxed, builder, Boxed.Builder, Boxed.Changed, Boxed.Render. +* [hidden](hidden/index.md) - Not in scope by default. +* [holder](holder/index.md) - Held count. +* [holder_1](holder_1/index.md) - The held value. +* [holder_2](holder_2/index.md) - Looks the value up. +* [i-shape](i-shape/index.md) - Renders the shape. +* [point](point/index.md) - Contains 2: Point.X, Point.Y. diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/point.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/point.md new file mode 100644 index 00000000..956cdd1f --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/point.md @@ -0,0 +1,26 @@ +--- +type: C# Type +title: Shapes.Point +description: A struct, so the value-type arm of the declaration query is exercised. +resource: https://example.com/acme/fixture/blob/main/src/Shapes.cs#L24-L25 +tags: + - csharp + - type + - public +description_source: doc-comment +generated: + by: okfgen/0.1.0 +--- + +# Shapes.Point + +A struct, so the value-type arm of the declaration query is exercised. + +## Signatures + +- `public struct Point` — `src/Shapes.cs#L24-L25` + +## Contains + +- [Point.X](/code/csharp/n/shapes/point/x) +- [Point.Y](/code/csharp/n/shapes/point/y) diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/point/index.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/point/index.md new file mode 100644 index 00000000..db18e698 --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/point/index.md @@ -0,0 +1,4 @@ +# C# Member + +* [Point.X](x.md) - Two declarators on one field, which is the rule no fixture reached. +* [Point.Y](y.md) - Two declarators on one field, which is the rule no fixture reached. diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/point/x.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/point/x.md new file mode 100644 index 00000000..0edd21f6 --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/point/x.md @@ -0,0 +1,21 @@ +--- +type: C# Member +title: Point.X +description: Two declarators on one field, which is the rule no fixture reached. +resource: https://example.com/acme/fixture/blob/main/src/Shapes.cs#L27 +tags: + - csharp + - member + - public +description_source: doc-comment +generated: + by: okfgen/0.1.0 +--- + +# Point.X + +Two declarators on one field, which is the rule no fixture reached. + +## Signatures + +- `public int X, Y` — `src/Shapes.cs#L27` diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/point/y.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/point/y.md new file mode 100644 index 00000000..71d58ba8 --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/point/y.md @@ -0,0 +1,21 @@ +--- +type: C# Member +title: Point.Y +description: Two declarators on one field, which is the rule no fixture reached. +resource: https://example.com/acme/fixture/blob/main/src/Shapes.cs#L27 +tags: + - csharp + - member + - public +description_source: doc-comment +generated: + by: okfgen/0.1.0 +--- + +# Point.Y + +Two declarators on one field, which is the rule no fixture reached. + +## Signatures + +- `public int X, Y` — `src/Shapes.cs#L27` diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/size.md b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/size.md new file mode 100644 index 00000000..8a2dcb3c --- /dev/null +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/code/csharp/n/shapes/size.md @@ -0,0 +1,21 @@ +--- +type: C# Type +title: Shapes.Size +description: A positional record, whose header carries no body at all. +resource: https://example.com/acme/fixture/blob/main/src/Shapes.cs#L31 +tags: + - csharp + - type + - public +description_source: doc-comment +generated: + by: okfgen/0.1.0 +--- + +# Shapes.Size + +A positional record, whose header carries no body at all. + +## Signatures + +- `public record Size(int Width, int Height)` — `src/Shapes.cs#L31` diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/overview.md b/producers/tests/OkfProducer.Tests/fixtures/golden/overview.md index 64a603e4..ad3c4f63 100644 --- a/producers/tests/OkfProducer.Tests/fixtures/golden/overview.md +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/overview.md @@ -5,8 +5,8 @@ description: Repository fixture-repo, containing 1 detected package. tags: - repository generated: - by: okfgen/0.1.0 - at: 2026-09-03T12:28:06Z + by: okfgen/0.1.0 tree-sitter/1.3.0 + at: 2026-09-06T07:38:05Z --- # fixture-repo diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/packages/fixture.md b/producers/tests/OkfProducer.Tests/fixtures/golden/packages/fixture.md index f96b84e4..2b02514a 100644 --- a/producers/tests/OkfProducer.Tests/fixtures/golden/packages/fixture.md +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/packages/fixture.md @@ -10,3 +10,7 @@ tags: # Fixture The one package of the producer's fixture repository. + +## Contains + +- [N](/code/csharp/n) From bedd3ce823feb6280fba0088c71e13b96aa6f821 Mon Sep 17 00:00:00 2001 From: Julien CHABLE Date: Sun, 6 Sep 2026 10:38:00 +0200 Subject: [PATCH 09/36] test(producer): stop the golden's own tests from being able to disarm themselves First tranche of the Minor register, taken in the order the user arbitrated: the one that protects the other tests, then the ones about test strength. **D3-9 -- nothing asserted that `--check` leaves its input bundle alone.** Six tests in `CheckTests` run `BundleDrift.Check` against the COMMITTED golden. A regression that regenerated in place rather than into the copy would rewrite the golden, and every one of those six would go on passing, comparing the bundle against itself. The golden was just enlarged from 15 concepts to 37, so it has more to lose than it did. Now hashed byte for byte, before and against after. Not a file count and not a timestamp: regenerating in place leaves the same names, and `generated.at` would not move either, since the golden is captured outside git where that field is excluded from the COMPARISON but not from a write. Verified by mutation: passing `bundlePath` where `Check` passes `copyPath` fails it. **D3-5 -- every "ordinal" assertion pinned sortedness, not Ordinal.** `Claimants_come_back_in_ordinal_order_whatever_order_they_arrived_in` compared `A` against `Z`, a pair that sorts the same way under every culture on earth. Swapping every `StringComparer.Ordinal` in `SourceOwnershipMap` for `CurrentCulture` left it green, while 6.2 pins Ordinal. The fixture is now `Z` against `a`: Ordinal compares code points, where `Z` (0x5A) precedes `a` (0x61), and a linguistic comparison puts `a` first. The two orders are opposite, so only one passes -- and the culture swap now fails it. **E2-4 -- `--force` was exercised by no test at all.** Documented as an alias for `--reset`; dropping the `|| forceOption` clause broke it silently, leaving an operator with a run that refuses a non-empty `--out` instead of recreating it. The fixture is what discriminates: a bundle holding a file the regeneration does not produce is the only shape where reset and no-reset differ observably. Verified by mutation. **D3-10 -- a comment defending against something that cannot happen.** `IsWithinPrefixRoot`'s doc said the component comparison stops a sibling whose name merely starts with the prefix (`code2/` beside `code/`) from being walked into and deleted. The walk only ever goes upward, so it never reaches a sibling. What the guard really does is stop the walk climbing ABOVE the owned prefix -- the sole condition between a `Directory.Delete(recursive: true)` and the bundle root -- and the comment now says that, and names the test that finally put it on a critical path. 628 tests green, format clean, no fixture touched. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0166CcRSdoZrL8g1Suruy6Uz --- .../Generation/BundleWriter.cs | 18 +++++++-- producers/tests/OkfProducer.Tests/CliTests.cs | 23 +++++++++++ .../Generation/CheckTests.cs | 40 +++++++++++++++++++ .../Generation/SourceOwnershipMapTests.cs | 13 ++++-- .../fixtures/golden/overview.md | 2 +- 5 files changed, 89 insertions(+), 7 deletions(-) diff --git a/producers/src/OkfProducer.Core/Generation/BundleWriter.cs b/producers/src/OkfProducer.Core/Generation/BundleWriter.cs index 9cddf0e5..5ca69b10 100644 --- a/producers/src/OkfProducer.Core/Generation/BundleWriter.cs +++ b/producers/src/OkfProducer.Core/Generation/BundleWriter.cs @@ -870,9 +870,21 @@ private static void RemoveEmptyDirectories(string outPath, string ownedPrefix, I /// /// Whether is the owned prefix's own directory or one nested inside - /// it. Compares path components rather than raw string prefixes, so a sibling directory whose name - /// merely starts with the prefix (code2/ beside code/) is not walked into and - /// deleted. + /// it. Compares path components rather than raw string prefixes. + /// + /// What it actually stops: the walk climbing ABOVE the owned prefix. + /// starts at a pruned concept's directory and moves upward one + /// parent at a time, so this is the sole condition standing between a + /// Directory.Delete(recursive: true) and the bundle root. It is the most destructive + /// statement on this branch, and until + /// PruningTests.The_directory_ladder_stops_at_the_owned_prefix_root_and_not_above_it it was + /// on no test's critical path -- every other fixture left a sibling that broke the loop one rung in. + /// + /// This comment used to describe a different scenario: a sibling directory whose name merely + /// starts with the prefix (code2/ beside code/) being walked into and deleted. That + /// cannot happen and never could -- the walk only ever goes upward, so it never reaches a sibling + /// at all. The component comparison is still the right implementation; it just was not defending + /// against the thing the comment named. /// private static bool IsWithinPrefixRoot(string directory, string prefixRoot) => string.Equals(directory, prefixRoot, PathComparison) diff --git a/producers/tests/OkfProducer.Tests/CliTests.cs b/producers/tests/OkfProducer.Tests/CliTests.cs index 11e26267..0a52c476 100644 --- a/producers/tests/OkfProducer.Tests/CliTests.cs +++ b/producers/tests/OkfProducer.Tests/CliTests.cs @@ -1217,6 +1217,29 @@ public void The_validate_verb_names_its_bundle_option_okf() Assert.NotEqual(0, Run("validate").ExitCode); Assert.NotEqual(0, Run("validate", "--bundle", bundle).ExitCode); } + + [Fact] + public void Force_is_an_alias_for_reset_and_not_a_flag_that_does_nothing() + { + // `--force` is documented as an alias for `--reset` and was exercised by no test at all: + // dropping the `|| forceOption` clause at the composition root broke the alias silently, and + // an operator using it would have got a run that refused a non-empty --out instead of + // recreating it. + // + // The fixture is what makes this discriminate. A bundle that already holds a file the + // regeneration does not produce is the only shape where reset and no-reset differ observably: + // without the alias the run refuses the non-empty directory, and with it the stray file is gone. + using var workspace = NewWorkspace(out var repo, out var bundle); + Assert.Equal(0, Run("generate", "--repo", repo, "--out", bundle).ExitCode); + + var stray = Path.Combine(bundle, "hand-written.md"); + File.WriteAllText(stray, "---\ntype: Note\ntitle: t\ndescription: d\n---\n\nbody\n"); + + var result = Run("generate", "--repo", repo, "--out", bundle, "--force"); + + Assert.Equal(0, result.ExitCode); + Assert.False(File.Exists(stray), "--force did not recreate the bundle, so it is not behaving as --reset."); + } // ---- assertions ------------------------------------------------------------------------- private sealed record CliResult(int ExitCode, string Output, string Error); diff --git a/producers/tests/OkfProducer.Tests/Generation/CheckTests.cs b/producers/tests/OkfProducer.Tests/Generation/CheckTests.cs index f719edcd..6d1352fa 100644 --- a/producers/tests/OkfProducer.Tests/Generation/CheckTests.cs +++ b/producers/tests/OkfProducer.Tests/Generation/CheckTests.cs @@ -656,6 +656,46 @@ public void The_golden_bundle_holds_one_occurrence_of_each_shape() Assert.Contains("## Calls (unresolved)\n", count, StringComparison.Ordinal); // an unresolved one } + [Fact] + public void Check_leaves_the_bundle_it_was_given_byte_for_byte_unchanged() + { + // The property every OTHER test in this file silently depends on. Six of them run Check against + // the COMMITTED golden, so a regression that regenerated in place would rewrite the golden -- + // and every one of those tests would go on passing, comparing the bundle against itself. A + // self-disarming suite is the failure this branch spent its remediation on; this is the + // assertion that stops it happening to the golden. + // + // Byte for byte over every file, not a count and not a timestamp: regenerating in place would + // leave the same file names, and `generated.at` would not move either, since the golden is + // captured outside git where that field is excluded from the COMPARISON but not from a write. + using var workspace = ProducerFixture.CopyRepoOutsideGit(); + + var before = Snapshot(ProducerFixture.GoldenBundle); + var report = RunCheck(workspace, ProducerFixture.GoldenBundle); + var after = Snapshot(ProducerFixture.GoldenBundle); + + // Asserted first, so a Check that somehow performed no comparison at all cannot satisfy the + // real assertions below by having done nothing. + Assert.True(report.IsClean, Explain(report)); + + Assert.Equal( + before.Keys.OrderBy(key => key, StringComparer.Ordinal), + after.Keys.OrderBy(key => key, StringComparer.Ordinal)); + + foreach (var (path, hash) in before) + { + Assert.True(after[path].SequenceEqual(hash), $"Check rewrote '{path}' in the bundle it was given."); + } + } + + /// Every file under , keyed by relative path, with its bytes hashed. + private static Dictionary Snapshot(string bundlePath) => + Directory.EnumerateFiles(bundlePath, "*", SearchOption.AllDirectories) + .ToDictionary( + path => Path.GetRelativePath(bundlePath, path).Replace(Path.DirectorySeparatorChar, '/'), + path => System.Security.Cryptography.SHA256.HashData(File.ReadAllBytes(path)), + StringComparer.Ordinal); + // -- fixture ---------------------------------------------------------------------------------- private static string RepoIn(ProducerFixture.TempDir workspace) => diff --git a/producers/tests/OkfProducer.Tests/Generation/SourceOwnershipMapTests.cs b/producers/tests/OkfProducer.Tests/Generation/SourceOwnershipMapTests.cs index 3fbc6543..784468f1 100644 --- a/producers/tests/OkfProducer.Tests/Generation/SourceOwnershipMapTests.cs +++ b/producers/tests/OkfProducer.Tests/Generation/SourceOwnershipMapTests.cs @@ -56,14 +56,21 @@ public void Claimants_come_back_in_ordinal_order_whatever_order_they_arrived_in( { // The Ordinal-first rule is only a rule if the order does not depend on which project the // caller queried first, so the fixture supplies them in the opposite order. + // + // The names are `Z` and `a`, not `A` and `Z`, and that is the fixture rather than a detail. + // §6.2 pins ORDINAL, but an `A`/`Z` pair sorts the same way under every culture on earth, so + // the assertion measured sortedness and called it Ordinal: swapping every + // `StringComparer.Ordinal` in this type for `CurrentCulture` left it green. Ordinal compares + // code points, where `Z` (0x5A) precedes `a` (0x61); a linguistic comparison puts `a` first. + // The two orders are now opposite, so only one of them passes. var map = SourceOwnershipMap.From("/repo", [ + new ProjectCompileItems("src/a/a.csproj", "net10.0", ["shared/Thing.cs"]), new ProjectCompileItems("src/Z/Z.csproj", "net10.0", ["shared/Thing.cs"]), - new ProjectCompileItems("src/A/A.csproj", "net10.0", ["shared/Thing.cs"]), ]); - Assert.Equal(["src/A/A.csproj", "src/Z/Z.csproj"], map.ClaimantsOf("shared/Thing.cs")); - Assert.Equal("src/A/A.csproj", map.OwnerOf("shared/Thing.cs")); + Assert.Equal(["src/Z/Z.csproj", "src/a/a.csproj"], map.ClaimantsOf("shared/Thing.cs")); + Assert.Equal("src/Z/Z.csproj", map.OwnerOf("shared/Thing.cs")); } [Fact] diff --git a/producers/tests/OkfProducer.Tests/fixtures/golden/overview.md b/producers/tests/OkfProducer.Tests/fixtures/golden/overview.md index ad3c4f63..1b5a670c 100644 --- a/producers/tests/OkfProducer.Tests/fixtures/golden/overview.md +++ b/producers/tests/OkfProducer.Tests/fixtures/golden/overview.md @@ -6,7 +6,7 @@ tags: - repository generated: by: okfgen/0.1.0 tree-sitter/1.3.0 - at: 2026-09-06T07:38:05Z + at: 2026-09-06T08:33:35Z --- # fixture-repo From 1a29328d36a36f9a69df7111dba8576fb96e4bf6 Mon Sep 17 00:00:00 2001 From: Julien CHABLE Date: Sun, 6 Sep 2026 10:44:55 +0200 Subject: [PATCH 10/36] fix(producer): an unborn branch is not a branch a permalink can point at MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **D4b-2.** `GitRevision.CurrentBranch` asked `git symbolic-ref` and stopped there. Measured: on a zero-commit repository that call SUCCEEDS and returns the unborn branch, while `rev-parse HEAD` fails. So `--repo-url` without `--rev` emitted a `resource` permalink on every code concept, each pointing into a branch that contains none of the files -- and the CLI's "no branch name could be read" note never fired to say otherwise. A wrong link reads exactly as confidently as a right one, which §2.3 calls the worse outcome, and this is not an exotic state: it is every repository on its first run of this producer. `symbolic-ref` is now followed by `rev-parse --verify HEAD`, and only where the first succeeded -- a caller that passed `--rev` never reaches this method. That second invocation is a real cost on a path whose whole documented virtue is being honest about process spawning, so `producers/README.md`'s count goes from "two to four" to "two to five" and says why. The test asserts both directions on the SAME repository: no branch before the commit, a branch after it. Without the second half, a machine with no working `git` would satisfy the first for the wrong reason. Verified by mutation: dropping the verify fails it. **B1-9 -- `RunStatus.Complete` has no production constructor, and now says so.** Grepped, not assumed: every production `RunStatus` comes out of `CodeGraphBuilder.Build` with the statuses it observed, and the only callers of this factory are test fixtures. Deliberately NOT removed, unlike `RoslynResolver`'s two summary properties earlier in this remediation, and the doc records the distinction: those made a claim §7.2 repeated and the code did not honour, while this is a factory for the type's own least surprising value and removing it would churn fourteen call sites for no behavioural gain. If you would rather it move to the test project for consistency, say so -- it is a one-pass change, and I did not want to read one arbitration as covering a different case. 629 tests green, format clean. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0166CcRSdoZrL8g1Suruy6Uz --- producers/README.md | 8 ++++-- .../OkfProducer.Core/CodeGraph/RunStatus.cs | 16 ++++++++++- .../Generation/GitRevision.cs | 28 ++++++++++++++++--- producers/tests/OkfProducer.Tests/CliTests.cs | 27 ++++++++++++++++++ 4 files changed, 71 insertions(+), 8 deletions(-) diff --git a/producers/README.md b/producers/README.md index a535b758..8aae970f 100644 --- a/producers/README.md +++ b/producers/README.md @@ -91,9 +91,11 @@ scanned tree, with the repository as the working directory, reading its `.git/co many times depends on the flags, so here is the whole of it rather than a number. `git show -s` and `git rev-parse` run on *every* generate — they stamp `overview`'s `generated.at` and `revision`. `git symbolic-ref` runs as well, **unless `--rev` already named the ref**, in -which case the branch is never read. And `--check` runs one further `git rev-parse` in the -scanned tree before the regeneration it compares against, on top of that regeneration's own. -Two to four invocations, then. Far less exposure than MSBuild — none of them triggers a hook, +which case the branch is never read — and where it succeeds it is followed by a second +`git rev-parse --verify HEAD`, because an *unborn* branch (`git init` with no commit yet) is a +branch name that names no commit, and permalinks built against it point at nothing. And +`--check` runs one further `git rev-parse` in the scanned tree before the regeneration it +compares against, on top of that regeneration's own. Two to five invocations, then. Far less exposure than MSBuild — none of them triggers a hook, an fsmonitor, or a pager with stdout redirected — but it is not nothing, and this section used to say "no process is spawned". And it is not free of structural cost: the source-ownership map comes out of the same MSBuild query, so with the flag on there is **no `packages` → diff --git a/producers/src/OkfProducer.Core/CodeGraph/RunStatus.cs b/producers/src/OkfProducer.Core/CodeGraph/RunStatus.cs index c3f67391..1f628d25 100644 --- a/producers/src/OkfProducer.Core/CodeGraph/RunStatus.cs +++ b/producers/src/OkfProducer.Core/CodeGraph/RunStatus.cs @@ -35,6 +35,20 @@ public sealed record RunStatus(bool TraversalComplete, IReadOnlyList<(string Pat /// public bool IsComplete => TraversalComplete && Skipped.All(s => s.Status == FileStatus.Extracted); - /// A run in which the traversal completed and every eligible file extracted cleanly. + /// + /// A run in which the traversal completed and every eligible file extracted cleanly. + /// + /// Nothing in producers/src constructs one through this. Grepped, not assumed: + /// every production comes out of CodeGraphBuilder.Build with the + /// statuses it actually observed, and the only readers here are this solution's test fixtures, + /// which want "a clean run" in one token. Recorded so the next reader does not take it for a + /// production path and reason about a code path that has no callers -- the shape this branch has + /// been caught by more than once. + /// + /// Kept rather than moved to the test project, unlike RoslynResolver's two removed + /// summary properties: those made a claim the spec repeated and the code did not honour, while this + /// is a factory for the type's own least surprising value, and the fourteen call sites it would + /// churn buy nothing. + /// public static RunStatus Complete { get; } = new(true, []); } diff --git a/producers/src/OkfProducer.Core/Generation/GitRevision.cs b/producers/src/OkfProducer.Core/Generation/GitRevision.cs index 6e53d85f..7a308348 100644 --- a/producers/src/OkfProducer.Core/Generation/GitRevision.cs +++ b/producers/src/OkfProducer.Core/Generation/GitRevision.cs @@ -72,8 +72,9 @@ public static string HeadCommitInstant(string repoRoot) /// /// The name of the branch currently checked out in (e.g. main, - /// feature/x), or when there is none to read: a detached - /// HEAD, a path outside any git repository, or git itself not runnable. + /// feature/x), or when there is none a permalink could be built + /// against: a detached HEAD, an unborn branch (initialised, no commit yet), a path + /// outside any git repository, or git itself not runnable. /// /// What the null is for. This is the default the CLI's --rev falls back to /// when building §4.3's resource permalinks, and a branch name is deliberately the only @@ -90,8 +91,27 @@ public static string HeadCommitInstant(string repoRoot) /// the caller as a null. /// /// The repository root to run git in. - public static string? CurrentBranch(string repoRoot) => - RunGit(repoRoot, "symbolic-ref", "--quiet", "--short", "HEAD") is { Length: > 0 } branch ? branch : null; + public static string? CurrentBranch(string repoRoot) + { + if (RunGit(repoRoot, "symbolic-ref", "--quiet", "--short", "HEAD") is not { Length: > 0 } branch) + { + return null; + } + + // An UNBORN branch is a branch name that names no commit, and a permalink built against it + // resolves to nothing. `git init` followed by no commit is exactly that state, and it is not + // exotic -- it is every repository on its first run of this producer. + // + // Measured: on a zero-commit repository `symbolic-ref` SUCCEEDS and returns the branch while + // `rev-parse HEAD` fails. So checking only the first left `--repo-url` without `--rev` emitting + // a `resource` on every code concept, all pointing into a branch containing none of the files, + // and the caller's "no branch name could be read" note never fired to say otherwise. A wrong + // link reads exactly as confidently as a right one, which §2.3 calls the worse outcome. + // + // The second invocation is the cost, and it is only paid where the first succeeded: a caller + // that passed --rev never reaches this method at all. producers/README.md counts it. + return RunGit(repoRoot, "rev-parse", "--verify", "--quiet", "HEAD") is { Length: > 0 } ? branch : null; + } /// Formats , taken as UTC, as yyyy-MM-ddTHH:mm:ssZ -- invariant, second precision, a literal Z. private static string FormatUtc(DateTimeOffset instant) => diff --git a/producers/tests/OkfProducer.Tests/CliTests.cs b/producers/tests/OkfProducer.Tests/CliTests.cs index 0a52c476..70fd72d0 100644 --- a/producers/tests/OkfProducer.Tests/CliTests.cs +++ b/producers/tests/OkfProducer.Tests/CliTests.cs @@ -1218,6 +1218,33 @@ public void The_validate_verb_names_its_bundle_option_okf() Assert.NotEqual(0, Run("validate", "--bundle", bundle).ExitCode); } + [Fact] + public void A_repository_with_no_commit_yet_reports_no_branch_name() + { + // MEASURED, and the reason this is not the detached-HEAD case one property over: on a + // zero-commit repository `git symbolic-ref` SUCCEEDS and returns the unborn branch, while + // `git rev-parse HEAD` fails. Reading only the first left `--repo-url` without `--rev` emitting + // a `resource` permalink on every code concept, each pointing into a branch that contains none + // of the files -- and the CLI's "no branch name could be read" note never fired to say so. A + // wrong link reads exactly as confidently as a right one, which §2.3 calls the worse outcome. + ProducerFixture.RequireGit(); + using var workspace = NewWorkspace(out var repo, out _); + + ProducerFixture.Git(repo, "init", "-q"); + + Assert.Null(GitRevision.CurrentBranch(repo)); + + // The other direction, so the assertion above cannot pass because the fixture simply has no + // working git: the SAME repository, one commit later, does report a branch. + ProducerFixture.Git(repo, "config", "user.email", "cli-tests@example.invalid"); + ProducerFixture.Git(repo, "config", "user.name", "CLI Tests"); + ProducerFixture.Git(repo, "config", "commit.gpgsign", "false"); + ProducerFixture.Git(repo, "add", "-A"); + ProducerFixture.Git(repo, "commit", "-q", "-m", "fixture"); + + Assert.NotNull(GitRevision.CurrentBranch(repo)); + } + [Fact] public void Force_is_an_alias_for_reset_and_not_a_flag_that_does_nothing() { From 1237991a67e7240e388b4f24ec81caf377c67f0a Mon Sep 17 00:00:00 2001 From: Julien CHABLE Date: Sun, 6 Sep 2026 11:02:18 +0200 Subject: [PATCH 11/36] fix(producer): refuse an offset that names no boundary, and an edge for a site nobody emitted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both are about the identity of a call site, and both had the same failure mode: a plausible WRONG answer rather than a missing one, which §2.1 calls the worse of the two because a wrong `## Calls` link reads exactly as confidently as a right one. **B1-7 -- off-boundary offsets were silently wrong, and inconsistently.** `Utf8Offsets` is the join key between two engines, and its class summary said both "only ever produce offsets that land on a codepoint boundary". That was an assumption, stated and unchecked, and each direction broke differently when it did not hold. `ToUtf8` ended its span on a lone high surrogate, which `GetByteCount` runs through the replacement fallback and scores as 3 bytes -- neither the 0 of the codepoint's start nor the 4 of its end. `ToUtf16` tested `utf8Count >= utf8Offset` and so ROUNDED UP: on `"A\U0001D11E"`, an offset one byte into the four-byte codepoint came back as the index after it, a real UTF-16 index for a position the caller never asked about. Both now refuse. The `>=` becoming `==` is the whole of the second fix, and the post-loop branch tells the two caller mistakes apart -- overshooting means inside a sequence, falling short means past the end -- so the message names the real one. The suite stayed green, which is the useful part of the result: no production path was relying on either rounding. **B1-4 -- the verdict dictionary took whatever a resolver handed back.** `verdicts[key] = edge` with no membership test, so an edge naming a site the extractor never emitted became a `## Calls` link attributed to a call that does not exist. `ISymbolResolver.Resolve`'s contract says a resolver answers the sites it was given; the dictionary is seeded from those sites, so membership IS that contract, and it is now enforced. Seeding switched from the indexer to `Add` for the same reason one property over: two sites sharing (path, offset) would be two calls at one byte, which the grammar cannot produce -- and if a future profile ever emits two, this throws rather than keeping whichever came last. The phantom test needed correcting before it discriminated, and the correction is recorded in it. Its first fixture gave the phantom an invented CALLER, so it was dropped by the pre-existing invariant that no edge may name a caller absent from `Symbols` -- machinery with nothing to do with this contract -- and the test passed with the guard removed. Measured. The phantom now differs from the real site only by offset, with a caller that really is declared, so only the new guard can stop it. 632 tests green, format clean. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0166CcRSdoZrL8g1Suruy6Uz --- .../CodeGraph/CodeGraphBuilder.cs | 22 +++++++++- .../OkfProducer.Core/CodeGraph/Utf8Offsets.cs | 34 +++++++++++++-- .../CodeGraph/CodeGraphBuilderTests.cs | 42 +++++++++++++++++++ .../CodeGraph/Utf8OffsetsTests.cs | 39 +++++++++++++++++ 4 files changed, 131 insertions(+), 6 deletions(-) diff --git a/producers/src/OkfProducer.Core/CodeGraph/CodeGraphBuilder.cs b/producers/src/OkfProducer.Core/CodeGraph/CodeGraphBuilder.cs index cd551d14..0cafc968 100644 --- a/producers/src/OkfProducer.Core/CodeGraph/CodeGraphBuilder.cs +++ b/producers/src/OkfProducer.Core/CodeGraph/CodeGraphBuilder.cs @@ -164,10 +164,18 @@ public CodeGraph Build(RepositorySnapshot snapshot, ExtractionLimits limits, Sco var sites = results.SelectMany(r => r.Result.Sites).ToList(); + // (path, offset) is a call site's identity -- the same key both engines match on -- so two + // sites sharing one would be two calls at the same byte, which the grammar cannot produce: + // an `invocation_expression`'s callee node starts where no other callee node starts. Seeded + // with `Add` rather than the indexer so that stops being an assumption: if a future grammar or + // profile ever emits two, this throws here instead of silently keeping whichever came last and + // dropping a call from the graph with nothing to show for it. var verdicts = new Dictionary<(string RelativePath, int Offset), ResolvedEdge>(); foreach (var site in sites) { - verdicts[(site.RelativePath, site.Offset)] = new ResolvedEdge(site, TargetContainer: null, TargetName: null, EdgeConfidence.Unresolved); + verdicts.Add( + (site.RelativePath, site.Offset), + new ResolvedEdge(site, TargetContainer: null, TargetName: null, EdgeConfidence.Unresolved)); } foreach (var resolver in resolvers) @@ -191,7 +199,17 @@ public CodeGraph Build(RepositorySnapshot snapshot, ExtractionLimits limits, Sco // are handled after the fact, by the degrade-to-Unresolved pass below. foreach (var edge in resolver.Resolve(ownedSites, declared)) { - verdicts[(edge.Site.RelativePath, edge.Site.Offset)] = edge; + // Only over a site the EXTRACTOR emitted. `verdicts[key] = edge` accepted anything a + // resolver handed back, so a resolver returning an edge for a site nobody extracted -- + // a stale offset, a file it read itself, an off-by-one -- added a phantom call to the + // graph, and a phantom edge is a `## Calls` link to a caller that does not exist. The + // dictionary is seeded from `sites` above, so membership IS the contract + // `ISymbolResolver.Resolve` states and never enforced: an edge answers a site it was + // given, or it is not an answer. + if (verdicts.ContainsKey((edge.Site.RelativePath, edge.Site.Offset))) + { + verdicts[(edge.Site.RelativePath, edge.Site.Offset)] = edge; + } } } diff --git a/producers/src/OkfProducer.Core/CodeGraph/Utf8Offsets.cs b/producers/src/OkfProducer.Core/CodeGraph/Utf8Offsets.cs index 398ed2d4..325fe195 100644 --- a/producers/src/OkfProducer.Core/CodeGraph/Utf8Offsets.cs +++ b/producers/src/OkfProducer.Core/CodeGraph/Utf8Offsets.cs @@ -57,6 +57,20 @@ public static int ToUtf8(string text, int utf16Offset) throw new ArgumentOutOfRangeException(nameof(utf16Offset), utf16Offset, "must be within the text."); } + // An offset splitting a surrogate pair is REFUSED rather than converted, because converting it + // silently returns a plausible wrong number: the span then ends on a lone high surrogate, + // which `GetByteCount` runs through the replacement fallback and scores as 3 bytes -- neither + // the 0 of the codepoint's start nor the 4 of its end. This type is a JOIN KEY between two + // engines, so a wrong number here does not lose a call site, it credits the call to whatever + // sits a few bytes away (§2.1). The class summary says both engines only ever produce + // boundary offsets; this is that assumption made checkable instead of merely stated. + if (utf16Offset > 0 && utf16Offset < text.Length + && char.IsHighSurrogate(text[utf16Offset - 1]) && char.IsLowSurrogate(text[utf16Offset])) + { + throw new ArgumentOutOfRangeException( + nameof(utf16Offset), utf16Offset, "splits a surrogate pair, so it names no codepoint boundary."); + } + return Encoding.UTF8.GetByteCount(text.AsSpan(0, utf16Offset)); } @@ -77,9 +91,14 @@ public static int ToUtf16(string text, int utf8Offset) var utf16Index = 0; var utf8Count = 0; + // `==`, not `>=`, and that one character is the whole fix. With `>=`, an offset landing INSIDE + // a multi-byte sequence silently rounded UP to the next boundary and returned it: on `"A\U0001D11E"` + // an offset of 2 -- one byte into the four-byte codepoint -- came back as the index after it. + // A plausible wrong number out of a join key credits a call to a neighbouring symbol rather + // than losing it, which §2.1 calls the worse of the two. foreach (var rune in text.EnumerateRunes()) { - if (utf8Count >= utf8Offset) + if (utf8Count == utf8Offset) { return utf16Index; } @@ -88,11 +107,18 @@ public static int ToUtf16(string text, int utf8Offset) utf16Index += rune.Utf16SequenceLength; } - if (utf8Count != utf8Offset) + if (utf8Count == utf8Offset) { - throw new ArgumentOutOfRangeException(nameof(utf8Offset), utf8Offset, "beyond the end of the text."); + return utf16Index; } - return utf16Index; + // Overshooting means the offset fell inside a sequence; falling short means it is past the end. + // Two different mistakes on the caller's side, told apart so the message names the real one. + throw new ArgumentOutOfRangeException( + nameof(utf8Offset), + utf8Offset, + utf8Count < utf8Offset + ? "beyond the end of the text." + : "does not land on a codepoint boundary."); } } diff --git a/producers/tests/OkfProducer.Tests/CodeGraph/CodeGraphBuilderTests.cs b/producers/tests/OkfProducer.Tests/CodeGraph/CodeGraphBuilderTests.cs index 58a9f2c4..41a569da 100644 --- a/producers/tests/OkfProducer.Tests/CodeGraph/CodeGraphBuilderTests.cs +++ b/producers/tests/OkfProducer.Tests/CodeGraph/CodeGraphBuilderTests.cs @@ -295,4 +295,46 @@ private static RepositorySnapshot SnapshotWith(params string[] relativePaths) return new RepositorySnapshot(repoPath, "test-repo", [], []); } + + /// Answers a site nobody gave it, which is precisely what 's contract forbids. + private sealed class PhantomResolver(CallSite phantom) : ISymbolResolver + { + public bool Owns(string relativePath) => true; + + public IReadOnlyList Resolve(IReadOnlyList sites, IReadOnlyList symbols) => + [new ResolvedEdge(phantom, "T", phantom.CalledName, EdgeConfidence.Exact)]; + } + + [Fact] + public void An_edge_for_a_site_the_extractor_never_emitted_is_dropped() + { + // The verdict dictionary used to take whatever a resolver handed back -- `verdicts[key] = edge` + // with no membership test -- so an edge naming a site nobody extracted was added to the graph. + // A stale offset, a file the resolver read for itself, an off-by-one in an engine's own + // conversion: any of them produced a `## Calls` link attributed to a caller that does not exist, + // and the contract Resolve states ("answer the sites you were given") was enforced nowhere. + // + // The phantom differs from the real site only by OFFSET, and its caller is a REAL declared + // symbol -- both deliberate. An unknown caller is already filtered later, by the invariant that + // no edge may name a caller absent from Symbols, so a phantom with a made-up caller would be + // caught by machinery that has nothing to do with this contract and the test would pass over a + // missing guard. Measured: the first version of this fixture did exactly that. + // Offset is also the identity half an engine is most likely to get wrong, and the half no + // name-based assertion would notice. + var real = new CallSite("T", "Caller", "Callee", "A.cs", 42); + var phantom = new CallSite("T", "Caller", "Callee", "A.cs", 999); + + var builder = new CodeGraphBuilder( + new StubExtractor(Member("T", "Caller"), Member("T", "Callee")) { Sites = [real] }, + CSharpProfiles, + [new PhantomResolver(phantom)]); + + var graph = builder.Build(SnapshotWith("A.cs"), ExtractionLimits.Default, ScopeOptions.Default); + + // The real site is still there, unresolved -- the phantom did not displace it either. + var edge = Assert.Single(graph.Edges); + Assert.Equal(42, edge.Site.Offset); + Assert.Equal("Caller", edge.Site.CallerName); + Assert.Equal(EdgeConfidence.Unresolved, edge.Confidence); + } } diff --git a/producers/tests/OkfProducer.Tests/CodeGraph/Utf8OffsetsTests.cs b/producers/tests/OkfProducer.Tests/CodeGraph/Utf8OffsetsTests.cs index 64b336c0..e82dad5c 100644 --- a/producers/tests/OkfProducer.Tests/CodeGraph/Utf8OffsetsTests.cs +++ b/producers/tests/OkfProducer.Tests/CodeGraph/Utf8OffsetsTests.cs @@ -115,4 +115,43 @@ public void ToUtf16_rejects_an_offset_past_the_end_of_the_text() { Assert.Throws(() => Utf8Offsets.ToUtf16("abc", 4)); } + + [Fact] + public void An_offset_that_splits_a_surrogate_pair_is_refused_rather_than_converted() + { + // The failure mode this type exists to prevent is a PLAUSIBLE WRONG NUMBER, not a missing one: + // it is the join key between two engines, so a bad conversion credits a call to whatever sits a + // few bytes away rather than dropping it (§2.1). + // + // Measured before the guard: `GetByteCount` over a span ending on a lone high surrogate runs it + // through the replacement fallback and scores 3 bytes -- neither the 0 of the codepoint's start + // nor the 4 of its end. Exactly the shape of answer that looks usable and is not. + const string Text = "A\U0001D11EB"; // A, a four-byte musical symbol (two UTF-16 units), B + + Assert.Equal(1, Utf8Offsets.ToUtf8(Text, 1)); // the boundary before the pair + Assert.Equal(5, Utf8Offsets.ToUtf8(Text, 3)); // the boundary after it + + Assert.Throws(() => Utf8Offsets.ToUtf8(Text, 2)); + } + + [Fact] + public void A_utf8_offset_inside_a_sequence_is_refused_rather_than_rounded_up() + { + // The other direction, and the one that was silently wrong rather than merely undefined: the + // walk tested `utf8Count >= utf8Offset`, so an offset one byte into the four-byte codepoint + // returned the index AFTER it -- a real UTF-16 index, for a different position than the caller + // asked about. + const string Text = "A\U0001D11EB"; + + Assert.Equal(1, Utf8Offsets.ToUtf16(Text, 1)); // the boundary before the codepoint + Assert.Equal(3, Utf8Offsets.ToUtf16(Text, 5)); // the boundary after it + + foreach (var inside in new[] { 2, 3, 4 }) + { + Assert.Throws(() => Utf8Offsets.ToUtf16(Text, inside)); + } + + // And the two mistakes are still told apart, so the guard did not swallow the pre-existing one. + Assert.Throws(() => Utf8Offsets.ToUtf16(Text, 99)); + } } From 0f67010749201f18f716c6135540c0e4d2157bf5 Mon Sep 17 00:00:00 2001 From: Julien CHABLE Date: Sun, 6 Sep 2026 12:56:21 +0200 Subject: [PATCH 12/36] docs(producer): pin the indexer/operator ripple, and say why test projects are still queried MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the behaviour lot of the Minor register. Neither of these changes behaviour, and both say so: one had a right outcome reached by two mechanisms that did not mention each other, and the other was read as an inconsistency when the code has a reason. **B2-6 -- the ripple of an accepted gap, now pinned.** Indexers, operator overloads and conversion operators are not extracted, deliberately: none has a `name` field in this grammar. What was never stated is what happens to a CALL inside one -- it finds no ancestor in `CallerMemberAncestorNodeTypes`, so the site comes out with an empty caller, and an edge naming a caller that has no concept is what §2.1 calls worse than no edge. `CodeGraphBuilder`'s invariant removes it. Right outcome, reached by two halves neither of which referenced the other, so a change to either would have let the orphan through unnoticed. The test pins both halves AND the gap itself, so it fails loudly rather than vacuously if the profile ever starts extracting those members -- at which point the ripple stops being the right behaviour. **INT-M7 -- test projects are queried with `--include-tests` off, and should be.** The register filed this as an inconsistency. It is not: the flag decides which FILES are extracted, while this list decides which projects are asked for their `Compile` item sets, and §5.1's ownership map is built from those. A test project can legitimately claim a PRODUCTION file -- a linked `` is ordinary -- and dropping it would silently remove that file's `## Also compiled by` entry, which is a fact about the production file, not about the test project. The cost is stated rather than hidden: one `dotnet msbuild` per test project on every run, for item sets whose own files the run will not extract. Measured on this repository, no test project claims a production file, so skipping them would change nothing in the emitted bundle here -- the price buys correctness in the general case, not in this one. If you would rather trade that for the subprocess time, the filter is one `Where` and I have deliberately not added it. 633 tests green, format clean. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0166CcRSdoZrL8g1Suruy6Uz --- producers/src/OkfProducer.Cli/GenerateRun.cs | 20 ++++++++++ .../Profiles/CSharpProfile.cs | 13 +++++- .../CodeGraph/TreeSitterExtractorTests.cs | 40 +++++++++++++++++++ 3 files changed, 72 insertions(+), 1 deletion(-) diff --git a/producers/src/OkfProducer.Cli/GenerateRun.cs b/producers/src/OkfProducer.Cli/GenerateRun.cs index d1a0a2ac..63ddeb40 100644 --- a/producers/src/OkfProducer.Cli/GenerateRun.cs +++ b/producers/src/OkfProducer.Cli/GenerateRun.cs @@ -609,6 +609,26 @@ private static bool IsOwned(string id) => /// family is generated from, so the ownership map's join key is by construction the one /// ConceptGenerator looks a package up by. /// + /// + /// Every nuget project MSBuild will be asked about -- including test projects, and including + /// them when --include-tests is off. + /// + /// That reads as an inconsistency and is not one, because the flag and this list govern + /// different things. --include-tests decides which FILES are extracted + /// (FileEligibility.IsEligible); this decides which projects are asked for their + /// Compile item sets, and §5.1's ownership map is built from those. A test project can + /// legitimately claim a PRODUCTION file -- a linked <Compile Include="..\..\src\Shared.cs"/> + /// is ordinary -- and dropping it here would silently remove that file's + /// ## Also compiled by entry, which is a fact about the production file rather than about + /// the test project. Narrowing the query would make the map answer a different question from the + /// one it is documented to answer. + /// + /// The cost, stated because it is real: one dotnet msbuild subprocess per test + /// project on every run, roughly a second each here, for item sets whose own files this run will + /// not extract. Measured on this repository, no test project claims a production file, so nothing + /// in the emitted bundle would change if they were skipped -- the price buys correctness in the + /// general case, not in this one. + /// private static IReadOnlyList CSharpProjectPaths(RepositorySnapshot snapshot) => [ .. snapshot.Packages diff --git a/producers/src/OkfProducer.CodeGraph.TreeSitter/Profiles/CSharpProfile.cs b/producers/src/OkfProducer.CodeGraph.TreeSitter/Profiles/CSharpProfile.cs index 1da4f10e..b68c1d7c 100644 --- a/producers/src/OkfProducer.CodeGraph.TreeSitter/Profiles/CSharpProfile.cs +++ b/producers/src/OkfProducer.CodeGraph.TreeSitter/Profiles/CSharpProfile.cs @@ -35,7 +35,18 @@ public static class CSharpProfile /// immediately after the anonymous operator keyword node, not a field. Naming either /// requires bespoke, untested-by-any-brief-requirement logic (walk to the child after the /// keyword; synthesize something like this[]), so this profile accepts the coverage gap - /// rather than add unverified naming rules. + /// rather than add unverified naming rules. + /// + /// What that gap ripples into, which this list used not to say. A CALL inside one of + /// them finds no ancestor in TreeSitterExtractor.CallerMemberAncestorNodeTypes either, so + /// the site is emitted with an empty caller. Nothing downstream can hang a ## Calls entry + /// on a caller that has no concept, and an edge naming one is exactly what §2.1 calls worse than + /// no edge -- so CodeGraphBuilder's invariant, that no edge may name a caller absent from + /// Symbols, is what removes it. The outcome is right; it was reached by two mechanisms + /// neither of which mentioned the other. + /// TreeSitterExtractorTests.A_call_inside_an_indexer_or_an_operator_yields_no_edge_at_all + /// pins both halves, and asserts the gap itself, so it fails loudly rather than vacuously if this + /// profile ever starts extracting them. /// /// public const string DeclarationQuery = """ diff --git a/producers/tests/OkfProducer.Tests/CodeGraph/TreeSitterExtractorTests.cs b/producers/tests/OkfProducer.Tests/CodeGraph/TreeSitterExtractorTests.cs index 3ffc70c1..5b894854 100644 --- a/producers/tests/OkfProducer.Tests/CodeGraph/TreeSitterExtractorTests.cs +++ b/producers/tests/OkfProducer.Tests/CodeGraph/TreeSitterExtractorTests.cs @@ -558,6 +558,46 @@ void IFoo.Bar() { } Assert.Equal(["Bar", "IFoo.Bar"], names); } + [Fact] + public void A_call_inside_an_indexer_or_an_operator_yields_no_edge_at_all() + { + // The documented gap: indexers, operator overloads and conversion operators are not extracted + // as symbols, because none of them has a `name` field in this grammar (an indexer is written + // `this[...]`; an operator's symbol is an anonymous child after the anonymous keyword). Naming + // them would need bespoke, unverified rules, so the profile accepts the gap. + // + // What that gap RIPPLES into was never stated or tested: a call inside one of those members + // finds no ancestor in CallerMemberAncestorNodeTypes, so the site comes out with an empty + // caller. An edge naming a caller that does not exist is exactly what §2.1 calls worse than no + // edge -- so what must be true is that NONE survives, and that is what this pins. + // + // It is not the extractor that drops it: the site is emitted with an empty caller and + // CodeGraphBuilder's invariant (no edge may name a caller absent from Symbols) is what removes + // it. Stated here because a future change to either half would silently let the orphan through. + var result = ExtractSource(""" + namespace N; + public class T + { + public void Target() { } + + public int this[int i] { get { Target(); return i; } } + + public static T operator +(T a, T b) { a.Target(); return a; } + } + """); + + // The gap itself, asserted so this test fails loudly rather than vacuously if the profile ever + // starts extracting them -- at which point the ripple below stops being the right behaviour. + Assert.DoesNotContain(result.Symbols, s => s.Name is "this" or "+" or "op_Addition"); + + // Every call site found inside those two members carries no caller to hang a concept off. + Assert.All( + result.Sites.Where(s => s.CalledName == "Target"), + site => Assert.True( + site.CallerName.Length == 0, + $"expected no caller for a call inside an indexer or operator, got '{site.CallerName}'.")); + } + private ExtractionResult ExtractSource(string source, string relativePath = "T.cs") { var directory = Directory.CreateTempSubdirectory("okfproducer-treesitter-").FullName; From 364cbc307ab335c65f0d6cbfa372a85b7f9e09a0 Mon Sep 17 00:00:00 2001 From: Julien CHABLE Date: Sun, 6 Sep 2026 13:03:05 +0200 Subject: [PATCH 13/36] fix(producer): a code block's content is source, and stops being flattened into a description D1b-M3. `IDescriptionSource.Describe` documents its `Text` as "a complete sentence, not a fragment", and a `` block's content flowed straight through it: a multi-line example arrived at `CollapseWhitespaceRuns` and came out as `var a = 1; var b = 2;` reading like the tail of a sentence. Neither the code nor a description, and it ships in the bundle. `` is now an opaque tag whose CONTENT is dropped -- the same argument the viewer's sanitizer makes for `