Skip to content

Commit 72ad6ad

Browse files
mickyarunmickyarun
committed
fix(learnings): fold merged + external contributor rows into active target
Two issues on the BUD detail Learnings tab: 1. The persisted contributor breakdown showed a separate row per GitHub identity even when the user was already merged via Settings → Members. The screenshot reproducer: "Yash Tiwari" with 14 commits and "yasht01 (external)" with 1 PR — same person, but the PR row didn't fold in because the persisted snapshot is frozen at close. 2. The inner section cards were rendered with v-card variant=outlined whose surface background and rounded corners stacked on top of the parent section-content-panel's own surface + corners, producing a double-card visual with a visible corner artifact at the bottom. For (1), introduce a read-time resolver that walks the Members → Merge backlink (deactivated_user.email → UserEmailAlias → target user) and folds the row's counts into the target's existing entry. Counts sum; active_days uses max because two rows for the same BUD can't legitimately exceed the BUD's active-day window. Multi-hop chains (A merged into B, then B merged into C) are supported via a visited-set guarded walk. The stored JSONB is left alone — the resolver runs on every fetch so later alias changes apply immediately, no backfill needed. For (2), replace the inner v-cards with plain <section> divs styled to match the metric-tile pattern at the top of the panel: single border, faint surface-variant tint, no nested rounded surface. Changes: - backend/app/repositories/user.py: add find_user_by_alias_email (single-hop alias backlink lookup, no is_active filter so the resolver can traverse deactivated intermediates). - backend/app/services/bud_learning_alias_resolver.py: new file with resolve_aliased_contributors + _walk_to_active. - backend/app/api/v1/bud.py: wire the resolver into get_bud_learning via model_copy so the SQLAlchemy ORM instance stays clean. - frontend/src/components/buds/LearningsPanel.vue: flatten the inner cards into sections matching the metric-tile pattern. Signed-off-by: Arun Rajkumar <mickyarunr@gmail.com>
1 parent dde83bc commit 72ad6ad

4 files changed

Lines changed: 261 additions & 70 deletions

File tree

‎backend/app/api/v1/bud.py‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@
9696
from app.services.bud_assignment_actions import assign_bud, unassign_bud
9797
from app.services.bud_edit_policy import assert_section_editable
9898
from app.services.bud_estimation import estimate_bud_dates
99+
from app.services.bud_learning_alias_resolver import resolve_aliased_contributors
99100
from app.services.bud_timeline import record_event
100101
from app.services.job_queue import JOB_BUD_AGENT, create_job
101102

@@ -544,7 +545,18 @@ async def get_bud_learning(
544545
status_code=status.HTTP_404_NOT_FOUND,
545546
detail="No learning recorded for this BUD yet",
546547
)
547-
return BUDLearningRead.model_validate(learning)
548+
read = BUDLearningRead.model_validate(learning)
549+
# Apply the Settings → Members merge backlink to the stored
550+
# contributor snapshot so deactivated / "(external)" rows fold into
551+
# their currently-active target. The stored JSONB is left alone —
552+
# the resolver runs on every fetch so later alias changes apply
553+
# immediately without a backfill.
554+
if read.metrics and read.metrics.get("contributors"):
555+
contribs = await resolve_aliased_contributors(
556+
db, current_user.org_id, read.metrics["contributors"]
557+
)
558+
read = read.model_copy(update={"metrics": {**read.metrics, "contributors": contribs}})
559+
return read
548560

549561

550562
# Status transitions QA owns directly via PATCH. Matches the manual-testing

‎backend/app/repositories/user.py‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -711,6 +711,33 @@ async def add_email_alias(
711711
self._db.add(alias)
712712
return alias
713713

714+
async def find_user_by_alias_email(
715+
self, org_id: uuid.UUID, email: str
716+
) -> User | None:
717+
"""Return the user who has ``email`` listed as a UserEmailAlias.
718+
719+
Walks one hop of the Settings → Members merge backlink: when
720+
member B is merged into A, B's primary email is recorded as an
721+
alias on A. Given B's email, this returns A.
722+
723+
Returns the immediate target without filtering on ``is_active``
724+
so multi-hop chains (A → B → C) can be traversed externally;
725+
callers that need a guaranteed-active user must loop until
726+
``user.is_active`` is true.
727+
"""
728+
if not email:
729+
return None
730+
result = await self._db.execute(
731+
select(User)
732+
.join(UserEmailAlias, UserEmailAlias.user_id == User.id)
733+
.where(
734+
UserEmailAlias.org_id == org_id,
735+
UserEmailAlias.email == email,
736+
)
737+
.limit(1)
738+
)
739+
return result.scalar_one_or_none()
740+
714741
async def list_aliases(self, user_id: uuid.UUID) -> list[UserEmailAlias]:
715742
"""List all email aliases for a user.
716743

‎backend/app/services/bud_learning_alias_resolver.py‎

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
# Copyright 2025-2026 Arun Rajkumar
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
"""Fold deactivated / external contributor rows into their merge target.
16+
17+
The contributor breakdown persisted at BUD close is a snapshot of who
18+
held an account when the Learning Agent ran. When Settings → Members
19+
later merges user B into user A, B's primary email becomes a
20+
``UserEmailAlias`` on A and B is deactivated — but the persisted
21+
contributor row still names B (or shows the PR as an "(external)"
22+
github_login row when B never had a user_id at PR-ingest time).
23+
24+
This module is invoked at read time by the GET /buds/{id}/learning
25+
endpoint to re-attribute those rows to the currently-active merge
26+
target via the alias backlink, without rewriting the stored JSONB.
27+
The stored snapshot stays untouched so a later un-merge or alias
28+
change is reflected on the next fetch.
29+
"""
30+
31+
from __future__ import annotations
32+
33+
import uuid
34+
from typing import Any
35+
36+
from sqlalchemy.ext.asyncio import AsyncSession
37+
38+
from app.models.user import User
39+
from app.repositories.user import UserRepository
40+
41+
42+
async def resolve_aliased_contributors(
43+
db: AsyncSession,
44+
org_id: uuid.UUID,
45+
contributors: list[dict[str, Any]],
46+
) -> list[dict[str, Any]]:
47+
"""Return ``contributors`` with merge-target rows folded together."""
48+
if not contributors:
49+
return contributors
50+
user_repo = UserRepository(db)
51+
52+
by_uid: dict[str, dict[str, Any]] = {}
53+
pending: list[dict[str, Any]] = []
54+
55+
for row in contributors:
56+
target = await _resolve_active_target(db, user_repo, org_id, row)
57+
if target is None:
58+
pending.append(dict(row))
59+
continue
60+
key = str(target.id)
61+
existing = by_uid.get(key)
62+
counts = _row_counts(row)
63+
if existing is None:
64+
by_uid[key] = {
65+
"user_id": key,
66+
"github_login": None,
67+
"name": target.name or target.email,
68+
**counts,
69+
}
70+
else:
71+
existing["commits"] += counts["commits"]
72+
existing["prs_merged"] += counts["prs_merged"]
73+
existing["todos_completed"] += counts["todos_completed"]
74+
existing["active_days"] = max(existing["active_days"], counts["active_days"])
75+
76+
merged = list(by_uid.values()) + pending
77+
merged.sort(
78+
key=lambda r: (r.get("commits", 0), r.get("prs_merged", 0)),
79+
reverse=True,
80+
)
81+
return merged
82+
83+
84+
async def _resolve_active_target(
85+
db: AsyncSession,
86+
user_repo: UserRepository,
87+
org_id: uuid.UUID,
88+
row: dict[str, Any],
89+
) -> User | None:
90+
"""Return the currently-active User for a contributor row, if any.
91+
92+
Resolution order: explicit user_id (active → return; deactivated →
93+
walk merge backlink), then github_login (resolve via username,
94+
walk backlink if deactivated). Returns None when nothing resolves
95+
to an active user — the caller then preserves the row verbatim so
96+
truly-external collaborators stay visible.
97+
"""
98+
uid_str = row.get("user_id")
99+
if uid_str:
100+
try:
101+
user = await db.get(User, uuid.UUID(str(uid_str)))
102+
except ValueError:
103+
user = None
104+
return await _walk_to_active(user_repo, org_id, user)
105+
106+
login = row.get("github_login")
107+
if not login:
108+
return None
109+
resolved_uid = await user_repo.get_id_by_github_login(org_id, login)
110+
if resolved_uid is None:
111+
return None
112+
user = await db.get(User, resolved_uid)
113+
return await _walk_to_active(user_repo, org_id, user)
114+
115+
116+
async def _walk_to_active(
117+
user_repo: UserRepository,
118+
org_id: uuid.UUID,
119+
user: User | None,
120+
) -> User | None:
121+
"""Follow the alias backlink chain until an active user is found.
122+
123+
Members → Merge can be applied repeatedly (A → B, then B → C). Each
124+
hop records the source's email as a UserEmailAlias on the target,
125+
so we walk while the current user is deactivated. A visited set
126+
guards against alias cycles that would otherwise loop forever.
127+
"""
128+
visited: set[uuid.UUID] = set()
129+
while user is not None and not user.is_active:
130+
if user.id in visited:
131+
return None
132+
visited.add(user.id)
133+
next_user = await user_repo.find_user_by_alias_email(org_id, user.email)
134+
if next_user is None or next_user.id == user.id:
135+
return None
136+
user = next_user
137+
return user
138+
139+
140+
def _row_counts(row: dict[str, Any]) -> dict[str, int]:
141+
"""Coerce the four count fields to ints with a 0 default."""
142+
return {
143+
"commits": int(row.get("commits") or 0),
144+
"prs_merged": int(row.get("prs_merged") or 0),
145+
"todos_completed": int(row.get("todos_completed") or 0),
146+
"active_days": int(row.get("active_days") or 0),
147+
}

‎frontend/src/components/buds/LearningsPanel.vue‎

Lines changed: 74 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -70,72 +70,66 @@
7070
</div>
7171

7272
<!-- Phase drift -->
73-
<v-card v-if="phaseRows.length" variant="outlined" class="learnings-panel__card">
74-
<v-card-title class="learnings-panel__card-title">Phase drift</v-card-title>
75-
<v-card-text>
76-
<v-table density="compact">
77-
<thead>
78-
<tr>
79-
<th>Phase</th>
80-
<th class="text-right">Estimated</th>
81-
<th class="text-right">Actual</th>
82-
<th class="text-right">Drift</th>
83-
</tr>
84-
</thead>
85-
<tbody>
86-
<tr v-for="row in phaseRows" :key="row.phase">
87-
<td>{{ row.phase }}</td>
88-
<td class="text-right">{{ formatDays(row.estimated_days) }}</td>
89-
<td class="text-right">{{ formatDays(row.actual_days) }}</td>
90-
<td class="text-right" :class="driftClass(row.drift_pct)">
91-
{{ formatDrift(row.drift_pct) }}
92-
</td>
93-
</tr>
94-
</tbody>
95-
</v-table>
96-
</v-card-text>
97-
</v-card>
73+
<section v-if="phaseRows.length" class="learnings-panel__section">
74+
<h3 class="learnings-panel__section-title">Phase drift</h3>
75+
<v-table density="compact" class="learnings-panel__table">
76+
<thead>
77+
<tr>
78+
<th>Phase</th>
79+
<th class="text-right">Estimated</th>
80+
<th class="text-right">Actual</th>
81+
<th class="text-right">Drift</th>
82+
</tr>
83+
</thead>
84+
<tbody>
85+
<tr v-for="row in phaseRows" :key="row.phase">
86+
<td>{{ row.phase }}</td>
87+
<td class="text-right">{{ formatDays(row.estimated_days) }}</td>
88+
<td class="text-right">{{ formatDays(row.actual_days) }}</td>
89+
<td class="text-right" :class="driftClass(row.drift_pct)">
90+
{{ formatDrift(row.drift_pct) }}
91+
</td>
92+
</tr>
93+
</tbody>
94+
</v-table>
95+
</section>
9896

9997
<!-- Contributors -->
100-
<v-card v-if="contributorRows.length" variant="outlined" class="learnings-panel__card">
101-
<v-card-title class="learnings-panel__card-title">Contributors</v-card-title>
102-
<v-card-text>
103-
<v-table density="compact">
104-
<thead>
105-
<tr>
106-
<th>Name</th>
107-
<th class="text-right">Commits</th>
108-
<th class="text-right">PRs merged</th>
109-
<th class="text-right">TODOs done</th>
110-
<th class="text-right">Active days</th>
111-
</tr>
112-
</thead>
113-
<tbody>
114-
<!-- External collaborators have user_id=null but
115-
github_login set, so key on whichever is present. -->
116-
<tr
117-
v-for="row in contributorRows"
118-
:key="row.user_id ?? row.github_login ?? row.name"
119-
>
120-
<td>{{ row.name }}</td>
121-
<td class="text-right">{{ row.commits }}</td>
122-
<td class="text-right">{{ row.prs_merged }}</td>
123-
<td class="text-right">{{ row.todos_completed }}</td>
124-
<td class="text-right">{{ row.active_days }}</td>
125-
</tr>
126-
</tbody>
127-
</v-table>
128-
</v-card-text>
129-
</v-card>
98+
<section v-if="contributorRows.length" class="learnings-panel__section">
99+
<h3 class="learnings-panel__section-title">Contributors</h3>
100+
<v-table density="compact" class="learnings-panel__table">
101+
<thead>
102+
<tr>
103+
<th>Name</th>
104+
<th class="text-right">Commits</th>
105+
<th class="text-right">PRs merged</th>
106+
<th class="text-right">TODOs done</th>
107+
<th class="text-right">Active days</th>
108+
</tr>
109+
</thead>
110+
<tbody>
111+
<!-- External collaborators have user_id=null but
112+
github_login set, so key on whichever is present. -->
113+
<tr
114+
v-for="row in contributorRows"
115+
:key="row.user_id ?? row.github_login ?? row.name"
116+
>
117+
<td>{{ row.name }}</td>
118+
<td class="text-right">{{ row.commits }}</td>
119+
<td class="text-right">{{ row.prs_merged }}</td>
120+
<td class="text-right">{{ row.todos_completed }}</td>
121+
<td class="text-right">{{ row.active_days }}</td>
122+
</tr>
123+
</tbody>
124+
</v-table>
125+
</section>
130126

131127
<!-- Retrospective markdown -->
132-
<v-card v-if="learning.retrospective_md" variant="outlined" class="learnings-panel__card">
133-
<v-card-title class="learnings-panel__card-title">Retrospective</v-card-title>
134-
<v-card-text class="learnings-panel__retro-body">
135-
<!-- eslint-disable-next-line vue/no-v-html -->
136-
<article class="markdown-body markdown-body--numeric" v-html="renderedRetro" />
137-
</v-card-text>
138-
</v-card>
128+
<section v-if="learning.retrospective_md" class="learnings-panel__section">
129+
<h3 class="learnings-panel__section-title">Retrospective</h3>
130+
<!-- eslint-disable-next-line vue/no-v-html -->
131+
<article class="markdown-body markdown-body--numeric" v-html="renderedRetro" />
132+
</section>
139133
</template>
140134
</div>
141135
</template>
@@ -229,20 +223,31 @@ function driftClass(value: number | null | undefined): string {
229223
font-weight: 500;
230224
}
231225
232-
.learnings-panel__card {
233-
border-color: rgba(var(--v-theme-on-surface), 0.08);
226+
.learnings-panel__section {
227+
padding: 12px 16px 16px;
228+
border: 1px solid rgba(var(--v-theme-on-surface), 0.08);
229+
border-radius: 8px;
230+
background: rgba(var(--v-theme-surface-variant), 0.1);
234231
}
235232
236-
.learnings-panel__card-title {
237-
font-size: 13px;
233+
.learnings-panel__section-title {
234+
font-size: 12px;
238235
font-weight: 600;
239236
letter-spacing: 0.04em;
240237
text-transform: uppercase;
241238
color: rgba(var(--v-theme-on-surface), 0.7);
242-
padding-bottom: 4px;
239+
margin: 0 0 8px;
240+
}
241+
242+
.learnings-panel__table {
243+
background: transparent;
244+
}
245+
246+
.learnings-panel__table :deep(.v-table__wrapper) {
247+
background: transparent;
243248
}
244249
245-
.learnings-panel__retro-body {
246-
padding-top: 0;
250+
.learnings-panel__table :deep(table) {
251+
background: transparent;
247252
}
248253
</style>

0 commit comments

Comments
 (0)