docs: strip self-assessment from comments added since 437e6324 - #272
Merged
Conversation
Comments merged over the last 113 commits accumulated a habit of reporting on the work rather than on the code: "verified live", "load-bearing", "Regression-shaped proof that", "MUTATION TESTED". That prose tells a reviewer the author was diligent. It tells the next maintainer nothing, and it buries the facts that are actually load bearing -- which PostgreSQL versions were checked, what the server returns, why a given query shape makes a bug bite. Every such phrase either carried a fact or it did not. Where it did, the fact stays and the self-assessment goes: "Verified live on PostgreSQL 16, 17, and 18: concat_ws(NULL,'x','y') IS NULL" becomes "PostgreSQL 16-18: concat_ws(NULL,'x','y') is NULL". Where it did not, the sentence goes. Also removed: emphasis-caps on ordinary English words, internal ticket references (#212, #219, #223, #238, #239, #240) and a PLAN-240.md pointer that will not resolve for anyone reading this later, and comments restating the line below them. Kept deliberately: intent markers whose reason is not recoverable from the code, since they guard against a future "fix"; the per-function audit trail in NeverNullSafeLists, which is a maintained list backed by a live sweep; full KDoc contract text on runtime's public API; and pgjdbc source citations, which name a specific method as evidence. Comment text only -- no code line added, removed, or reordered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Marius Volkhart (MariusVolkhart)
requested review from
Matthew Foster (MatthewFoster624) and
Ryan LuMaye (RyanLuMaye)
as code owners
September 5, 2026 11:45
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Comments merged over the last 113 commits accumulated a habit of reporting on the work rather than on the code: "verified live", "load-bearing", "Regression-shaped proof that", "MUTATION TESTED". That prose tells a reviewer the author was diligent. It tells the next maintainer nothing, and it buries the facts that actually matter — which PostgreSQL versions were checked, what the server returns, why a given query shape makes a bug bite.
Every such phrase either carried a fact or it did not. Where it did, the fact stays and the self-assessment goes:
Verified live on PostgreSQL 16, 17, and 18: concat_ws(NULL,'x','y') IS NULL→PostgreSQL 16-18: concat_ws(NULL,'x','y') is NULL63-byte budget -- verified against the live server in JdbcAnalyzerTest→63-byte budget (PostgreSQL's NAMEDATALEN - 1 limit)MUTATION TESTED: ... was run by hand against this test and confirmed to turn ... red ... before being reverted back to the fix.→Reverting [...] fails this test on the chainDepth=3, shadowed=true cases: the emitted expression evaluates to the shadow's LOWER(description) value instead of the real chain's UPPER(name) value.Where the phrase carried nothing, the sentence goes.
Also removed: emphasis-caps on ordinary English words, internal ticket references (#212, #219, #223, #238, #239, #240), and two
PLAN-240.mdpointers to a file that has never existed in this repository.Kept deliberately:
NeverNullSafeLists, a maintained list backed by a live sweep.runtime's public API, which users read in their IDE.Scope was comments added or modified since 437e632. Pre-range comments were left alone even where they read badly.
Comment text only — no code line added, removed, or reordered. This was proved mechanically rather than by inspection: a Kotlin-aware tokenizer stripped comments from the before and after of all 57 changed files, preserving raw strings, escaped strings, and nested block comments, and diffed the results byte-identical. No code, string literal, annotation, or test name moved.
./gradlew :generator:check :gradle-plugin:check :runtime:checkpasses. Golden files unchanged.Two verification rounds each found real defects, both since fixed. The first caught six places where a fact had been deleted rather than shortened — most seriously,
SqlLexer'sstripCommentsKDoc lost the clause explaining thatskipLineCommentconsumes the comment's own trailing newline, without which a reader concludes theappend(' ')is redundant and removable, fusingdandWHEREintodWHERE. The second confirmed those fixes and caught thePLAN-240.mdreferences.One thing reviewers should know: the
buildBatchKDoc inImplementationBuilder.ktwas flagged during this work as contradicting its code. It does not — "discarding an intermediate flush ... leavesresultsempty" is a hypothetical explaining why the code captures every flush, not a claim about current behavior. It is correct but ambiguous enough to be misread, and it predates this branch. Left untouched.🤖 Generated with Claude Code