Require fallback parameter in valueSet() for empty list safety

[dahlia]

Summary

Fixes #492

valueSet([]) and values([]) previously returned empty message content that, when embedded in message tagged templates, collapsed surrounding prose into malformed sentences like "Expected one of ." or "Allowed values: .".

This PR makes valueSet() require a second parameter that specifies what to display when the values array is empty. The parameter can be either a simple fallback string or an options object with a required fallback field. This is a breaking change to the valueSet() API.

// Simple fallback string
message`Expected one of ${valueSet(choices, "(none)")}.`
// → "Expected one of (none)." when choices is empty

// Options object with fallback
message`Expected ${valueSet(choices, { fallback: "(none)", type: "disjunction" })}.`

// Empty fallback string produces an empty Message (same as old behavior)
message`Available: ${valueSet(branches, "")}.`

The fallback is enforced both at the type level (ValueSetOptions.fallback is a required field) and at runtime (throws TypeError for JavaScript callers or code compiled against the old signature).

For values(), which is only used internally to represent consumed tokens, the function now throws TypeError for empty arrays since an empty consumed token list is always a bug.

The git parser error messages in @optique/git have been updated to omit the "Available ..." suffix when the list is empty, and user-facing callbacks like notFound(input, available) can now use the fallback parameter directly instead of manual ternary guards: valueSet(available ?? [], "none").

Test plan

• mise test passes across all runtimes (Deno, Node.js, Bun)
• mise check passes (type check, lint, format, dry-run publish)
• pnpm build under docs/ passes (Twoslash type checking for code examples)

[codecov]

Codecov Report

❌ Patch coverage is 75.43860% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.33%. Comparing base (6945a5e) to head (b2463c8).
⚠️ Report is 11 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
packages/git/src/index.ts	39.13%	14 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #747      +/-   ##
==========================================
- Coverage   91.38%   91.33%   -0.06%     
==========================================
  Files          39       39              
  Lines       20867    20901      +34     
  Branches     5689     5710      +21     
==========================================
+ Hits        19070    19089      +19     
- Misses       1749     1764      +15     
  Partials       48       48              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[gemini-code-assist]

Code Review

This pull request introduces a breaking change to the valueSet() function, which now requires a mandatory second parameter to specify fallback text for empty input arrays. Additionally, the values() function now throws a TypeError when given an empty array. The changes include comprehensive updates to documentation, unit tests, and internal logic across multiple packages to ensure compliance with the new API requirements. I have no feedback to provide.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 32898a37-fafe-4d7c-903f-956ca602b994

📥 Commits

Reviewing files that changed from the base of the PR and between 85d8b7b and b2463c8.

⛔ Files ignored due to path filters (1)

• deno.lock is excluded by !**/*.lock

📒 Files selected for processing (15)

• CHANGES.md
• docs/concepts/messages.md
• docs/integrations/git.md
• packages/core/src/constructs.test.ts
• packages/core/src/doc.test.ts
• packages/core/src/message.test.ts
• packages/core/src/message.ts
• packages/core/src/primitives.ts
• packages/core/src/valueparser.ts
• packages/env/src/index.ts
• packages/git/src/index.test.ts
• packages/git/src/index.ts
• packages/man/src/man.test.ts
• packages/man/src/roff.test.ts
• packages/zod/src/index.ts

---

Walkthrough

This PR changes empty-list handling for message utilities: valueSet() now requires a second parameter (either a fallback string or an options object with a required fallback: string) and, when given an empty values array, returns either an empty Message (for fallback === "") or a single text term containing the fallback. values() now throws TypeError when passed an empty array. Call sites, tests, docs, and changelog entries were updated to pass explicit fallbacks; several git-related error messages were also adjusted to omit the “Available …” suffix when no alternatives exist.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• Fix formatDocPage() rendering empty choices/default as malformed suffixes #692: Similar changes addressing empty-choice/values rendering and avoiding malformed “Available …” suffixes in generated documentation and messages.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: requiring a fallback parameter in valueSet() to handle empty lists safely, addressing the core breaking change in this PR.
Description check	✅ Passed	The description is directly related to the changeset, explaining the motivation (issue #492), the solution (required fallback parameter), and implementation details across multiple files.
Linked Issues check	✅ Passed	The PR fully addresses issue #492 by making valueSet() require a fallback parameter to prevent malformed sentences when embedding empty value lists, and values() now throws TypeError for empty arrays, meeting all stated objectives.
Out of Scope Changes check	✅ Passed	All changes are directly in scope: valueSet() signature and implementation changes, values() error handling, test/documentation updates across all packages reflect the required fallback parameter, and git parser updates to omit empty availability lists.
Docstring Coverage	✅ Passed	Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/issue-492-empty-list-message

---

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 and usage tips.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/concepts/messages.md`:
- Around line 361-363: The locale definition entry contains a sentence fragment;
update the second sentence in the `locale` description so it is a complete
sentence (e.g., "Defaults to the system locale.") and ensure the `locale` entry
clearly describes accepted types (string, array of strings, Intl.Locale object,
or array of Intl.Locale objects) and the default behavior; edit the `locale`
definition text to replace the fragment with the complete sentence.

In `@packages/core/src/message.test.ts`:
- Around line 1129-1141: Update the test for valueSet to assert the thrown error
message as well as the TypeError for both call sites: when calling (valueSet as
any)([]) and (valueSet as any)([], { locale: "en" }). Locate the two
assert.throws calls in the "should throw TypeError when called without fallback"
test and replace/augment them to validate the error.message (via exact string or
regex) so the test verifies the expected diagnostic text from the runtime
validation path in addition to the TypeError.
- Around line 202-207: Update the test that currently only checks the error type
for values([]) to also assert the exact diagnostic message: when calling
values([]) assert.throws should verify both TypeError and the specific
error.message (either by passing the expected message string or a RegExp) so the
test asserts the error classification and the exact validation text produced by
the values function/constructor.

🪄 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: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: d2ba95be-7da2-4616-8d92-67754c69a21c

📥 Commits

Reviewing files that changed from the base of the PR and between dd14129 and 9d9b7f8.

⛔ Files ignored due to path filters (1)

• deno.lock is excluded by !**/*.lock

📒 Files selected for processing (15)

• CHANGES.md
• docs/concepts/messages.md
• docs/integrations/git.md
• packages/core/src/constructs.test.ts
• packages/core/src/doc.test.ts
• packages/core/src/message.test.ts
• packages/core/src/message.ts
• packages/core/src/primitives.ts
• packages/core/src/valueparser.ts
• packages/env/src/index.ts
• packages/git/src/index.test.ts
• packages/git/src/index.ts
• packages/man/src/man.test.ts
• packages/man/src/roff.test.ts
• packages/zod/src/index.ts

[dahlia]

@codex review

[dahlia]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a breaking change to the valueSet() function, which now requires a second parameter to specify a fallback string or options object for empty arrays. Additionally, the values() function has been updated to throw a TypeError when provided with an empty array. These changes aim to prevent malformed sentences in message templates when lists are empty. The feedback provided suggests refactoring several error message implementations in packages/git/src/index.ts to reduce code duplication by using nested message templates for conditional parts.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. 👍

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[dahlia]

@codex review

[dahlia]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a breaking change to the valueSet() function, which now requires a mandatory second parameter for fallback text when the input array is empty. Additionally, the values() function has been updated to throw a TypeError if provided with an empty array. These changes aim to prevent malformed error messages in CLI outputs. The feedback suggests extracting a repeated conditional message pattern in packages/git/src/index.ts into a helper function to improve maintainability and adhere to DRY principles.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Hooray!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".