Skip to content

chore: ban nested ternaries via biome#544

Merged
SgtPooki merged 1 commit into
mainfrom
fix/biome-config
May 18, 2026
Merged

chore: ban nested ternaries via biome#544
SgtPooki merged 1 commit into
mainfrom
fix/biome-config

Conversation

@SgtPooki
Copy link
Copy Markdown
Collaborator

Summary

  • Enable Biome style/noNestedTernary: error repo-wide so nested ternaries fail CI.
  • Fix the two existing violations:
    • apps/web/src/pages/Landing.tsx: replace nested ternary picking approvedSpDashboardUrl by network with a Record<Network, ...> lookup (gains compile-time exhaustiveness if Network grows).
    • apps/backend/src/deal-addons/strategies/ipni.strategy.ts: replace inline nested ternary on failureReason with a new getErrorMessage helper.
  • Add getErrorMessage(error: unknown): string in apps/backend/src/common/logging.ts. Derives from toStructuredError (so redaction is inherited) and prefers cause.message over outer message when the cause is a real Error — preserves PR feat(ipni): expose failureReason as top-level field on ipni_tracking_failed #491 / issue IPNI xxx root CID not verified #473 intent of keeping failureReason aggregable in BetterStack/Grafana (no high-cardinality deal IDs in the flat field). Guards against non-Error causes leaking "Non-Error thrown".

Peer-reviewed in three rounds via Claude, Cursor, and Gemini agents; consensus approved after the cause-preferred semantics were restored and the cause.type === "error" guard added.

Test plan

  • pnpm lint:check — passes (the only remaining warning is in an untracked scratch file).
  • pnpm build (apps/backend) — passes.
  • pnpm typecheck (apps/web) — passes.
  • pnpm vitest run src/common/logging.spec.ts src/deal-addons/strategies/ipni.strategy.spec.ts — 23/23 pass.
  • Post-merge: confirm BetterStack WHERE event = 'ipni_tracking_failed' GROUP BY failureReason still buckets cleanly (no deal-ID prefix regression).

Copilot AI review requested due to automatic review settings May 18, 2026 14:25
@FilOzzy FilOzzy added this to FOC May 18, 2026
@github-project-automation github-project-automation Bot moved this to 📌 Triage in FOC May 18, 2026
@SgtPooki SgtPooki self-assigned this May 18, 2026
@SgtPooki SgtPooki moved this from 📌 Triage to 🔎 Awaiting review in FOC May 18, 2026
Copy link
Copy Markdown
Collaborator Author

@SgtPooki SgtPooki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self review.. lgtm

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enforces a consistent code style across the repo by enabling Biome’s style/noNestedTernary rule as an error (CI-failing), and updates the two existing nested-ternary violations to compliant, clearer implementations. It also centralizes “flat” error-message extraction logic to preserve low-cardinality failureReason fields for logging/observability use cases.

Changes:

  • Enable Biome style/noNestedTernary: "error" repo-wide.
  • Refactor the web Landing page’s approved-SP dashboard URL selection to a Record<Network, ...> lookup.
  • Add getErrorMessage(error: unknown) in backend logging utilities and use it for ipni_tracking_failed.failureReason.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
biome.json Enforces noNestedTernary as an error to prevent nested ternaries repo-wide.
apps/web/src/pages/Landing.tsx Replaces nested ternary network URL selection with a typed lookup map.
apps/backend/src/deal-addons/strategies/ipni.strategy.ts Uses getErrorMessage for failureReason to avoid nested ternary and keep logs aggregable.
apps/backend/src/common/logging.ts Introduces getErrorMessage built on toStructuredError redaction semantics.

@SgtPooki SgtPooki requested a review from silent-cipher May 18, 2026 15:54
@github-project-automation github-project-automation Bot moved this from 🔎 Awaiting review to ✔️ Approved by reviewer in FOC May 18, 2026
@SgtPooki SgtPooki merged commit 3986d52 into main May 18, 2026
12 checks passed
@SgtPooki SgtPooki deleted the fix/biome-config branch May 18, 2026 19:26
@github-project-automation github-project-automation Bot moved this from ✔️ Approved by reviewer to 🎉 Done in FOC May 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🎉 Done

Development

Successfully merging this pull request may close these issues.

4 participants