diff --git a/roadmap-planner/backend/config.example.yaml b/roadmap-planner/backend/config.example.yaml index 7745d1f..a4358f1 100644 --- a/roadmap-planner/backend/config.example.yaml +++ b/roadmap-planner/backend/config.example.yaml @@ -125,7 +125,6 @@ github: private_key_path: "" # path to the PEM file (mount via Secret) private_key_pem: "" # raw PEM contents (use env GITHUB_APP_PRIVATE_KEY_PEM in K8s) -<<<<<<< HEAD # GitLab ingestion — mirrors `github` for self-hosted GitLab installs. # Requires storage.enabled = true. gitlab: @@ -137,52 +136,31 @@ gitlab: backfill_days: 0 # 0 = inherit storage.backfill_days groups: - "devops/**" # "**" = whole subtree; "group/*" = direct children only - hydrate_diff: false # fetch additions/deletions/changed_files per merged MR - include_archived: false # include archived projects in the sweep - # W1 Pass B (2026-05-19). When true, every cycle also sweeps - # `/merge_requests?scope=all&author_username=` for every member in - # the derived allowlist (`team_analytics.{github,gitlab}_username_prefills` - # minus `team_analytics.member_denylist`). Captures MRs your team - # filed in projects outside `groups`. No-op when no prefills are - # configured — adopt gradually. + hydrate_diff: true # W6: fetch additions/deletions/changed_files per merged MR + include_archived: false + # W1 Pass B (2026-05-19): per-allowlisted-member instance-wide MR sweep. member_instance_sweep: false -# Team analytics — pillar attribution + identity prefills + W1 filters. +# Team analytics — pillar attribution + identity prefills + W1/W2/W4 knobs. # All keys here are optional and additive; the dashboard works without # them (it just shows the un-curated raw data). team_analytics: # Identity prefills — one-shot github_login / gitlab_username # assignment to Jira members on the first sync. Manual UI edits always # win on subsequent syncs. - github_login_prefills: + github_login_prefills: {} # daniel: danielfbm # jtcheng: chengjingtao - gitlab_username_prefills: + gitlab_username_prefills: {} # daniel: daniel # jtcheng: chengjingtao # W1 allowlist denylist. The effective allowlist is the union of the - # two prefill maps above minus the entries here. Use this to suppress - # ex-team-members or contractors who shouldn't count. + # two prefill maps above minus the entries here. member_denylist: [] # - gxjiao # - lmhe -======= -# Team analytics — pillar attribution + identity prefills + W1/W2 knobs. -# All keys here are optional and additive; the dashboard works without -# them (it just shows the un-curated raw data). -team_analytics: - # github_login_prefills shortcuts the first-time Jira → GitHub - # identity link for members the operator already knows. Manual UI - # edits always win on subsequent syncs. - github_login_prefills: {} - # daniel: danielfbm - # jtcheng: chengjingtao - gitlab_username_prefills: {} - # daniel: daniel - # jtcheng: chengjingtao # W2 (2026-05-19): explicit list of bot logins that get folded into - # the synthetic `bot` member. Match is exact + case-folded; no - # suffix magic. Adding a new bot here + restarting picks it up. + # the synthetic `bot` member. Match is exact + case-folded. bot_logins: [] # - alaudabot # - alaudaa-renovate @@ -190,5 +168,4 @@ team_analytics: # - copilot-pull-request-reviewer[bot] # - kilo-code-bot[bot] # - copilot ->>>>>>> ee78c5b (feat(roadmap-planner): W2 — fold bot contributions into synthetic bot member) # pillars: { … } # see docs/team-analytics/PROPOSAL.md \ No newline at end of file diff --git a/roadmap-planner/backend/internal/api/handlers/contributions.go b/roadmap-planner/backend/internal/api/handlers/contributions.go index d3e0ed4..3dbc076 100644 --- a/roadmap-planner/backend/internal/api/handlers/contributions.go +++ b/roadmap-planner/backend/internal/api/handlers/contributions.go @@ -264,7 +264,6 @@ func (h *ContributionsHandler) UpdateMember(c *gin.Context) { DisplayName *string `json:"display_name"` GitHubLogin *string `json:"github_login"` GitLabUsername *string `json:"gitlab_username"` - PillarID *string `json:"pillar_id"` } if err := c.ShouldBindJSON(&req); err != nil { c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) @@ -297,9 +296,6 @@ func (h *ContributionsHandler) UpdateMember(c *gin.Context) { if req.GitLabUsername != nil { existing.GitLabUsername = strings.ToLower(strings.TrimSpace(*req.GitLabUsername)) } - if req.PillarID != nil { - existing.PillarID = strings.TrimSpace(*req.PillarID) - } // Literal-overwrite — UpsertMember's COALESCE semantics (which // protect operator edits from being clobbered by the Jira sync) @@ -309,7 +305,6 @@ func (h *ContributionsHandler) UpdateMember(c *gin.Context) { DisplayName: existing.DisplayName, GitHubLogin: existing.GitHubLogin, GitLabUsername: existing.GitLabUsername, - PillarID: existing.PillarID, }); err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) return diff --git a/roadmap-planner/backend/internal/contributions/prefills.go b/roadmap-planner/backend/internal/contributions/prefills.go index 4a9938a..77b7952 100644 --- a/roadmap-planner/backend/internal/contributions/prefills.go +++ b/roadmap-planner/backend/internal/contributions/prefills.go @@ -72,7 +72,6 @@ func ApplyGitHubLoginPrefills(ctx context.Context, store storage.Store, prefills DisplayName: m.DisplayName, GitHubLogin: gh, GitLabUsername: m.GitLabUsername, - PillarID: m.PillarID, } if sErr := store.SetMemberIdentity(ctx, m.ID, ident); sErr != nil { failures = append(failures, fmt.Sprintf("%s→%s: %v", jid, gh, sErr)) @@ -122,7 +121,6 @@ func ApplyGitLabUsernamePrefills(ctx context.Context, store storage.Store, prefi DisplayName: m.DisplayName, GitHubLogin: m.GitHubLogin, GitLabUsername: gl, - PillarID: m.PillarID, } if sErr := store.SetMemberIdentity(ctx, m.ID, ident); sErr != nil { failures = append(failures, fmt.Sprintf("%s→%s: %v", jid, gl, sErr)) diff --git a/roadmap-planner/backend/internal/contributions/service_extras.go b/roadmap-planner/backend/internal/contributions/service_extras.go index 573f829..64c6e1c 100644 --- a/roadmap-planner/backend/internal/contributions/service_extras.go +++ b/roadmap-planner/backend/internal/contributions/service_extras.go @@ -143,32 +143,121 @@ func (s *Service) NetworkDensity(ctx context.Context, q storage.MemberWeekQuery) // 3. Cross-pillar review %. // - // A review is "cross-pillar" when the reviewer's pillar differs - // from the author's pillar AND both pillars are populated. Reviews - // where either side has no pillar are excluded from the - // denominator — we cannot answer the question without the data. + // W9 (2026-05-19) rewrite: `members.pillar_id` is gone. Both + // "sides" come from PillarMap: + // - PR pillar set: PillarsForRepo(pr.repo_id). + // - Reviewer pillar set: union of pillars across PRs the + // reviewer authored in [from, to). + // A review is cross-pillar when the two sets are disjoint AND + // both are non-empty. Empty either side → excluded from the + // denominator. + pm := s.PillarMap() + if !pm.Configured() { + return out, nil + } + revPillarSet, err := s.pillarsByAuthor(ctx, db, dialect, q, pm) + if err != nil { + return nil, fmt.Errorf("network cross-pillar reviewer-pillars: %w", err) + } xSQL := rebindSimple(dialect, ` - SELECT - COUNT(*) AS total, - SUM(CASE WHEN ma.pillar_id <> mr.pillar_id THEN 1 ELSE 0 END) AS xpillar + SELECT rv.reviewer_id, pr.repo_id FROM pr_reviews rv JOIN pull_requests pr ON pr.id = rv.pr_id - JOIN members ma ON ma.id = COALESCE(NULLIF(pr.author_id, ''), '') - JOIN members mr ON mr.id = COALESCE(NULLIF(rv.reviewer_id, ''), '') WHERE rv.submitted_at >= ? AND rv.submitted_at < ? AND rv.is_bot = 0 - AND ma.pillar_id IS NOT NULL AND ma.pillar_id <> '' - AND mr.pillar_id IS NOT NULL AND mr.pillar_id <> ''`) - var xTotal, xCross sql.NullInt64 - if err := db.QueryRowContext(ctx, xSQL, q.From, q.To).Scan(&xTotal, &xCross); err != nil { - return nil, fmt.Errorf("network cross-pillar: %w", err) + AND rv.reviewer_id IS NOT NULL AND rv.reviewer_id <> ''`) + rows2, err := db.QueryContext(ctx, xSQL, q.From, q.To) + if err != nil { + return nil, fmt.Errorf("network cross-pillar reviews: %w", err) + } + defer rows2.Close() + var xTotal, xCross int64 + for rows2.Next() { + var reviewerID, repoID string + if err := rows2.Scan(&reviewerID, &repoID); err != nil { + return nil, err + } + prPillars := setOfStrings(pm.PillarsForRepo(repoID)) + revPillars := revPillarSet[reviewerID] + if len(prPillars) == 0 || len(revPillars) == 0 { + continue + } + xTotal++ + if disjointSet(prPillars, revPillars) { + xCross++ + } } - if xTotal.Int64 > 0 { - out.CrossPillarReviewPct = float64(xCross.Int64) / float64(xTotal.Int64) * 100 + if err := rows2.Err(); err != nil { + return nil, err + } + if xTotal > 0 { + out.CrossPillarReviewPct = float64(xCross) / float64(xTotal) * 100 } return out, nil } +// pillarsByAuthor walks every PR authored in [q.From, q.To) and +// returns author_id → set-of-pillars derived from +// PillarMap.PillarsForRepo. +func (s *Service) pillarsByAuthor( + ctx context.Context, + db *sql.DB, + dialect storage.Dialect, + q storage.MemberWeekQuery, + pm *PillarMap, +) (map[string]map[string]struct{}, error) { + rows, err := db.QueryContext(ctx, rebindSimple(dialect, ` + SELECT author_id, repo_id + FROM pull_requests + WHERE created_at >= ? AND created_at < ? + AND author_id IS NOT NULL AND author_id <> ''`), + q.From, q.To) + if err != nil { + return nil, err + } + defer rows.Close() + out := map[string]map[string]struct{}{} + for rows.Next() { + var authorID, repoID string + if err := rows.Scan(&authorID, &repoID); err != nil { + return nil, err + } + pillars := pm.PillarsForRepo(repoID) + if len(pillars) == 0 { + continue + } + set, ok := out[authorID] + if !ok { + set = map[string]struct{}{} + out[authorID] = set + } + for _, p := range pillars { + set[p] = struct{}{} + } + } + return out, rows.Err() +} + +func setOfStrings(xs []string) map[string]struct{} { + out := make(map[string]struct{}, len(xs)) + for _, x := range xs { + out[x] = struct{}{} + } + return out +} + +func disjointSet(a, b map[string]struct{}) bool { + if len(a) > len(b) { + a, b = b, a + } + for x := range a { + if _, ok := b[x]; ok { + return false + } + } + return true +} + // ---------------------------------------------------------------------- // /api/contributions/pillars — Pillar throughput stack chart // ---------------------------------------------------------------------- diff --git a/roadmap-planner/backend/internal/storage/generic.go b/roadmap-planner/backend/internal/storage/generic.go index b0261ac..ccf301e 100644 --- a/roadmap-planner/backend/internal/storage/generic.go +++ b/roadmap-planner/backend/internal/storage/generic.go @@ -231,21 +231,20 @@ func (s *genericStore) UpsertMember(ctx context.Context, m Member) error { active = 1 } q := rebind(s.d, ` - INSERT INTO members (id, display_name, email, jira_account_id, github_login, gitlab_username, pillar_id, active, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + INSERT INTO members (id, display_name, email, jira_account_id, github_login, gitlab_username, active, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(id) DO UPDATE SET display_name = excluded.display_name, email = excluded.email, jira_account_id = excluded.jira_account_id, github_login = COALESCE(NULLIF(excluded.github_login, ''), members.github_login), gitlab_username = COALESCE(NULLIF(excluded.gitlab_username, ''), members.gitlab_username), - pillar_id = COALESCE(NULLIF(excluded.pillar_id, ''), members.pillar_id), active = excluded.active, updated_at = excluded.updated_at`) _, err := s.db.ExecContext(ctx, q, m.ID, m.DisplayName, nullable(m.Email), nullable(m.JiraAccountID), nullable(m.GitHubLogin), nullable(m.GitLabUsername), - nullable(m.PillarID), active, m.CreatedAt, m.UpdatedAt) + active, m.CreatedAt, m.UpdatedAt) return err } @@ -256,11 +255,11 @@ func (s *genericStore) UpsertMember(ctx context.Context, m Member) error { func (s *genericStore) SetMemberIdentity(ctx context.Context, id string, ident MemberIdentity) error { q := rebind(s.d, ` UPDATE members - SET display_name = ?, github_login = ?, gitlab_username = ?, pillar_id = ?, updated_at = ? + SET display_name = ?, github_login = ?, gitlab_username = ?, updated_at = ? WHERE id = ?`) res, err := s.db.ExecContext(ctx, q, ident.DisplayName, nullable(ident.GitHubLogin), nullable(ident.GitLabUsername), - nullable(ident.PillarID), time.Now().UTC(), id) + time.Now().UTC(), id) if err != nil { return err } @@ -275,7 +274,7 @@ func (s *genericStore) ListMembers(ctx context.Context) ([]Member, error) { rows, err := s.db.QueryContext(ctx, ` SELECT id, display_name, COALESCE(email, ''), COALESCE(jira_account_id, ''), COALESCE(github_login, ''), COALESCE(gitlab_username, ''), - COALESCE(pillar_id, ''), active, created_at, updated_at + active, created_at, updated_at FROM members ORDER BY display_name`) if err != nil { @@ -287,7 +286,7 @@ func (s *genericStore) ListMembers(ctx context.Context) ([]Member, error) { var m Member var active int if err := rows.Scan(&m.ID, &m.DisplayName, &m.Email, &m.JiraAccountID, - &m.GitHubLogin, &m.GitLabUsername, &m.PillarID, &active, &m.CreatedAt, &m.UpdatedAt); err != nil { + &m.GitHubLogin, &m.GitLabUsername, &active, &m.CreatedAt, &m.UpdatedAt); err != nil { return nil, err } m.Active = active != 0 diff --git a/roadmap-planner/backend/internal/storage/migrations/0007_drop_members_pillar.sql b/roadmap-planner/backend/internal/storage/migrations/0007_drop_members_pillar.sql new file mode 100644 index 0000000..8ecae44 --- /dev/null +++ b/roadmap-planner/backend/internal/storage/migrations/0007_drop_members_pillar.sql @@ -0,0 +1,29 @@ +-- ---------------------------------------------------------------------- +-- 0007_drop_members_pillar — W9 (2026-05-19). +-- +-- The `members.pillar_id` column was added back in B1's first cut as a +-- single-pillar tag for the operator drawer. Two problems with it: +-- +-- 1. Engineers contribute across pillars regularly. The per-member +-- pillar was a forced choice that didn't reflect reality. +-- 2. The audit showed all 21 members had `pillar_id = NULL` in +-- prod, so `cross_pillar_review_pct` (which compared +-- `ma.pillar_id <> mr.pillar_id`) was structurally always 0%. +-- +-- W9 (per Q9 / 2026-05-19 maintainer answer = option (i)) drops the +-- column outright. `cross_pillar_review_pct` is rewritten to derive +-- both sides from the PR's repo (via PillarMap.PillarsForRepo) and +-- the reviewer's pillar set (union of pillars across their recent PRs +-- in the same window). See service_extras.go::NetworkDensity. +-- +-- Postgres + SQLite ≥ 3.35 support `ALTER TABLE … DROP COLUMN`. We +-- already require SQLite ≥3.25 for `RENAME COLUMN` (migration 0003), +-- and the alauda devpod ships with 3.46. Production runs on +-- modernc.org/sqlite which embeds 3.50+. +-- ---------------------------------------------------------------------- + +-- SQLite refuses to DROP a column with a dependent index. Drop the +-- index first; both engines tolerate the explicit order. +DROP INDEX IF EXISTS idx_members_pillar; + +ALTER TABLE members DROP COLUMN pillar_id; diff --git a/roadmap-planner/backend/internal/storage/store.go b/roadmap-planner/backend/internal/storage/store.go index a4bce75..b0c2fbd 100644 --- a/roadmap-planner/backend/internal/storage/store.go +++ b/roadmap-planner/backend/internal/storage/store.go @@ -159,7 +159,6 @@ type MemberIdentity struct { DisplayName string GitHubLogin string GitLabUsername string - PillarID string } // Member is the join entity across Jira, GitHub, and GitLab. @@ -175,7 +174,6 @@ type Member struct { JiraAccountID string `json:"jira_account_id,omitempty"` GitHubLogin string `json:"github_login,omitempty"` GitLabUsername string `json:"gitlab_username,omitempty"` - PillarID string `json:"pillar_id,omitempty"` Active bool `json:"active"` CreatedAt time.Time `json:"created_at"` UpdatedAt time.Time `json:"updated_at"` diff --git a/roadmap-planner/backend/internal/storage/store_test.go b/roadmap-planner/backend/internal/storage/store_test.go index c128a4f..f708997 100644 --- a/roadmap-planner/backend/internal/storage/store_test.go +++ b/roadmap-planner/backend/internal/storage/store_test.go @@ -41,7 +41,6 @@ func TestSQLiteRoundTrip(t *testing.T) { Email: "alice@alauda.io", JiraAccountID: "abc-123", GitHubLogin: "alicetan", - PillarID: "ci-cd", Active: true, } if err := store.UpsertMember(ctx, mem); err != nil { @@ -55,13 +54,13 @@ func TestSQLiteRoundTrip(t *testing.T) { t.Fatalf("members round-trip failed: %+v", mems) } - // upsert again with new pillar — should update, not insert - mem.PillarID = "essentials" + // Upsert again with a new display name — should update, not insert. + mem.DisplayName = "Alice Tan (updated)" if err := store.UpsertMember(ctx, mem); err != nil { t.Fatalf("re-upsert member: %v", err) } mems, _ = store.ListMembers(ctx) - if len(mems) != 1 || mems[0].PillarID != "essentials" { + if len(mems) != 1 || mems[0].DisplayName != "Alice Tan (updated)" { t.Fatalf("upsert did not update: %+v", mems) }