Validate empty `metavar` in `logOutput()` by dahlia · Pull Request #708 · dahlia/optique

dahlia · 2026-03-22T15:58:18Z

Fixes #459

logOutputValueParser() in packages/logtape/src/output.ts was the only value parser in the codebase that did not call ensureNonEmptyString() on the metavar option. This allowed empty strings to slip through at runtime and produce malformed help output like --log-output with a blank metavar.

const parser = logOutput({ metavar: "" as never });
const page = getDocPage(parser)!;
console.log(formatDocPage("app", page, { colors: false }));
// => "--log-output " (blank metavar in usage text)

The fix adds the same ensureNonEmptyString() call that all other 30+ value parsers already use, so an empty metavar now throws TypeError at construction time instead of silently producing broken output.

logOutputValueParser() was the only value parser that did not call ensureNonEmptyString() on the metavar option, allowing empty strings to produce malformed help output. Add the same validation that all other value parsers use. Close #459 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

gemini-code-assist · 2026-03-22T15:58:32Z

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves an issue where the logOutput() function in the @optique/logtape package would silently accept an empty string for its metavar option, resulting in incorrectly formatted help text. The change introduces a runtime validation check, ensuring that metavar is always a non-empty string, thereby improving the robustness and consistency of argument parsing across the library.

Highlights

logOutput() metavar validation: The logOutput() function now validates that its metavar option is a non-empty string, preventing malformed help output. An empty metavar will now throw a TypeError at construction time, aligning its behavior with other value parsers.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request addresses an inconsistency in the logOutput() value parser by adding validation to prevent an empty metavar option, which previously led to malformed help text. The fix introduces a runtime check using ensureNonEmptyString, aligning it with over 30 other value parsers in the project. The change is accompanied by a new regression test and appropriate JSDoc updates. The implementation is correct and follows existing patterns, so I have no suggestions for improvement.

codecov · 2026-03-22T15:59:52Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.91%. Comparing base (84b0f47) to head (fa14550).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #708   +/-   ##
=======================================
  Coverage   93.91%   93.91%           
=======================================
  Files          38       38           
  Lines       19193    19196    +3     
  Branches     5248     5249    +1     
=======================================
+ Hits        18025    18028    +3     
  Misses       1144     1144           
  Partials       24       24

☔ 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.

coderabbitai · 2026-03-22T16:02:09Z

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: fa588f41-bcac-4f1b-9f20-0a82e7fa4d4e

📥 Commits

Reviewing files that changed from the base of the PR and between 43a2095 and fa14550.

📒 Files selected for processing (1)

packages/logtape/src/index.test.ts

Walkthrough

This PR validates the metavar used by logOutput() by calling ensureNonEmptyString on the resolved metavar (defaulting to "FILE"), causing a TypeError when an empty string is provided. JSDoc for logOutput and its internal logOutputValueParser was updated to document the thrown TypeError. A test was added asserting logOutput({ metavar: "" as never }) throws the expected TypeError. CHANGES.md was updated with a changelog entry and issue/PR references.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Validate empty metavar in zod() and valibot() #691 — Applies the same ensureNonEmptyString validation pattern to other value-parsing helpers (zod and valibot integrations).
Fix logOutput() hidden-file completion for dot-prefixed paths #699 — Modifies logOutput() in the same file and touches related behavior (hidden-file completion logic).

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely summarizes the main change: adding validation for empty metavar values in logOutput().
Description check	✅ Passed	The description is clearly related to the changeset, explaining the problem, solution, and providing reproduction steps and examples.
Linked Issues check	✅ Passed	The PR fully addresses issue `#459` by adding ensureNonEmptyString() validation to logOutputValueParser(), which now throws TypeError for empty metavar values as required.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing issue `#459`: validation logic, test coverage, documentation, and changelog entry are all focused on the metavar validation requirement.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% 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-459-logoutput-metavar-validation

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

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

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@packages/logtape/src/index.test.ts`:
- Around line 447-451: Replace the current assert.throws(...) that passes
TypeError and a message string with an object matcher so the thrown error type
and message are both validated; specifically, change the assertion to call
assert.throws(() => logOutput({ metavar: "" as never }), { name: "TypeError",
message: "Expected a non-empty string." }); this targets the same failure from
ensureNonEmptyString invoked by logOutput and follows the pattern used elsewhere
in the file.

🪄 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: 11051536-450d-4d30-ac1c-f7fb7568acff

📥 Commits

Reviewing files that changed from the base of the PR and between 84b0f47 and 43a2095.

📒 Files selected for processing (3)

CHANGES.md
packages/logtape/src/index.test.ts
packages/logtape/src/output.ts

packages/logtape/src/index.test.ts

dahlia · 2026-03-22T16:34:26Z

@codex review

chatgpt-codex-connector · 2026-03-22T16:37:36Z

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

ℹ️ 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".

The third argument to assert.throws(fn, ErrorClass, msg) is the assertion failure message, not the expected error message. Switch to an object matcher to properly validate both the error type and message, matching the pattern used for createConsoleSink tests in the same file. #708 (comment) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

dahlia · 2026-03-22T16:41:04Z

@codex review

dahlia · 2026-03-22T16:41:08Z

/gemini review

gemini-code-assist

Code Review

This pull request adds validation for the metavar option in logOutput() to prevent empty strings, which previously caused malformed help output. The change aligns logOutput() with other value parsers by throwing a TypeError for empty metavar values. The implementation, which includes adding a call to ensureNonEmptyString, updating JSDoc comments, adding a regression test, and documenting the fix in the changelog, is consistent with the project's development practices. I have reviewed the changes and found no issues.

chatgpt-codex-connector · 2026-03-22T16:42:37Z

Codex Review: Didn't find any major issues. Chef's kiss.

ℹ️ 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 self-assigned this Mar 22, 2026

dahlia added the bug Something isn't working label Mar 22, 2026

dahlia added this to the Optique 1.0 milestone Mar 22, 2026

gemini-code-assist bot reviewed Mar 22, 2026

View reviewed changes

coderabbitai bot reviewed Mar 22, 2026

View reviewed changes

packages/logtape/src/index.test.ts Show resolved Hide resolved

gemini-code-assist bot reviewed Mar 22, 2026

View reviewed changes

dahlia merged commit 437307f into main Mar 22, 2026
8 checks passed

dahlia deleted the fix/issue-459-logoutput-metavar-validation branch March 22, 2026 17:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Validate empty `metavar` in `logOutput()`#708

Validate empty `metavar` in `logOutput()`#708
dahlia merged 2 commits intomainfrom
fix/issue-459-logoutput-metavar-validation

dahlia commented Mar 22, 2026

Uh oh!

gemini-code-assist bot commented Mar 22, 2026

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

codecov bot commented Mar 22, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Mar 22, 2026 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

dahlia commented Mar 22, 2026

Uh oh!

chatgpt-codex-connector bot commented Mar 22, 2026

Uh oh!

dahlia commented Mar 22, 2026

Uh oh!

dahlia commented Mar 22, 2026

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

chatgpt-codex-connector bot commented Mar 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

dahlia commented Mar 22, 2026

Uh oh!

gemini-code-assist bot commented Mar 22, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

codecov bot commented Mar 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai bot commented Mar 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

Possibly related PRs

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dahlia commented Mar 22, 2026

Uh oh!

chatgpt-codex-connector bot commented Mar 22, 2026

Uh oh!

dahlia commented Mar 22, 2026

Uh oh!

dahlia commented Mar 22, 2026

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

chatgpt-codex-connector bot commented Mar 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

codecov bot commented Mar 22, 2026 •

edited

Loading

coderabbitai bot commented Mar 22, 2026 •

edited

Loading