Skip to content

refactor(teams): replace (T) sum-then-apply with (Σ) cumulative impact#43

Merged
msogin merged 2 commits into
mainfrom
feat/team-impact-sum-no-saturation
May 20, 2026
Merged

refactor(teams): replace (T) sum-then-apply with (Σ) cumulative impact#43
msogin merged 2 commits into
mainfrom
feat/team-impact-sum-no-saturation

Conversation

@msogin
Copy link
Copy Markdown
Contributor

@msogin msogin commented May 20, 2026

Summary

Follow-up to #42 — fixes the saturation issue in the Total (T) column on the Teams tab.

Problem

The (T) Total column applied the IC impact formula to team-level sums. Once a team crossed min(commits/20, 1) and min(PRs/10, 1) — about a single IC's monthly throughput — extra output no longer counted. Live example from /report/<id>/team?view=teams:

Active devs Commits PRs T (old) W
GDN 6 44 41 7.6 5.4
Integrations 11 230 49 7.4 5.7

Integrations had ~5× more commits but scored lower on T — the cap discarded all the extra output. Confusing for anyone trying to read team scale from the column.

Fix

Replace (T) with (Σ) — cumulative team impact = sum of active developers' impact_score.

  • No saturation: scales linearly with team size, bounded only by team_size × ~9.3.
  • Each IC's full contribution counts, capped only at IC scale (where the IC formula's per-dev caps belong).
  • Trivial to explain: "add up everyone's individual impact score".

After the change:

Active devs Σ (new) A W
GDN 6 ~30 5.2 5.4
Integrations 11 ~57 5.2 5.7

Now Integrations clearly dominates Σ as expected, while A and W stay as before (per-IC and per-headcount views).

What's in the diff

  • src/lib/teams/team-aggregator.ts — replaced the computeImpactScore({...team sums}) call with activeDevs.reduce(...) over per-dev impact_score. Field renamed impact_totalimpact_sum.
  • src/lib/__tests__/unit/team-aggregator.test.ts — replaced the "(T) sum-then-apply" test with a "(Σ) sum of impacts" test; updated the single-member-team identity (now Σ = A = dev's own impact_score instead of the old W = T).
  • src/app/report/[id]/team/team-table.tsx — column label (T)(Σ), tooltip rewritten, sort key updated.

Test plan

  • /report/<id>/team?view=teams — last impact column reads (Σ) in the header.
  • Integrations row's Σ value is meaningfully larger than GDN's (reflecting team size + per-IC quality).
  • Sort by Σ — works; column header caret toggles.
  • Σ for a single-active-member team equals that dev's stored impact_score.

🤖 Generated with Claude Code

The (T) Total column applied the IC impact formula to team-level sums,
which produced misleading results: once a team crossed the IC formula's
`min(commits/20, 1)` and `min(PRs/10, 1)` saturation thresholds, larger
teams could no longer outscore smaller ones on absolute output. Live
example: GDN (6 devs, 44 commits) reported a higher T than Integrations
(12 devs, 230 commits) — the cap discarded Integrations' 5× larger
volume.

Replace with (Σ) impact_sum: the arithmetic sum of active developers'
individual `impact_score`. Properties:

- No saturation — scales linearly with team size (bounded only by
  team_size × IC_max ≈ 9.3).
- Each IC's full contribution counts, capped only at IC scale (where
  the IC formula's per-dev caps belong).
- Trivial to explain: "add up everyone's individual impact score".
- Single-active-member team: Σ = A = the dev's own impact_score
  (the previous "W = T" invariant is gone, but the new Σ = A
  invariant pins down the degenerate case just as well).

Column header changes from `(T)` → `(Σ)`. Tooltip updated. Field
renamed from `impact_total` → `impact_sum` across the aggregator,
tests, and component sort keys.

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor Author

@msogin msogin left a comment

Choose a reason for hiding this comment

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

Code Review — automated (review-loop × smartling-review)

🟡 Recommend addressing before merge

src/lib/__tests__/unit/team-aggregator.test.tsMath.round rounding path untested

The new (Σ) impact_sum test uses only integer scores (7.0 + 6.0 + 5.0 = 18.0). The Math.round(impact_score_sum * 10) / 10 line in the aggregator is newly-introduced code that never gets exercised — integer inputs produce an integer sum with no rounding to do.

Consider adding one assertion with decimal scores whose sum crosses a rounding boundary:

// Confirms the Math.round(x * 10) / 10 rounding path
const devs = [
  { ...DEV_BASE, github_login: 'x', impact_score: 3.35 },
  { ...DEV_BASE, github_login: 'y', impact_score: 3.35 },
  { ...DEV_BASE, github_login: 'z', impact_score: 3.35 },
];
// 3.35 × 3 = 10.05 → rounds to 10.1
expect(row.impact_sum).toBe(10.1);

🟡 Minor display gap

src/app/report/[id]/team/team-table.tsximpact_sum cell shows 0.0 for all-inactive teams

Other conditional cells render when there's no data (e.g. avg_complexity, pr_percentage). For teams where active_count === 0, impact_sum will be 0.0, which could read as "the team scored zero" rather than "no active devs". Consider:

<td ...>{row.active_count > 0 ? row.impact_sum.toFixed(1) : '—'}</td>

🔵 Suggestions

src/lib/teams/team-aggregator.ts — Comment upper-bound uses team_size but should use active_count

The inline comment says "bounded by team_size × ~9.3" but impact_sum is computed over activeDevs, not total team members. Correct to:

// capped at the IC max (~9.3), so the column is bounded by active_count × ~9.3.

src/lib/teams/team-aggregator.tsimpact_sum and impact_avg round independently

impact_sum rounds impact_score_sum first; impact_avg divides impact_score_sum then rounds. This means impact_sum ≠ impact_avg × active_count in some cases. Worth a comment on the TeamRow interface:

impact_sum: number;  // NOT equal to impact_avg × active_count — rounded independently

🟣 Author's call

src/app/report/[id]/team/team-table.tsx — Tooltip: size-scaling confound

The new tooltip accurately describes the mechanics ("Scales with team size; no saturation") but doesn't caution against the sorting misuse: a 12-person team of average ICs will always outscore a 6-person team of high performers on (Σ). The old tooltip said "use as a context column, not a primary sort" — that guard is gone. If the intent is that (Σ) is explicitly a scale/size column and users are expected to pair it with (A)/(W) themselves, no change needed. Otherwise consider appending: "Sort by (A) or (W) to compare quality independent of team size."


Cross-cutting

Run a quick impact_total grep before merge to confirm the rename is complete — API routes, type exports, snapshot tests, and any downstream consumers of TeamRow aren't covered by this diff.

- Add test for the rounding path: 3 × 3.35 = 10.05 → asserts 10.1.
  The original Σ test used integer scores so the Math.round(x*10)/10 line
  was never exercised.

- Render '—' instead of '0.0' for impact_sum / impact_avg / impact_weighted
  when active_count === 0, matching the existing behavior of the ratio
  columns. An all-inactive team shouldn't read as if it scored zero.

- Fix inline comment: the Σ upper bound is active_count × ~9.3, not
  team_size × ~9.3 (the sum is over active devs, not all members).

- Document on the TeamRow type that impact_sum ≠ impact_avg × active_count
  in general — both are rounded independently to 1 decimal.

- Tooltip on the (Σ) column now points users at (A)/(W) for quality
  comparison independent of team size, replacing the warning the old (T)
  tooltip carried about not using as primary sort.

Co-Authored-By: Claude <noreply@anthropic.com>
@msogin msogin merged commit b34b4d9 into main May 20, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant