fix: deprecated snake-case twins overwrite explicit values (camelKey collision)#335
Merged
Conversation
Add a 'twins' test group covering deprecated snake_case arguments that collide with their kebab-case canonical twin on the same camelKey: an inline-declared pair (flag-a/flag_a) and a pair mirroring the real-world shape where the canonical inherits a default from the module's global _arguments.yml (show-preview/show_preview). The pinned golden shows the defect: an explicit `show-preview=false` is silently overwritten to `true` because the unset deprecated twin inherits the canonical's global default and sorts after it (underscore > hyphen) in the schema loop, and `show_preview` incorrectly appears in `defaulted`. A second case shows the same unconditional last-write-wins ordering flips the result when both spellings are explicitly provided. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A deprecated snake_case twin (e.g. show_preview) and its kebab-case canonical (show-preview) camelize to the same output key. ArgsSchema.html's snake->kebab normKey lookup let the twin inherit the canonical's global default, and Args.html's schema loop iterates map keys in sorted order, so the twin (sorting after the canonical, since '_' > '-') silently overwrote an explicit show-preview=false with its inherited default of true. Two changes fix this: - ArgsSchema.html: a node carrying "deprecated" never stores "default" or "config", whether inherited from the global definition or declared inline. - Args.html: the camelKey merge is now precedence-safe and order-independent. Provided-canonical values beat provided-deprecated values, which beat defaulted values, and a defaulted value never overwrites an existing entry. Explicit values now always win over any default, on both the strict Args.html path and the InitArgs.html compatibility shim. Verified against a new "twins" golden fixture covering both an inline-declared twin pair and a pair that inherits its default from the module's global _arguments.yml. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
🎉 This PR is included in version 6.0.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
5 tasks
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.
Summary
Structures that declare a kebab-case argument plus a deprecated snake_case
twin (e.g.
show-preview+show_preview, a common pattern across Hinode)silently overwrote an explicit value with the twin's inherited default.
Defect
Both names camelize to the same
camelKey(showPreview). InArgsSchema.html, the deprecated twin's node inherited the canonicalargument's global definition — including its
default— via thesnake→kebab
normKeylookup against the module's global_arguments.yml.In
Args.html's schema loop, Go template map iteration visits keys insorted order, and the twin sorts after the canonical (
_>-in ASCII).Since the loop unconditionally overwrote
$out[$node.camelKey]on everynon-nil result, the twin's defaulted value clobbered the caller's explicit
show-preview=false, turning it intotrue. The same unconditionallast-write-wins also flipped results whenever both spellings were
explicitly provided, regardless of intended precedence. Both the strict
Args.htmlpath and theInitArgs.htmlcompatibility shim were affected(the shim delegates to
Args.html).Fix (two layers)
ArgsSchema.html(compile-node): a node carryingdeprecatedneverstores
defaultorconfig— whether inherited from the globaldefinition via the
normKeylookup or declared inline. A deprecatedalias exists only to accept an explicit legacy value and warn; defaults
belong to the canonical replacement. This also keeps a deprecated twin
out of
defaultedwhen the caller leaves it unset.Args.html(main schema loop): the camelKey merge is nowprecedence-safe and order-independent via a small
providedKeystracking dict:
provided-canonical > provided-deprecated > defaulted. Adefaulted value never overwrites an existing entry; a provided value
overwrites a defaulted entry; a provided value from a deprecated node
never overwrites an existing provided entry.
Golden evidence
New
twinstest group (exampleSite/data/structures/test-twin.yml+exampleSite/data/tests/twins.yml) covers an inline-declared twin pair(
flag-a/flag_a) and a pair that inherits its default from the module'sglobal
_arguments.yml(show-preview/show_preview, verified via grepto carry
default: true).Pinned (buggy) golden —
explicit-false-on-canonical:Fixed golden — same case:
both-spellings-provided(kebabfalse+ snaketrue) flips fromflagA/showPreview: true(twin's explicit value wins, wrong) tofalse(canonical wins, correct) on both the
args(strict) andinitargs(shim)paths.
unset-uses-canonical-defaultkeepsflagA/showPreviewat theircanonical default (
true) but thedefaultedlist dropsshow_preview,leaving only the canonical
show-preview/flag-aentries.explicit-value-on-deprecated-twin(deprecation warning + explicit twinvalue honored) is unchanged before/after — it was already accidentally
correct, and is now correct by design via the precedence lattice.
Other-golden drift: none.
git diff --stat tests/golden/after the fixtouches only
twins.json; all 12 pre-existing golden groups(bookshop, casting, child, defaults, envelope, frontmatter, inittypes,
nesting, options, positional, required, schema) are byte-identical.
Test plan
pnpm test:update && pnpm test— 13/13 golden groups passthe exact defect described above
all four twin cases, on both the strict and shim paths
collisions" subsection under "Argument validation"
This ships as v6.0.1 (bug fix, no API change).
🤖 Generated with Claude Code