Skip to content

Cache plain generation results in the incremental pipeline - #40

Merged
skuirrels merged 2 commits into
mainfrom
fix/incremental-pipeline-and-review-fixes
Sep 5, 2026
Merged

Cache plain generation results in the incremental pipeline#40
skuirrels merged 2 commits into
mainfrom
fix/incremental-pipeline-and-review-fixes

Conversation

@skuirrels

Copy link
Copy Markdown
Owner

Description

Authored with Claude Code from a code review of the repository; every finding below was verified locally before being fixed.

Incremental pipeline. The cached pipeline value previously held the mapper INamedTypeSymbol and the Compilation, compared only by a content fingerprint. Because the driver keeps the previous value when the comparer reports equality, each cached mapper retained the compilation it was last generated from, and the fingerprint could miss dependencies (a member on an inherited interface of the source was not observed). The transform now runs MapperCompiler and yields a plain MapperGenerationResult (hint name, source text, DiagnosticData), so the driver compares emitted output and re-adds only mappers whose output changed. A reflection test proves the cached value references no compilation, symbol, tree, location or diagnostic.

Incremental benchmark (short job, 64 mappers): NoOpCore unchanged at 13.9 µs; IsolatedContractEdit 290 µs → 659 µs because every mapper now regenerates inside the transform; SharedContractEdit 2296 µs → 1878 µs because the fingerprint walk is gone.

Engine. MapperCompiler is split into partial-class files by responsibility. Attributes are matched by symbol through GetTypesByMetadataName instead of display-string comparison. Deferred construction is planned through CreationPlan instead of a base64 text marker. Generated members are stamped with the generator assembly version instead of a fixed 0.0.1.0.

Versions and CI. Adds the missing 1.2.1 changelog entry, pins the README to 1.2.1, raises PackageValidationBaselineVersion to 1.2.1, and derives the development version in test.yml, package.sh, docs.yml and the Docusaurus config from Directory.Build.props. Removes the rule 52 exclusion from package validation now that the repository is public, and drops dead configuration and unused test packages.

Tests. 140 tests (from 120): incremental regeneration for inherited interface members, output-neutral edits reporting Unchanged, pipeline value purity, rehydrated diagnostic locations, assembly version stamp, symbol attribute matching, creation ordering, five Verify full-file snapshots, and repository consistency guards for versions.

Fixes # (issue)

Checklist

  • I did not use AI tools to generate this PR, or I have manually verified that the code is correct, optimal, and follows the project guidelines and architecture
  • I understand that low-quality, AI-generated PRs will be closed immediately without further explanation
  • The existing code style is followed
  • The commit message follows our guidelines
  • Performed a self-review of my code
  • Hard-to-understand areas of my code are commented
  • The documentation is updated (as applicable)
  • Unit tests are added/updated
  • Integration tests are added/updated (as applicable, especially if feature/bug depends on roslyn or framework version in use)

🤖 Generated with Claude Code

- Run MapperCompiler inside the ForAttributeWithMetadataName transform and
  cache only emitted source and diagnostic data, so cached mappers keep no
  compilation or symbols alive and every contract edit is observed
- Split MapperCompiler into partial-class files by responsibility
- Match mapping attributes by symbol, stamp generated members with the
  assembly version, and plan deferred construction structurally
- Align versions: add the 1.2.1 changelog entry, pin README to 1.2.1, raise
  the package validation baseline, and derive CI and script versions from
  Directory.Build.props
- Add incremental, pipeline-value, snapshot and repository-consistency tests
@skuirrels skuirrels added bug Something isn't working enhancement New feature or request labels Sep 3, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'ComparisonMappingBenchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.25.

Benchmark suite Current: 280d0f5 Previous: 187ef20 Ratio
DomainMapper.Benchmarks.ComparisonMappingBenchmarks.DomainMapperFlat 11.87693651765585 ns (± 0.3271875392584718) 7.598527964949608 ns (± 0.11638510408811246) 1.56
DomainMapper.Benchmarks.ComparisonMappingBenchmarks.DomainMapperRenamedFlattened 9.483548912812363 ns (± 0.28219199013618357) 7.177408418485096 ns (± 0.07761475547820171) 1.32
DomainMapper.Benchmarks.ComparisonMappingBenchmarks.DomainMapperValueObjectFactory 6.90919190744559 ns (± 0.16355923549592302) 4.894221918547855 ns (± 0.15484899538459032) 1.41
DomainMapper.Benchmarks.ComparisonMappingBenchmarks.MapperlyValueObjectFactory 7.574208869536718 ns (± 0.09981800287601893) 5.710719750435264 ns (± 0.2272405772790416) 1.33

This comment was automatically generated by workflow using github-action-benchmark.

@skuirrels
skuirrels merged commit 4a3d1b0 into main Sep 5, 2026
20 of 24 checks passed
@skuirrels
skuirrels deleted the fix/incremental-pipeline-and-review-fixes branch September 5, 2026 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant