Skip to content

Main branch build is broken: duplicate validateTeamSplits export in utils.ts fails next build (TS2393) #68

Description

@chonilius

Overview

src/lib/utils.ts currently exports two separate implementations of validateTeamSplits, back to back:

export function validateTeamSplits(splits: { percentage: number }[]): { valid: boolean; sum: number; message?: string } {
  if (!splits || splits.length === 0) return { valid: true, sum: 0 };
  const sum = splits.reduce((acc, s) => acc + (s.percentage ?? 0), 0);
  const tolerance = 0.01; // 0.01% tolerance for floating point
  const valid = Math.abs(sum - 100) <= tolerance;
  return { valid, sum, message: valid ? undefined : `Team splits sum to ${sum.toFixed(2)}% (expected 100%)` };
}

export function validateTeamSplits(
  splits: Array<{ percentage: string | number }>,
  tolerance = 0.01
): { valid: boolean; sum: number; message?: string } {
  const percentages = splits.map((s) =>
    typeof s.percentage === "string" ? Number(s.percentage) : s.percentage
  );
  const sum = percentages.reduce((a, b) => a + b, 0);
  const valid = Math.abs(sum - 100) <= tolerance;
  return {
    valid,
    sum,
    message: valid ? undefined : `Team splits sum to ${sum.toFixed(2)}% (expected 100%)`,
  };
}

This isn't a style nit — it's a hard compile failure. I built the app from a clean clone of main (HEAD 9c8f4b3, "Merge pull request #67 from legend4tech/feat/statcard-edge-cases") to verify:

$ npx tsc --noEmit
src/lib/utils.ts(50,17): error TS2323: Cannot redeclare exported variable 'validateTeamSplits'.
src/lib/utils.ts(50,17): error TS2393: Duplicate function implementation.
src/lib/utils.ts(58,17): error TS2323: Cannot redeclare exported variable 'validateTeamSplits'.
src/lib/utils.ts(58,17): error TS2393: Duplicate function implementation.

$ NEXT_PUBLIC_STELLAR_NETWORK=TESTNET NEXT_PUBLIC_API_URL=http://localhost:4000/api npx next build
▲ Next.js 16.2.10 (Turbopack)
Error: Turbopack build failed with 1 errors:
./src/lib/utils.ts:58:17
the name `validateTeamSplits` is defined multiple times

next build genuinely aborts. And .github/workflows/ci.yml's build-and-lint job runs exactly that command on every push/PR to main (step run: npm run build, after verify:env and before verify:headers) — so as of this HEAD, CI on main is red, and anyone pulling main to start a fresh feature can't produce a production build.

The interesting part is why nobody noticed: npm run lint (ESLint via eslint-config-next) doesn't flag this (no-redeclare isn't catching it here), and npm test (Jest via ts-jest) also passes cleanly — 11/11 green — because Jest never runs a whole-program type-check, and at the raw-JS level a second function validateTeamSplits(...) {} declaration is perfectly legal (it just silently shadows the first). So the app runs in next dev and tests pass, while next build/tsc --noEmit — the only two checks that do a full-program TypeScript pass — both fail. That's exactly the kind of break that slips through review if CI's build step isn't actually being watched.

Root cause, from git log -- src/lib/utils.ts: the second implementation (with tolerance as a parameter and string-or-number percentage) was added by commit 7c8260c "Fix #5: add team-split validation with rounding tolerance", on top of the first implementation that was already there from the initial commit — the PR that closed #5 appended a new implementation instead of replacing the old one.

Requirements

  • Delete one of the two validateTeamSplits implementations and keep the other, reconciling them into a single implementation and signature.
  • The kept implementation must satisfy both prior use cases: (a) accept percentage as either string | number (the second implementation's improvement, needed because TeamSplit.percentage is a number after coercePercentage but raw backend splits may arrive as numeric strings before coercion), and (b) support a configurable tolerance parameter (the second implementation's addition) while preserving a sane default (0.01).
  • Update every call site (src/lib/adapters.ts's adaptBounty, and any test/consumer) to the single, final signature.
  • Re-run npx tsc --noEmit and npm run build locally and confirm both are clean before considering this closed — passing npm test alone is not sufficient evidence, as demonstrated above.
  • Add a check to CI (or to npm run lint) that would have caught this class of error going forward — at minimum, confirm tsc --noEmit is actually part of the lint script or add it as its own CI step, since npm run build catching it only at the very end of the CI pipeline (after verify:env) wastes CI time on a build that was always going to fail structurally.

Acceptance Criteria

  • src/lib/utils.ts exports exactly one validateTeamSplits function.
  • npx tsc --noEmit exits 0 with no errors.
  • NEXT_PUBLIC_STELLAR_NETWORK=TESTNET NEXT_PUBLIC_API_URL=http://localhost:4000/api npx next build completes successfully (Turbopack build succeeds, no "defined multiple times" error).
  • npm test continues to pass, and an existing/new test exercises validateTeamSplits with both string and number percentage inputs and a non-default tolerance.
  • src/lib/adapters.ts's adaptBounty (the only current call site) still compiles and behaves identically for the existing test fixtures.
  • CI's build-and-lint job is confirmed green on the fix branch.

Additional Notes

Precise references (confirmed against main @ 9c8f4b3):

  • src/lib/utils.ts:50-56 — first validateTeamSplits implementation ({ percentage: number }[], fixed 0.01 tolerance).
  • src/lib/utils.ts:58-72 — second validateTeamSplits implementation (Array<{ percentage: string | number }>, configurable tolerance).
  • src/lib/adapters.ts:56-62,80 — the only current call site, inside adaptBounty: teamSplitsValid: splits ? validateTeamSplits(splits) : undefined, where splits is already TeamSplit[] (i.e. percentage: number) at that point, so the string-handling branch of the second implementation is currently unreachable from this call site — worth keeping anyway since it's a reasonable defensive API for future callers (e.g. a future maintainer-facing "create team split" form that reads raw string input from a percentage field before it's coerced).
  • .github/workflows/ci.yml:30-34 — the CI steps, in order: npm cinpm run lintnpm run verify:envnpm run buildnpm run verify:headersnpm audit. npm run build is the step that fails.
  • git log --oneline -- src/lib/utils.ts: 7c8260c ("Fix Validate team-split percentage sums and asset-precision-safe payout display #5: add team-split validation with rounding tolerance") is the commit that introduced the second implementation without removing the first; b1eb0f4 and 3a4eed2 precede it.

Edge cases to preserve in the merged implementation:

  • Empty/undefined splits array — first implementation explicitly short-circuits to { valid: true, sum: 0 }; the second implementation does not have this guard and would instead run [].reduce(...)sum: 0, then Math.abs(0 - 100) <= tolerancevalid: false for an empty array, which is a behavioral regression from the first implementation if the second is kept as-is verbatim. Whichever implementation is kept as the base, explicitly carry over the empty-array short-circuit — this is a real, silent behavior difference between the two, not just a signature difference.
  • Mixed string/number percentage values in the same array (partial coercion) — only the second implementation handles this.
  • A single split at exactly 100% (no team split, i.e. solo bounty) — should be valid: true.

Test/reproduction plan: add unit tests in a new src/lib/utils.test.ts (there currently is no test file for utils.ts at all — the only existing test file in the repo is src/components/ui/StatCard.test.tsx) covering: empty array, single 100% split, splits summing to exactly 100, splits within tolerance (e.g. 99.995/0.005), splits outside tolerance, string-typed percentages, and a custom non-default tolerance argument. Then re-run npx tsc --noEmit and npm run build as the actual regression guard for the compile-breaking half of this bug — no test can catch this class of duplicate-declaration bug, which is exactly why it shipped past npm test in the first place.

Metadata

Metadata

Assignees

Labels

GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardOfficial Campaign | FWC26Campaign: Official Campaign | FWC26Third CampaignCampaign: Third CampaignbugSomething isn't workingtestingTesting/QA infrastructurevery hardVery difficult task, expert-level effort required

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions