Sign up
View stargazersSign up

chore(lint): enforce eslint/no-unneeded-ternary#25105

merged1
lint-rules/unicorn-no-useless-fallback-in-spread
lint-rules/eslint-no-unneeded-ternary
opened2d agobyJJulius Marminge
2 files changed
View on GitHubTwo-way sync enabledPR last imported 2d ago

Promote eslint/no-unneeded-ternary to an error. Use direct boolean expressions instead of ternaries returning boolean literals.

This PR changes only this rule and its existing call sites. Each changed rule has its own PR.

Validation: the original full stack passed its local checks. This rule is now in the mechanical landing batch ahead of held changes; updated batch checks and CI are running.

Model: GPT-6.1 Sol. Harness: Codex.

Checks

1 failing · 24 successful · 14 skipped
Some checks failed

Failing1

a01656c
Ggithub-actionsBot2d ago
Synced from GitHub

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wire—5.0 KiB—6.8 KiB✅
CodexThread snapshot wire—3.8 KiB—4.9 KiB✅
CodexLive turn WebSocket wire—1.2 KiB—2.0 KiB✅
CodexLive turn WebSocket decoded—20.9 KiB—29.3 KiB✅
CodexLive turn messages—2—8✅
ClaudeTotal thread wire—5.0 KiB—6.8 KiB✅
ClaudeThread snapshot wire—3.8 KiB—4.9 KiB✅
ClaudeLive turn WebSocket wire—1.2 KiB—2.0 KiB✅
ClaudeLive turn WebSocket decoded—21.2 KiB—29.3 KiB✅
ClaudeLive turn messages—2—8✅

Baseline: unavailable · PR result: a01656c · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

MmacroscopeappBot2d ago
Synced from GitHub

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval — Although the PR enables lint diagnostics and includes many mechanical cleanups, it also changes production event compaction, guest-share analytics, and web/mobile runtime behavior. The persistent data-retention change and mixed scope require focused human review.

You can add or adjust custom eligibility rules. Learn more.

Added 1 commit

  • chore(lint): enforce eslint/no-unneeded-ternarya01656c
CcoderabbitaiBot2d ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review.

External image Recent review info
External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between 135a7340fc659ad0b026e2d2db05ab0600d8d928 and a01656c4a2d93f0b14a00ffc6a326293c03ad743.

External image Files selected for processing: 2
  • packages/shared/src/serverSettings.test.ts
  • vite.config.ts

Limit details: You’ve used all 10 included reviews currently available.

Walkthrough

The pull request enables eslint/no-unneeded-ternary as an error and updates the fallback model-selection test to use a direct equality check.

Changes: Lint rule and test cleanup

Layer / File(s)Summary
Enable and satisfy the ternary lint rule
vite.config.ts, packages/shared/src/serverSettings.test.ts		The lint configuration enables eslint/no-unneeded-ternary as an error. The test predicate changes to a direct equality check. The test still expects no fallback for a configured non-default instance when configured instances are excluded.

Priority: External image Low

Estimated code review effort: 1 (Trivial) | ~4 minutes

Change: Other

Possibly related PRs

Suggested reviewers: yordis, flamboh

Merge Risk: External image Minimal · up to a0165

This PR contains a behavior-preserving test cleanup and lint configuration change with no identified production impact.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check nameStatusExplanationResolution
Description checkWarningThe description states the purpose: promoting eslint/no-unneeded-ternary to an error and replacing boolean-literal ternaries with direct boolean expressions. It also states the scope is limited to thi…Add the template sections. (1) Problem: explain why the rule should be an error. Describe the current behavior, such as ternaries that return boolean literals are still allowed. (2) Change: list the rule config change in vite.config.ts and …
Passed Passed checks: 3 passed
Check nameStatusExplanation
Title checkPassedThe title "chore(lint): enforce eslint/no-unneeded-ternary" matches the main change: the rule is promoted to an error and existing ternaries that return boolean literals are replaced with direct boole…
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title "chore(lint): enforce eslint/no-unneeded-ternary" matches the main change: the rule is promoted to an error and existing ternaries that return boolean literals are replaced with direct boolean expressions. It is a conventional commit title and names the specific rule, so it is concise and specific.

Full details: Description check

Explanation

The description states the purpose: promoting eslint/no-unneeded-ternary to an error and replacing boolean-literal ternaries with direct boolean expressions. It also states the scope is limited to this rule and its existing call sites. It gives a validation note and model/harness. It does not follow the template: the Problem, Change, and Scope and approval sections are missing. The Verification section lacks focused tests or manual checks with observed results for the changed behavior; it refers to the batch and CI runs, which are still pending. The author does not link a triaged issue or maintainer approval, and the template requires that for focused configuration changes. The description is not vague, but it is missing required sections and the verification detail the template asks for.

Resolution

Add the template sections. (1) Problem: explain why the rule should be an error. Describe the current behavior, such as ternaries that return boolean literals are still allowed. (2) Change: list the rule config change in vite.config.ts and the test change in packages/shared/src/serverSettings.test.ts. Explain why the test rewrite is part of this rule change. (3) Scope and approval: link the triaged issue or the maintainer discussion that approved this lint direction. Or explain why this focused configuration change of an established lint capability needs no prior approval, and state what the option controls. (4) Verification: list the focused commands you ran, such as the serverSettings test file and the lint run for the changed files, with observed results. State what you did not check, such as the pending final test rerun and CI. Keep the model and harness line at the end.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

JJulius Marminge2d ago

The current PR targets lint-rules/unicorn-no-useless-fallback-in-spread. Its diff at a01656c changes only packages/shared/src/serverSettings.test.ts (replacing a boolean ternary with its predicate) and vite.config.ts (enabling eslint/no-unneeded-ternary). The event-compaction and guest-share changes mentioned in the approvability summary are inherited base changes, not changes introduced by this PR. I reviewed this layer as a behavior-preserving test simplification and rule enforcement.