Skip to content

[ConfigManager] Register Node Sections 4/4 - #3995

Open
bdchatham wants to merge 31 commits into
plt-775-node-sections-3from
plt-775-node-sections-4
Open

[ConfigManager] Register Node Sections 4/4#3995
bdchatham wants to merge 31 commits into
plt-775-node-sections-3from
plt-775-node-sections-4

Conversation

@bdchatham

@bdchatham bdchatham commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Declares the keys at the top of the node's configuration file, with no table of their own: 11
keys. With this the binary declares 27 sections and 271 keys across both configuration files,
with no refused registration.

Why a schema rather than the node's own type

That type carries the root keys and every table, so declaring against it would declare each
table's keys a second time. The schema squashes the same base group that type squashes, so the
spellings still come from the node's own tags, and restates only the fields it holds beside that
group. Each of those is held against the type it came from by name, tag and type, with the
schema's own field count holding the set, so a further one appearing there fails rather than
going undeclared.

A root field carrying no mapstructure tag fails too. The reader falls back to the field's own
name, so such a field is a key an operator can write under that spelling which no section
declares — and the walk used to skip it while its doc said it fails.

What is left out, and why

home is the file's own location, which the command line carries. mode is the fact the file
states at the top under its own name; declaring a second spelling would let an operator write
one kind of node in the file and resolve another. A generated file does state the node kind, so
a reader handed a decoded file reports it beside an operator's typos, and exempting that one key
belongs to the reader rather than to this declaration.

Three more name settings the node removed: the address of an out-of-process application it no
longer runs, the transport to that application, and peer filtering through it. Nothing in the
tree reads any of them and the generated file writes none. Two state an affirmative value in
the node's own defaults, an address and a transport name, so declaring them put settings for a
transport the binary no longer has into the space a writer renders from.

One key that is not what a generated file carries

The node name. The command takes it as a required argument, so a generated file always carries
an operator's own, and the declaration answers with the hostname of whatever machine is asking.
It joins the peer seeds on the list of keys the generator fills, and the test checks what makes
each declaration wrong rather than comparing it to a value it cannot know.

Three checks that were measuring less than they claimed

The check on what varies by node kind matched keys by treating a section's name as a dotted
prefix, so it was blind to every key sitting at the root of the file — the claim that those
answer the same for every node kind was measured by nothing. It now reads the keys out of the
registry, which also stops it assuming a section's name and its prefix are the same string.

The pairing of a section to the structs carrying its deprecation markings was written by hand
and nothing held it against the section's own paths, so a row naming a wrong but existing struct
gave a marking set for a type the section does not declare and both checks reading it passed
measuring nothing. Confirmed against the unfixed file: pairing the peer-to-peer section with the
remote procedure call struct leaves the whole package green while twenty-one keys go unwatched.
Every path a section derives now has to name a field of a struct its row lists, and every listed
struct has to be named by one of those paths.

The root section was exempt from the key-count table, on the stated reason that a squashed group
counts as a single tagged field. That is false — the count walk recurses, giving sixteen, and
sixteen less five exclusions is the eleven the section registers — so the exemption cost the one
check that holds the registry's own derivation against an independent count, on the only section
that lacked it. The row is there now.

The collision check

The root-key collision check claimed to cover the whole registered set and could not: nothing
imports either registrar, so in that package's test binary the set was the ten sections it
registers itself. It moves to the package that links every registrar, with both registrars
linked there as named test imports, so a dropped import stops compiling rather than quietly
narrowing the union to one file.

Any reused name fails, including a cross-file pair that is harmless on its own. Telling the two
apart would need a section to state which file it belongs to, and it does not; rather than add
that for a case which has never arisen, the rule is the stronger one, that a name is a table's
or a root key's once across the whole key space.

Formatters, go vet and golangci-lint clean; race detector clean. Mutation-verified,
including against the unfixed file so each check is shown to have content: a root key made to
vary by node kind, a section paired with the wrong struct, a root field left untagged, and a
node root key pointed at a section name from the other file each fail with the reason that
names them.

Fourteen keys sit at the top of that file with no table of their own. They are
declared against a schema rather than the node's top-level type, because that
type carries the nine tables as well and declaring against it would declare
every table's keys a second time. The schema squashes the same base group that
type squashes, so those spellings still come from the node's own tags, and it
restates the two fields held beside that group. A test holds those two to the
type they came from by name, tag and type, and holds the count, so a third one
appearing there fails rather than going undeclared.

Two paths are left out. The home directory is where this file is found, so a
value inside it would be the file naming its own location, and the command line
already carries it. The node mode is the fact the file states at the top under
its own name, and a second spelling would let the two disagree, with the
resolution answering for one and the node reading the other.

A test also checks that no root key is another section's name. Nothing refuses
that collision, and the two settings it produces cannot both be written because
no file holds a value for a name and a table under it. This is the first change
to declare root keys beside another package's, so the check lives here until it
has somewhere better to be.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.32%. Comparing base (4ae239d) to head (574d272).

Additional details and impacted files

Impacted file tree graph

@@                   Coverage Diff                    @@
##           plt-775-node-sections-3    #3995   +/-   ##
========================================================
  Coverage                    60.31%   60.32%           
========================================================
  Files                         2065     2065           
  Lines                       177011   177025   +14     
========================================================
+ Hits                        106773   106787   +14     
  Misses                       60473    60473           
  Partials                      9765     9765           
Flag Coverage Δ
sei-chain-pr 54.05% <100.00%> (-45.95%) ⬇️
sei-db 69.80% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
config/tendermintbase/tendermintbase.go 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedAug 26, 2026, 11:54 PM

# Conflicts:
#	config/tendermintbase/tendermintbase_test.go
@bdchatham
bdchatham marked this pull request as ready for review August 25, 2026 21:14
@cursor

cursor Bot commented Aug 25, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes expand the declared configuration key space and default-resolution behavior for root settings; mistakes could mislead operators or config tooling, though impact is mostly metadata and tests rather than live node decoding.

Overview
Registers top-level node config.toml keys (section node_base) via a nodeRootSchema that squashes BaseConfig without re-declaring table keys, with explicit exclusions for home, mode, and removed ABCI-related settings. moniker joins filledByTheGenerator because init supplies it per operator, not from mode defaults.

Registration is refactored to declareSection / declareRootKeys with registeredHere so tests walk the same set the binary registers. Cross-file root-key vs table-name collision checking moves to cmd/seid with named imports of cosmosbase and tendermintbase, so dropped imports fail compile instead of shrinking coverage.

tendermintbase tests now read keys from the registry (including root keys), add markedIn / root schema and exclusion tests, and harden generatedFileCarries for the headerless root region and commented template lines.

Reviewed by Cursor Bugbot for commit 574d272. Bugbot is set up for automated code reviews on this repo. Configure here.

bdchatham and others added 2 commits August 25, 2026 14:17
A list written beside the registrations was a second statement of the same fact,
and the root-key section was registered and left off it. So the test that asks
whether every section this package registers is usable, and the filter that
decides which keys belong to it, both skipped that section.

The registration records itself now. Dropping a section from the recording fails
the tests, where dropping it from a hand-kept list did not.

Found while counting sections for a diagram, which is a poor substitute for a
check and the reason there is one now.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
seidroid[bot]
seidroid Bot previously requested changes Aug 25, 2026

@seidroid seidroid 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.

The new root-key section declares three deprecated, no-op node settings (proxy-app, abci, filter-peers) that the generated config.toml does not carry, contradicting both rootDefaults' own doc and the exclusion rule this file already applies to the removed consensus settings. The rest of the change is well-covered, though the new cross-package collision test cannot see any other package's sections in a package-local test binary.

Findings: 1 blocking | 3 non-blocking | 2 posted inline

Blockers

  • None at the file/PR level.
  • 1 blocking issue(s) flagged inline on specific lines.

Non-blocking

  • [suggestion] Root keys sit outside the whatVariesByNodeKind safety net. ours() (tendermintbase_test.go:65) matches on name+".", and declaredSections() was not extended with RootSectionName, so TestWhatVariesByNodeKindIsTheRecordedSet skips all 14 root keys. rootDefaults goes through forMode(mode), so a future mode rule touching e.g. DBPath or LogLevel would start varying by node kind with nothing recording it — the exact failure that test exists to catch for the nine tables. Making ours() also return true for a key declared by a registered section with an empty prefix would close it.
  • [suggestion] Excluding mode has a read-side consequence the PR description does not cover: mode is written at the top of every generated config.toml (sei-tendermint/config/toml.go:103), so once a caller feeds a real file into registry.Resolve, mode lands in Resolved.Unknown alongside genuine typos. Whatever consumes the registry will need an explicit exemption, not just the write-side mapping the description mentions.
  • 1 suggestion(s)/nit(s) flagged inline on specific lines.

Comment thread config/tendermintbase/tendermintbase.go Outdated
Comment thread config/tendermintbase/tendermintbase_test.go Outdated

@cursor cursor 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.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ad5474c. Configure here.

Comment thread config/tendermintbase/tendermintbase_test.go
bdchatham and others added 2 commits August 25, 2026 15:46
The deprecation check carried forward named three root keys this branch declares.
The node marks each field deprecated and nothing in the tree reads any of them:
out-of-process ABCI was removed, and so was peer filtering through it. Two state an
affirmative value in the node's own defaults, an address and a transport name, so
declaring them put settings for a transport the binary no longer has into the space
a writer renders from. The generated file writes twelve root keys and none of these.

The node name joins the peer seeds as a key the generator fills. It is the sharper
of the two: the command takes it as a required argument, so a generated file always
carries an operator's own, while the declaration answers with the hostname of
whatever machine is asking. Nothing caught it because both sides of the comparison
that would have read the same host. The test now checks what makes each declaration
wrong rather than comparing it to a value.

The check on what varies by node kind matched keys by treating a section's name as a
dotted prefix, so it covered a hundred and ten of a hundred and twenty-four keys and
was blind to the whole root section. The claim that those fourteen answer the same
for every node kind was measured by nothing. It now reads the keys out of the
registry, which also stops it assuming a section's name and its prefix are the same
string. Verified by making a root key vary: it is caught where it previously was not.

The root-key collision check claimed to cover the whole registered set and could not.
Nothing imports either registrar, so in that package's test binary the set was the
ten sections it registers itself, and the cross-package collision the check is about
was never exercised. It moves to the package that links every registrar, with the two
registrars linked there as test imports so the claim is true, and it now reports the
pair with the file each key came from rather than failing: across two files both
settings are writable, so that is a judgement rather than a defect. Verified by
pointing a node root key at a section name from the other file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bdchatham and others added 3 commits August 26, 2026 07:58
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two checks were narrower than their own documentation.

The cross-file collision check named the whole key space and rested on two blank
imports nothing asserted. Dropping either narrowed the union by a whole
configuration file, five sections and forty-three keys, and both guards passed
because both test only for emptiness. The two root sections are now named, so the
imports are load-bearing by compilation rather than by comment and a third
configuration file has to join the assertion to be covered.

The key-count table was iterated only by itself, so a section registered without a
row went unmeasured. Its sibling map already had that loop and said so in its own
comment; the table now has it too, with the root section named as the one absence
rather than skipped by silence.

One count corrected: the schema squashes fourteen spellings, of which nine become
keys.

Verified by mutation: dropping a registrar import and registering a section with no
count row are each caught.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@bdchatham

Copy link
Copy Markdown
Contributor Author

@seidroid review

seidroid[bot]
seidroid Bot previously requested changes Aug 26, 2026

@seidroid seidroid 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.

The root-key section for config.toml is well constructed and the two prior review findings are genuinely fixed, but the new cross-package collision test does the opposite of what its doc comment (and the reply on the previous thread) says: it hard-fails on a cross-file name match instead of reporting it, and it cannot name the file a section came from.

Findings: 1 blocking | 3 non-blocking | 4 posted inline

Blockers

  • None at the file/PR level.
  • 1 blocking issue(s) flagged inline on specific lines.

Non-blocking

  • None at the file/PR level.
  • 3 suggestion(s)/nit(s) flagged inline on specific lines.

Comment thread cmd/seid/cmd/registry_sections_test.go Outdated
Comment thread config/tendermintbase/tendermintbase.go Outdated
Comment thread config/tendermintbase/tendermintbase_test.go Outdated
Comment thread config/tendermintbase/tendermintbase.go Outdated
bdchatham and others added 7 commits August 26, 2026 10:09
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The collision check's doc described behaviour the code does not have. It said a match
is reported rather than failed and that the output names which file each key came
from. It fails, and nothing names a file: a section carries a name and a prefix and
no file, so the message printed the root section and the key.

Reconciled toward what the code does, because the alternative needs a section to
state which file it belongs to and that distinction is not worth its complexity for a
rule two spellings already keep. Any reused name is refused, including a cross-file
pair that is harmless on its own, and the doc now says so and says why the two cases
are not told apart.

The claim that the rule costs nothing is left to the test passing rather than written
beside it as a count. Two rounds of review found counts in these comments that the
lists beneath them had outgrown, and a second statement of a measured fact is what
goes stale.

Verified by mutation: a node root key pointed at a table name from the other file
fails with the message that explains the rule.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The doc on the key set read from the registry named the two totals the prefix form
covered and missed. Both moved when the exclusions changed one branch earlier, and
neither is load-bearing: what matters is that the prefix form was blind to every key
sitting at the root of the file, which does not need a number to say.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bdchatham and others added 4 commits August 26, 2026 12:29
One name for the key set the mode-variance walk reads, rather than two across the
series for the same mechanism. The doc kept is the one that names what the prefix form
was blind to, since the root section is what makes that concrete.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The check that an exclusion still has its justification now walks the registered set
rather than the table of key counts, so the section whose keys sit at the root of the
file is covered. Three of its exclusions rest on exactly that reason, and the one
remaining root exclusion that does not is named with the reason it has instead.

Verified by mutation: the node dropping the note on the out-of-process application
address fails with the reason that names both possibilities.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The root section leaves out five paths for two different reasons and the test named
one of them, holding the other three by their absence alone. Renamed to what it
checks, and the three the node removed are now measured against a generated file,
which is the half of their reason a test can hold and the half that expires first:
templating one back in makes it a key an operator writes whose value this space
refuses. Verified by mutation.

The accessor that returned the recorded set was exported for no caller outside the
package. The one consumer is a test in the same package, which reads the slice
directly and keeps the copy.

Leaving the node kind out is right, since the kind is what a resolution is asked
about and a declared answer for it would be the question. What was missing is the
other half: a generated file states it, so a caller handing a decoded file to the
resolver files it beside an operator's typos. The reader is where that is settled and
saying so here is what stops it being rediscovered.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@bdchatham

Copy link
Copy Markdown
Contributor Author

@seidroid review

@seidroid seidroid 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.

The root-key section for config.toml is well constructed and all four unresolved findings from the previous review are genuinely fixed. Two non-blocking notes remain: the deprecation check covers only nine of the root section's eleven keys, and one doc line calls the new named imports "blank".

Findings: 0 blocking | 2 non-blocking | 2 posted inline

Blockers

  • None at the file/PR level.

Non-blocking

  • None at the file/PR level.
  • 2 suggestion(s)/nit(s) flagged inline on specific lines.

Comment thread config/tendermintbase/tendermintbase_test.go Outdated
Comment thread cmd/seid/cmd/registry_sections_test.go Outdated
@seidroid
seidroid Bot dismissed stale reviews from themself August 26, 2026 19:50

Superseded: latest AI review found no blocking issues.

The root section's keys come from two of the node's structs and its markings were read
from one. The schema squashes the base group and restates two fields the node holds
beside it, so a deprecation note on either of those two lived on the top-level type and
nothing watched it: eleven keys, nine of them covered, and the two uncovered ones were
the two this package spells out by hand. A section now names the set of structs its
keys come from, and the root one names both.

Verified by mutation: marking a restated root field deprecated is caught, where it
previously left the key declared and the suite green.

The table of sections is also the single statement of them here, and unlike the
branches below it is held against the registrations: the count check walks what the
registrations recorded and fails on a section with no row, so the limit written down
earlier in the series is closed at this point rather than carried forward.

One contradiction removed: a comment called the two registrar imports blank nine lines
after the import block explains they are named and why that is load-bearing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@bdchatham

Copy link
Copy Markdown
Contributor Author

@seidroid review

seidroid[bot]
seidroid Bot previously requested changes Aug 26, 2026

@seidroid seidroid 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.

The root-key section itself is well built and both prior findings are genuinely fixed, but the change that was supposed to bring root keys under the node-kind check does not: declaredHere still walks declaredAgainst, so all 11 root keys remain unmeasured while the rewritten doc comment and the PR description say otherwise.

Findings: 1 blocking | 0 non-blocking | 1 posted inline

Blockers

  • None at the file/PR level.
  • 1 blocking issue(s) flagged inline on specific lines.

Non-blocking

  • None at the file/PR level.

Comment thread config/tendermintbase/tendermintbase_test.go Outdated
bdchatham and others added 2 commits August 26, 2026 13:45
The walk over what varies by node kind took its sections from the table that pairs each
with its prototype, and that table leaves the root section out on purpose, because the
arithmetic it does would be wrong for a squashed group. So the walk covered nine sections
and 105 keys while the doc above it said the eleven at the root of the file were in, and
the two deprecation checks beside it had already been moved to the recorded set. It now
takes the same set they do: 116 keys across ten sections.

That gap was the one this branch exists to close, and it was open while three places said
otherwise. Verified by mutation with the case named in review: making the log level follow
the node kind is now caught and named, where the suite was green before.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@bdchatham

Copy link
Copy Markdown
Contributor Author

@seidroid review

@seidroid seidroid 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.

The root-key section for config.toml is well built and the previous review's blocker is genuinely fixed — declaredHere now walks registeredHere, so all eleven root keys enter the node-kind check. What remains are non-blocking notes about the checks' own docs: the stated reason for keeping the root section out of the key-count table is contradicted by taggedFields, declaredAgainst's doc no longer describes what reads it, and the new schema test skips an untagged root field.

Findings: 0 blocking | 5 non-blocking | 5 posted inline

Blockers

  • None at the file/PR level.

Non-blocking

  • None at the file/PR level.
  • 5 suggestion(s)/nit(s) flagged inline on specific lines.

Comment thread config/tendermintbase/tendermintbase_test.go Outdated
Comment thread config/tendermintbase/tendermintbase_test.go Outdated
Comment thread config/tendermintbase/tendermintbase_test.go
Comment thread config/tendermintbase/tendermintbase_test.go Outdated
Comment thread config/tendermintbase/tendermintbase_test.go Outdated
@seidroid
seidroid Bot dismissed their stale review August 26, 2026 21:00

Superseded: latest AI review found no blocking issues.

Carries the doc discipline forward and applies it to what this branch adds.

The root-key exclusions had a comment attached to no declaration, sitting between the
list it was about and the next one's doc, so it documented nothing and read as the
opening of the wrong paragraph. Its content is the reader-side fact that a generated
file states the node kind, which belongs on the list that leaves the kind out, and it
is there now.

Three docs shed a closing paragraph that was commentary about the comment rather than
about its subject: that a rule costs nothing while nothing matches, that a test
passing is the statement, that a walk reads one set rather than another. Two more shed
a count and an argument for their own existence.

Both properties a future editor could break are kept: that any reused name fails
because no section states which file it belongs to, and that a section's markings come
from a set of structs because the root schema carries none of its own.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@bdchatham

Copy link
Copy Markdown
Contributor Author

Applied the godoc discipline from 3993 to what this branch adds, rather than waiting for it to be found here.

Removed: a comment attached to no declaration (the root-key exclusions had one sitting between the list it described and the next declaration's doc), three closing paragraphs that were commentary about the comment rather than its subject, a count that reads four on one branch and five on the next, and a paragraph narrating the reasoning an earlier revision used before this one changed it.

Kept, in the present tense: that an excluded path whose field stops being deprecated is a setting the node honours and the space refuses; that the state sync keys are a set an operator fills together with the scratch directory the one the template omits; that any reused name fails because no section states which file it belongs to; and that a section's markings come from a set of structs because the root schema carries none of its own.

@bdchatham

Copy link
Copy Markdown
Contributor Author

@seidroid review

@seidroid seidroid 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.

The root-key section for the node's config.toml is well built — the schema derives its spellings from the node's own tags, the exclusions each carry a checked reason, and moving the cross-package collision check into the package that links both registrars makes its claim true. No blockers, but the five suggestions from the previous review are all still open at the current head, two of them independently re-found by Codex.

Findings: 0 blocking | 5 non-blocking | 5 posted inline

Blockers

  • None at the file/PR level.

Non-blocking

  • None at the file/PR level.
  • 5 suggestion(s)/nit(s) flagged inline on specific lines.

Comment thread config/tendermintbase/tendermintbase_test.go Outdated
Comment thread config/tendermintbase/tendermintbase_test.go
Comment thread config/tendermintbase/tendermintbase_test.go
Comment thread config/tendermintbase/tendermintbase_test.go Outdated
Comment thread config/tendermintbase/tendermintbase_test.go Outdated
bdchatham and others added 3 commits August 26, 2026 16:23
It was a bare literal in three places while four neighbours in the same maps were
constants carrying their reason. Now a constant beside the one for the root directory,
with the reason on it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Keeps the recorded registration set as the source of section names here, rather than
the table of key counts the branch below derives them from. The record is written as
each registration runs, so a walk over it cannot drift from what the registry holds,
which the table can.

Unsplices the root-schema doc. A sentence had been inserted into the middle of another,
leaving one comment line at 184 characters in a file that otherwise wraps at 107, and
leaving an ambiguous clause about how many spellings become keys. The count is gone,
since nothing measures it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s own paths

A section's keys and their deprecation markings were paired by hand in a table the
registry knows nothing about, so a row naming a wrong but existing struct gave a
marking set for a type the section does not declare, and both checks that read it
measured nothing and passed. Confirmed against the unfixed file: pairing the
peer-to-peer section with the remote procedure call struct leaves the whole package
green while twenty-one keys go unwatched.

Every path a section derives now has to name a field of a struct its row lists, and
every listed struct has to be named by one of those paths, so a wrong row and a
superfluous row both fail. The rule reads a tag the way the deprecation parse does,
which is a struct's own fields and not a squashed group's: following the squash instead
would let the top-level type alone satisfy the root section while the markings that
live on the base group went unread.

The root section is no longer exempt from the key-count table. The reason given for the
exemption was that a squashed group counts as a single tagged field, which is false: the
count walk recurses, giving sixteen, and sixteen less five exclusions is the eleven the
section registers. The exemption cost the one check that holds the registry's own
derivation against a count taken independently, on the only section that lacked it.

A root field carrying no mapstructure tag was skipped where the doc said it fails. The
reader falls back to the field's own name, so such a field is a key an operator can
write under that spelling which no section declares. It fails now.

The region check in the generated-file helper looked for a table header, which is true
by construction for the keys above the first table, so the three root probes passed
measuring nothing. It counts written keys instead, which closes the named-table and the
root case with one rule.

Two counts and two literals removed: the node kind and the root directory each had one
spelling left as a bare string, and two counts about the template and the schema had
nothing measuring them.

Verified by mutation, including against the unfixed file to show each check has content.
Race detector clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@bdchatham

Copy link
Copy Markdown
Contributor Author

Also closed a hole outside the five, found while checking them: the generated-file helper looked for a table header to decide whether it had measured anything, and that is true by construction for the keys above the first table. So the three root probes passed measuring nothing, which is the shape the earlier scoping fix removed for named tables and left open here. It counts written keys instead, which closes both cases with one rule.

Verified by mutation against the unfixed file in each case, so every check is shown to have content rather than only to pass. Race detector clean.

@bdchatham

Copy link
Copy Markdown
Contributor Author

@seidroid review

@seidroid seidroid 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.

The root-key section for the node's config.toml is well built: the schema derives its spellings from the node's own tags, every exclusion carries a reason that is measured rather than asserted, and the struct-pairing check now fails on both a wrong row and a superfluous one. All five suggestions from the previous review are genuinely fixed at head, and I found nothing new worth blocking or flagging.

Findings: 0 blocking | 0 non-blocking | 0 posted inline

Blockers

  • None at the file/PR level.

Non-blocking

  • None at the file/PR level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants