Skip to content

fix: correctness sweep for latent emission and pipeline defects - #48

Merged
kamronbatman merged 1 commit into
mainfrom
kb/correctness_sweep
Aug 22, 2026
Merged

fix: correctness sweep for latent emission and pipeline defects#48
kamronbatman merged 1 commit into
mainfrom
kb/correctness_sweep

Conversation

@kamronbatman

Copy link
Copy Markdown
Member

Summary

Fixes every latent defect surfaced by #47's compile gate, plus the pipeline hygiene issues found during the incrementality analysis. Each fix is pinned by a new snapshot fixture or diagnostic test that failed before the fix (6 red tests → all green).

Fixes

  • MarkDirty()() emission: an instance MarkDirty() on the serialized class emitted ClassName.MarkDirty()() (ToDisplayString() already includes qualification and parens). Now emits the method name with the proper receiver. Pinned by the CustomMarkDirty fixture.
  • KeyValuePair fields (three defects, KeyValuePairField fixture):
    • setters compared with a nonexistent != — setters now fall back to EqualityComparer<T>.Default.Equals for types without an inequality operator (with Nullable<T> operator lifting respected, so int? fields are unchanged);
    • deserialization assigned to undeclared key/value locals — now declared with unique names so multiple pair fields can't collide;
    • a bogus ClearX() calling KVP.Clear() was emitted — data-structure methods now require an actual array/collection/dictionary type.
  • Struct emission: Serialize/Deserialize were marked virtual (CS0106). Fixing that exposed that the SG3009 contract was unsatisfiable: it required a user-declared Deserialize, which always collides with the generated one (both flavors, CS0111), while the migration rules consume the generated instance method. SG3009 is inverted: it now fires when a value type declares a conflicting Deserialize. The Structs fixture's KnownBroken marker is gone.
  • Unqualified Server-namespace references: migration content structs (IGenericReader) and timer emission (Core.Now) only compiled for classes under Server.*. Both fully qualified now — pinned by the new NonServerNamespace fixture, which generates a versioned class with a timer outside Server.* and must compile.
  • Thread safety: the generator's Migrations map is mutated from source-output callbacks that can run concurrently — now a ConcurrentDictionary. Also removed the no-op CultureInfo set/restore in Initialize.
  • New diagnostics: SG3011 (duplicate migration file for the same class+version was silently ignored — data-loss hazard with nested migration folders) and SG3012 (migration file above the current version is never read — leftover from a rolled-back bump). The file at the current version is the schema record the migration tool maintains and is not flagged; warnings now survive successful generation instead of being dropped.

Proof

  • Suite green: 83/83 (6 new red tests flipped by the fixes).
  • DiffTool manifest vs. main across a 3,786-source real corpus: exactly 25 hashes change — all migration content structs and timer classes picking up qualified names — and zero diagnostics fire, confirming no real-world duplicate or stale migration files and no unintended output drift. (An earlier draft of SG3012 used >= and lit up the entire corpus — the manifest run caught it immediately.)

🤖 Generated with Claude Code

All caught by the snapshot corpus's compile gate and pinned by new
fixtures or diagnostic tests:

- An instance MarkDirty() on the serialized class emitted
  ClassName.MarkDirty()() via ToDisplayString plus appended parens; use
  the method name with the proper receiver.
- KeyValuePair fields: the setter compared with a nonexistent != (setters
  now fall back to EqualityComparer<T> when the type has no inequality
  operator, with Nullable<T> lifting respected), deserialization assigned
  to undeclared key/value locals (now declared, uniquely named), and a
  bogus ClearX() calling KVP.Clear() was emitted (data-structure methods
  now require an actual collection type).
- Struct/record-struct emission marked Serialize/Deserialize virtual
  (CS0106). With that fixed, the SG3009 contract was inverted and shown to
  be unsatisfiable: it demanded a user Deserialize that always collided
  with the generated one, while rules consume the generated instance
  method. SG3009 now fires on a user-declared Deserialize instead.
- Migration content structs and timer emission referenced IGenericReader
  and Core unqualified, compiling only for classes under Server.*; both
  are fully qualified now.
- The generator's Migrations map is mutated from source-output callbacks
  that can run concurrently; now a ConcurrentDictionary. Removed the no-op
  CultureInfo set/restore in Initialize.
- Duplicate migration files for a class and version were silently ignored
  (SG3011) and files above the current version were dead data (SG3012);
  both are reported now, and warnings survive successful generation
  instead of being dropped. The file at the current version is the schema
  record and is not flagged.

Corpus impact: 25 of 3,786 real-corpus sources change, all migration
content structs and timer classes picking up qualified names; zero
diagnostics fire on the real corpus.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@kamronbatman
kamronbatman merged commit 483adfa into main Aug 22, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant