Skip to content

Let codegen use the code generator rather than a copy of one - #155

Merged
matt-edmondson merged 3 commits into
mainfrom
claude/holotype-cpp-schema-integration-8nzvut
Sep 11, 2026
Merged

Let codegen use the code generator rather than a copy of one#155
matt-edmondson merged 3 commits into
mainfrom
claude/holotype-cpp-schema-integration-8nzvut

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

KtsuTools.CodeGen/CodeGenService.cs was 456 lines, and about 400 of them were a second, smaller ktsu.Coder:

In this module In the package
IAstNode, FunctionDeclaration, ParameterNode, VariableDeclaration, ReturnStatement the same, plus ~25 more node types
ILanguageGenerator, CSharpGenerator, PythonGenerator the same, plus C++ and JavaScript
a hand-written YAML reader for one node shape YamlDeserializer, and a serializer back
four tests of the two emitters a 496-test suite

Two implementations of one idea, and the smaller one was the one nobody was improving. The module is net10.0 and the package is net10.0;net9.0, so there was never a targeting reason for it.

What this does

Replaces the body with a package reference and keeps what a command-line front end is actually for: finding the file, choosing the generator, putting the result somewhere. CodeGenService.GenerateAsync keeps its signature, so CodeGenCommand is untouched.

Two things get better as a side effect. codegen --lang cpp and --lang javascript now exist. And the command reads any node Coder can read rather than only a functionDeclaration — a whole source file, a namespace, a class — because the reader is no longer a parser for one shape.

No format break. Coder's reader expects the same keys the hand-written one did (functionDeclarationname, returnType, parameters, body), so a document that worked still works. There is a test that reads one.

Tests

They follow the same split. They no longer assert what a generator emits — that package tests its own across four languages far more thoroughly than a copy here did. What is left to test is the part this module still owns: that a document reaches a generator, that the right one is chosen, that a document describing nothing is reported rather than thrown out of a command, and one end-to-end run producing C++ from a file.

The other commit

main does not currently build. Dependabot moved ktsu.Semantics.Paths to 4.0.0 in #153 and left ktsu.Semantics.Strings at 3.2.1; Paths 4.0.0 depends on Strings >= 4.0.0, so central package management pins below what the graph requires and restore fails with NU1605: Detected package downgrade. Raising the pin is the whole fix. It is unrelated to the change beside it and would have been its own pull request, except nothing here builds until it lands — so it is the first commit, and each commit builds on its own.

Testing

dotnet build clean, 0 warnings. 64/64 tests pass.

🤖 Generated with Claude Code

https://claude.ai/code/session_0165XQS7s11ssp36AEY8tvCF


Generated by Claude Code

[patch]

Dependabot moved ktsu.Semantics.Paths to 4.0.0 and left ktsu.Semantics.Strings
at 3.2.1. Paths 4.0.0 depends on Strings >= 4.0.0, so central package management
now pins a version below what the graph requires and every project referencing
either fails to restore with NU1605: "Detected package downgrade". main does not
build.

Raising the pin to match is the whole fix. It is unrelated to the change after
it and would have been its own pull request, except that nothing in this
repository builds until it lands.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0165XQS7s11ssp36AEY8tvCF
[minor]

KtsuTools.CodeGen carried its own IAstNode, five node types, a hand-written YAML
reader and a C# and a Python emitter - 456 lines, every one of which ktsu.Coder
already had, along with C++, JavaScript, a round trip back to YAML, a node-graph
editor and a test suite. Two implementations of one idea, and the smaller one
was the one nobody was improving.

So the module now references that package and keeps what a command-line front
end is actually for: finding the file, choosing the generator, and putting the
result somewhere. It gains C++ and JavaScript, and it gains every node Coder can
read rather than only a function declaration - the YAML shape a caller already
wrote is the shape Coder's own reader expects, so a document that worked still
works.

The tests follow the same split. They no longer check what a generator emits,
because that package checks its own across four languages far more thoroughly
than a copy here did; what is left to test is that a document reaches a
generator, that the right one is chosen, and that a document describing nothing
is reported rather than thrown out of a command.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0165XQS7s11ssp36AEY8tvCF
[patch]

SonarCloud reported new issues on the rewritten file, and reproducing its
analyzers locally found one of them in code this change touches: S2325, "make
GenerateAsync a static method".

It is the same complaint CA1822 makes, which the method already carries a
suppression for, and the answer is the same: CodeGenService is a singleton
injected into CodeGenCommand's constructor, so a static method would leave the
command holding a dependency it takes and never uses. The suppression now names
both rules, and the reason moves into the documentation where a reader will find
it rather than sitting above the pragma.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0165XQS7s11ssp36AEY8tvCF

Copy link
Copy Markdown
Contributor Author

Acting on the SonarCloud comment above — with one gap worth naming, since I could not close it from here.

This repository has no .sonarlint/ local opt-in (the way Semantics, CodeBlocker and KtsuBuild do), so Sonar's rules do not run on a plain dotnet build and its findings are invisible until a push. I reproduced them by pointing CustomAfterMicrosoftCommonProps at a scratch props file adding SonarAnalyzer.CSharp, outside the repository so nothing here changes:

dotnet build KtsuTools.CodeGen/KtsuTools.CodeGen.csproj \
  -p:CustomAfterMicrosoftCommonProps=/path/to/sonar-local.props

That found one issue in the files this PR changes:

CodeGenService.cs(57,25): S2325: Make 'GenerateAsync' a static method.

It is the same complaint CA1822 makes, and the method already carried a suppression for that one — CodeGenService is a singleton injected into CodeGenCommand's constructor, so a static method would leave the command holding a dependency it takes and never uses. The suppression now names both rules, and the reason moved into the XML documentation where a reader will find it. Pushed in b69b8cc.

Four of the five I cannot see. The analyzer package ships some rules disabled that a project's quality profile enables, and sonarcloud.io is not reachable from this environment, so there is no way from here to tell whether the remaining four are in CodeGenService.cs, in the rewritten tests, or are rules my local run never enabled. The gate passed and every check is green, so nothing is blocked — but if you have dashboard access, the four are worth a glance before merging.

Worth noting separately: the analyzers report a good number of pre-existing findings across BuildMonitor, FileDedupe, MemFrag, Merge, Project and SyncS108, S2325, S3267, S6966. None are in this diff and I have not touched them. Adding the .sonarlint/ opt-in so they are visible locally would be its own change.


Generated by Claude Code

@sonarqubecloud

Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor Author

Correcting my previous comment: S2325 was not one of the five.

The re-analysis after b69b8cc still reports 5 new issues, unchanged. So either that rule was never among them and my local run simply enables rules this project's quality profile does not, or the scanner does not count a #pragma suppression the way a local build does — and there is no way to tell which from here.

The suppression stands on its own merits: CA1822 was already suppressed on that method for exactly the same reason, and naming S2325 alongside it keeps a local analyzer run clean. But it did not reduce the count, and my earlier comment implied it addressed one of the reported five. It did not.

So the position is: all five remain unidentified from this environment, not four. Everything is green and the gate passes, so nothing is blocked — but the dashboard is the only place those five can actually be read.


Generated by Claude Code

@matt-edmondson
matt-edmondson merged commit f0d136c into main Sep 11, 2026
12 checks passed
@matt-edmondson
matt-edmondson deleted the claude/holotype-cpp-schema-integration-8nzvut branch September 11, 2026 09:10
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.

2 participants