ci(config): enforce lowerCamelCase and max depth in reference.conf#30
Open
bladehan1 wants to merge 1 commit into
Open
ci(config): enforce lowerCamelCase and max depth in reference.conf#30bladehan1 wants to merge 1 commit into
bladehan1 wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughAdds a Python CLI ChangesReference Configuration Validation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/check_reference_conf.py:
- Around line 108-117: The parser only pushes the key prefix into frames when
the delimiter is a literal '{', so cases like "foo = {", "foo: {", or "foo = [ {
... } ]" lose the "foo" prefix; update the branch in the block around ident
handling (the code that checks p2 < n and src[p2] in '=:{') to treat a following
'{' or '[' as a nested-value indicator regardless of whether the delimiter was
'=' or ':' and push the full_parts prefix onto frames; if src[p2] == '{' push
{"kind":"obj","prefix":full_parts}, if src[p2] == '[' push
{"kind":"arr","prefix":full_parts}, and set pos = p2 + 1 in both cases so nested
object/array keys retain the parent key prefix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ac43e6ed-37ff-4ccc-b11a-352a56d2c404
📒 Files selected for processing (2)
.github/scripts/check_reference_conf.py.github/workflows/pr-check.yml
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pr-check.yml:
- Around line 106-113: Pin the GitHub action and the pyhocon install to
immutable versions: replace uses: actions/setup-python@v5 with the action
referenced by its full commit SHA (e.g., actions/setup-python@<commit-sha>) and
change the pip install step to install a specific pyhocon release (e.g., pip
install pyhocon==<version>), updating the workflow steps that currently
reference actions/setup-python@v5 and the pip install command so the action and
dependency are immutable and reproducible.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 423dc7fa-48b7-40eb-80e8-3482cc0ee539
📒 Files selected for processing (2)
.github/scripts/check_reference_conf.py.github/workflows/pr-check.yml
bladehan1
force-pushed
the
ci/reference-conf-gate
branch
from
e7d540a to
52d2e77
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/check_reference_conf.py:
- Line 54: The constant MAX_DEPTH is set one too high; change MAX_DEPTH from 6
to 5 to enforce the PR contract of <= 5 depth (update the variable MAX_DEPTH in
this script and any tests or references that assume the limit); ensure any
related comparisons or error messages referencing MAX_DEPTH still reflect the
new value.
- Around line 84-95: The walk function currently only increments and emits the
synthetic array step (array_path/array_depth) when list elements are ConfigTree,
so pure scalar lists never report the array step; modify the list branch in walk
(the variables node, array_path, array_depth, seen) to always emit the
array_path once for scalar elements: detect if the list contains any
non-ConfigTree scalars (or no ConfigTree yielded subpaths) and yield array_path
with array_depth and sub_leaf=True (or appropriate leaf flag) exactly once
before/if you skip into per-element traversal, and still keep the existing logic
for ConfigTree elements while using seen to avoid duplicate sub_path emissions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6b6d3ef0-c4ee-4a5f-b0ed-98edebc650b2
📒 Files selected for processing (2)
.github/scripts/check_reference_conf.py.github/workflows/pr-check.yml
bladehan1
force-pushed
the
ci/reference-conf-gate
branch
from
52d2e77 to
ff0474c
Compare
bladehan1
commented
May 20, 2026
bladehan1
force-pushed
the
ci/reference-conf-gate
branch
from
ff0474c to
ef37233
Compare
Add a CI gate that scans common/src/main/resources/reference.conf and fails the build when any key violates lowerCamelCase (^[a-z][a-zA-Z0-9]*$ per dot-separated segment) or exceeds the maximum hierarchy depth (6). Array element keys are validated the same way; each array step counts as one depth level — e.g. an inner field at `rate.limiter.rpc[].component` is depth 5. Parsing is delegated to pyhocon, the reference Python HOCON implementation. It returns a fully-merged ConfigTree where dotted-form keys expand into nested objects — the same canonical key set Typesafe Config and ConfigBeanFactory see at runtime — and handles triple-strings, substitutions, includes, +=, and block comments without us re-implementing the grammar. Four legacy PBFT* keys are grandfathered via an in-script allowlist so the gate fails only on new violations. A consolidated GHA error annotation lists every offending key, and sys.exit(1) drives step failure. The script also accepts `--debug` to print every parsed key with its depth (trailing `/` marks namespace intermediates) for manual verification against the source file. Runs as a new step in the existing checkstyle job of pr-check.yml (setup-python + `pip install pyhocon`), so no extra runner spin-up.
bladehan1
force-pushed
the
ci/reference-conf-gate
branch
from
ef37233 to
cd96b77
Compare
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
1 participant
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.
What does this PR do?
Adds a CI gate that scans
common/src/main/resources/reference.confand fails the build when any key violates either of:^[a-z][a-zA-Z0-9]*$(lowerCamelCase — required byConfigBeanFactoryauto-binding).<= 6. Each list/array step counts as one additional level — e.g.rate.limiter.rpc[].componentis depth 5 (rate=1, limiter=2, rpc=3, []=4, component=5).Implementation:
.github/scripts/check_reference_conf.py— parses via pyhocon, the reference Python HOCON implementation. Returns a fully-mergedConfigTreewhere dotted-form keys expand into nested objects — the same canonical key set Typesafe Config /ConfigBeanFactorysee at runtime. Handles triple-strings, substitutions, includes,+=, block comments, etc. without us re-implementing the grammar.rate.limiter.rpc = [{ component=..., ... }]) are validated the same way. Element keys are deduplicated across list entries because well-formed arrays use homogeneous object shapes.Validate reference.conf key names and depthstep in the existingcheckstylejob of.github/workflows/pr-check.yml— saves runner-minutes vs a standalone workflow.::error file=...) with all entries packed via%0Aso they show in the Checks panel;sys.exit(1)is the sole failure driver (noexit 1in the workflow shell wrapper).--debugflag prints every parsed key with its depth in walk order; trailing/marks namespace intermediates. Useful for manual verification against the source file.Why are these changes required?
Without an automated gate, key names in
reference.confhave drifted: 4 legacy keys (node.http.PBFTEnable,node.http.PBFTPort,node.rpc.PBFTEnable,node.rpc.PBFTPort) start with an uppercase acronym and require manual normalization inArgs.java. Each additional drift costs reviewer attention and risks silentConfigBeanFactorybinding failures.The 4 existing violators are grandfathered via an explicit
ALLOWLISTso the gate fails only on new violations.This PR has been tested by:
common/src/main/resources/reference.confreturnsOK: ... 294 keys, all lowerCamelCase, depth <= 6(exit 0). The 294 includes 13 array-element fields undergenesis.block.assets[],genesis.block.witnesses[], andevent.subscribe.topics[].::errorannotation; reverting passes again.--debugoutput manually eyeballed against the source file; depth column shows the[]array steps explicitly so MAX_DEPTH math is verifiable by inspection.Follow up
Args.javasince userconfig.conffiles use the legacy names).XxxConfig.javafield) — deliberately not coupled with this PR.Extra details
MAX_DEPTH = 6(current max is 5, one level of headroom).KEY_REGEX = ^[a-z][a-zA-Z0-9]*$; allowlist contains exactly the 4 PBFT keys.pBFTExpireNum,allowPBFT,isOpenFullTcpDisconnectstart with lowercase and pass the regex unchanged.reference.conf(e.g.node.shutdown.BlockTime) are not seen by pyhocon, so they don't need to be allowlisted.Summary by cubic
Adds a CI gate to enforce lowerCamelCase and a max depth of 6 for
reference.confkeys, including keys inside arrays. Parses withpyhoconto match runtime binding, emits one consolidated::erroron violations, allowlists four legacyPBFT*keys, and supports--debugto print parsed keys and depths..github/scripts/check_reference_conf.pyin thecheckstylejob.pyhoconin.github/workflows/pr-check.yml.Written for commit cd96b77. Summary will update on new commits. Review in cubic
Summary by CodeRabbit