feat(derive): hold the bytes a word arrived as

[jdx]

A value used to reach its field through from_utf8_lossy, so a path with a stray byte in it
became a path with U+FFFD in it — a different file, silently. That is the worst shape a bug
can take: no error, wrong answer.

The partial holds the bytes now and the conversion happens once, where the struct is built.
apply cannot report anything — it answers whether an event belonged to this command — which
is why the bytes have to travel that far before anyone can complain about them. A word that
is not UTF-8 is an InvalidValue naming the field, showing the value lossily for the message
only, and saying what was wrong with it.

It also retires a hazard review raised on the last PR: String was recognised by how it was
written so that its value could be moved rather than converted, which broke for an adopter
who shadowed the name. There is no identity case left — everything converts — so recognising
the spelling only skips a second step now, and getting it wrong is a compile error rather
than a mangled value.

Costs +656 instructions (1.6%) and one allocation against main, measured on the same fixture:
the collecting path rebuilds a Vec<String> from bytes rather than moving one. That is what
not corrupting a value is worth.

Reporting such a value is not the same as accepting it, and accepting it needs
OsStr::from_encoded_bytes_unchecked — unsafe, in a crate that has none. The call would be
sound and PLAN.md says why, but that is jdx`s call rather than mine.

---

_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

---

Note

Medium Risk
Changes how all CLI string values are stored and converted during parse; behavior improves for invalid UTF-8 but touches hot derive codegen paths with a small measured perf cost.

Overview
Argv values are kept as raw bytes through binding instead of being turned into strings with from_utf8_lossy. The derive partial now stores Vec<u8> for string-like fields; conversion to String / FromStr types happens once in build, where invalid UTF-8 becomes InvalidValue (the error message may show the value lossily for display only).

value_enum / choices checks skip bytes that are not valid UTF-8 so users get a UTF-8 error instead of a misleading InvalidChoice list.

The old String identity fast path is removed—every typed field goes through the same UTF-8 validation path. Defaults and env fallbacks write UTF-8 bytes into the partial to match.

Conformance tests cover non-UTF-8 --out paths and enumerated --shell values. PLAN.md marks “report, don’t mangle” done and notes accepting exact non-UTF-8 paths is still future work.

^{Reviewed by Cursor Bugbot for commit ea7b427. Bugbot is set up for automated code reviews on this repo. Configure here.}

Cost

Measured on the mise-scale shadow (use -g node@20), differencing two runs of the same
binary against the same fixture on main:

	main	this branch
instructions, cold parse	39,828	40,484
allocations, bound parse	3	4
allocations, bare parse	0	0

+656 instructions, or 1.6%, and one allocation — the Vec<u8> a value is held in before it
becomes a String. That is the price of not silently substituting a different filename, and
it is paid only by invocations that carry a value.

What this does not do

A non-UTF-8 value is reported, not accepted: PathBuf on Unix could hold those bytes
faithfully, but constructing an OsStr from them needs OsStr::from_encoded_bytes_unchecked,
and usage-argv has no unsafe in it today. The bytes do come from as_encoded_bytes in this
same process and every split the parser makes is at an ASCII byte, so the call would be sound
— but that is a policy decision about the crate rather than a detail of this change, so this
branch takes the conservative half and names the field in the error instead.

Resolved in #844, which is stacked on this one: jdx approved the unsafe, and it then
turned out to be needed only on Windows — on Unix the safe OsString::from_vec does the job.
A PathBuf field there accepts the bytes exactly, and does so for 567 instructions against a
String field's 660.

AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable.

Summary by CodeRabbit

• Bug Fixes

  • Preserved command-line, default, and environment values without lossy UTF-8 replacement.
  • Added clear validation errors for invalid UTF-8 values, including typed options, paths, strings, and collections.
  • Improved distinction between invalid values and valid text that does not match available choices.

• Documentation

  • Updated documentation to explain byte-preserving value handling and current limitations.

• Tests

  • Added coverage for valid paths and text, invalid UTF-8 input, enum validation, and field-specific errors.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: d1091c85-17d7-4cba-9599-b8c0d417a3ea

📥 Commits

Reviewing files that changed from the base of the PR and between 5257420 and ea7b427.

📒 Files selected for processing (2)

• conformance/tests/typed.rs
• derive/src/codegen.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• derive/src/codegen.rs

---

📝 Walkthrough

Walkthrough

The parser now preserves CLI and environment values as bytes until final conversion. UTF-8 validation reports InvalidValue errors instead of replacement. Typed and repeatable fields use conversion logic, and conformance tests cover valid and invalid values.

Changes

Non-UTF-8 value handling

Layer / File(s)	Summary
Byte-preserving intermediate values derive/src/codegen.rs	Generated helpers, partial fields, defaults, and environment fallbacks now retain values as byte vectors.
UTF-8 validation and field conversion derive/src/codegen.rs	Final construction validates UTF-8, applies FromStr for typed fields, converts repeatable values element by element, and validates choices.
Conformance coverage and documented status conformance/tests/typed.rs, derive/src/lib.rs, PLAN.md	Tests cover invalid and valid values. Documentation records byte preservation and the remaining unsupported exact acceptance of non-UTF-8 values.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Mergeability Score: 🟡 Moderate · up to ea7b4

Invalid UTF-8 values used with choices may be reported as the wrong error type, which can mislead callers and violate the documented validation behavior. The PR should address this before merge, and the diagnostic-value assertion should be added.

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant GeneratedParser
  participant FieldConverter
  participant TargetStruct
  CLI->>GeneratedParser: provide argument or environment bytes
  GeneratedParser->>FieldConverter: pass byte-preserving values
  FieldConverter->>FieldConverter: validate UTF-8 and apply FromStr
  FieldConverter->>TargetStruct: construct converted fields
  FieldConverter-->>CLI: return InvalidValue for invalid UTF-8

Loading

Possibly related PRs

• jdx/usage#803: Introduced the generated parser code extended by this byte-preserving handling.
• jdx/usage#833: Added typed-field conversion and InvalidValue handling extended by this change.
• jdx/usage#838: Modified enum parsing and validation in the same code and test areas.

Poem

I’m a rabbit guarding bytes in a row,
No lossy replacements shall enter the flow.
Typed paths convert when the bytes are right,
Invalid UTF-8 gets reported outright.
Hop, hop—clean values now land just so!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: preserving the original bytes of each command-line word during derive parsing.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[cursor]

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

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

^{Reviewed by Cursor Bugbot for commit 5257420. Configure here.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

derive/src/codegen.rs (1)

1540-1549: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not convert invalid UTF-8 into InvalidChoice.

check runs before field_final. Line 1542 converts invalid bytes to "". Line 1543 then returns InvalidChoice before final conversion can return InvalidValue.

If "" is a declared choice, this check passes and later returns InvalidValue. The error type must not depend on the choice list. Skip choice validation when UTF-8 decoding fails. Let field_final report InvalidValue.

Proposed fix

-                let __usage_text = ::std::str::from_utf8(value).unwrap_or_default();
-                if !`#choices.contains`(&__usage_text) {
-                    return ::std::result::Result::Err(
-                        ::usage_argv::Error::InvalidChoice {
-                            name: `#name`,
-                            choices: `#choices`,
-                        },
-                    );
+                if let ::std::result::Result::Ok(__usage_text) = ::std::str::from_utf8(value) {
+                    if !`#choices.contains`(&__usage_text) {
+                        return ::std::result::Result::Err(
+                            ::usage_argv::Error::InvalidChoice {
+                                name: `#name`,
+                                choices: `#choices`,
+                            },
+                        );
+                    }
                 }

The PR objective requires InvalidValue for invalid UTF-8.

🤖 Prompt for 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.

In `@derive/src/codegen.rs` around lines 1540 - 1549, Update the choice validation
around __usage_text to skip the contains check when ::std::str::from_utf8(value)
fails, rather than replacing invalid UTF-8 with an empty string for validation.
Preserve choice validation for successfully decoded text, and let field_final
produce InvalidValue for decoding failures regardless of the declared choices.

🤖 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 `@conformance/tests/typed.rs`:
- Around line 342-344: Guard the non-UTF-8 test and its Unix-specific imports in
the typed conformance tests with #[cfg(unix)], including the OsStrExt usage, so
the test module compiles on Windows while retaining the existing behavior on
Unix.

---

Outside diff comments:
In `@derive/src/codegen.rs`:
- Around line 1540-1549: Update the choice validation around __usage_text to
skip the contains check when ::std::str::from_utf8(value) fails, rather than
replacing invalid UTF-8 with an empty string for validation. Preserve choice
validation for successfully decoded text, and let field_final produce
InvalidValue for decoding failures regardless of the declared choices.

🪄 Autofix

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: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 6d317488-11ff-4f27-aa1d-90b315ee812c

📥 Commits

Reviewing files that changed from the base of the PR and between bc9d648 and 5257420.

📒 Files selected for processing (4)

• PLAN.md
• conformance/tests/typed.rs
• derive/src/codegen.rs
• derive/src/lib.rs

[github-actions]

Instruction counts

benchmark	trend	instructions	Δ	wall (min)	Δ
markdown	▂▁▆▇▅█▄▂▄▇▃▅	148,191,912 → 148,231,127	+0.03%	14.56 → 15.01ms	+3.05%
startup	▁▁▁█████████	1,201,903 → 1,201,983	+0.01%	0.98 → 1.05ms	+7.66%

No instruction-count regression above 1%.

_{Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run.}

_{Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.}

Shadow comparison

Parsing mise use -g node@20 against a shadow of mise's committed spec.
Reported, not gated: the shadow grows as the derive learns to express more, so
what to watch is the ratio rather than either column.

	usage	clap	ratio
instructions, cold parse	40499	5960254	147x

usage: argv -> struct                            1839 ns      1.84 µs
clap: build tree + parse -> struct             513928 ns    513.93 µs
clap: parse -> struct, tree reused              24638 ns     24.64 µs
clap: build tree only                          315968 ns    315.97 µs

_{ea7b42778542 vs bc9d64801f56 · measured on the runner, not pushed to the history.}

[greptile-apps]

Greptile Summary

This PR preserves argv values as bytes until typed struct construction, preventing lossy UTF-8 substitution.

• Converts retained bytes with String::from_utf8 and reports invalid input as a field-specific InvalidValue.
• Ensures choice validation defers non-UTF-8 values to typed conversion.
• Adds conformance coverage for invalid choice bytes and valid UTF-8 paths.
• Updates the derive documentation and implementation plan.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
derive/src/codegen.rs	Byte-preserving generated storage, conversion, defaults, environment fallback, and choice validation are consistent across required, optional, and collection shapes.
conformance/tests/typed.rs	Adds focused regression coverage proving non-UTF-8 choice values return InvalidValue while ordinary invalid choices still return InvalidChoice.
derive/src/lib.rs	Updates public documentation to accurately describe delayed UTF-8 conversion and the remaining non-UTF-8 path limitation.
PLAN.md	Records completion of non-lossy reporting and separates acceptance of non-UTF-8 paths into future work.

_{Reviews (2): Last reviewed commit: "fix(derive): report the UTF-8 failure, n..." | Re-trigger Greptile}

[jdx]

Addressed in the latest push.

`choices` masked a UTF-8 failure — all three of you found this independently, which it deserved. A non-UTF-8 value became `""` through `unwrap_or_default()`, matched no choice, and came back as `InvalidChoice`: a message listing words, about a value that is not a word. The comment beside it already claimed `build` reported this; it did not. Bytes that will not convert are now passed over and left for `build`, which names the field and gives the UTF-8 error as the reason. A value that is text and is not one of the choices still gets the list. Reproduced first, then fixed — `ea7b427`.

AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: unavailable.