diff --git a/src/group.test.ts b/src/group.test.ts index f611cda..fef878d 100644 --- a/src/group.test.ts +++ b/src/group.test.ts @@ -1,15 +1,21 @@ import { describe, expect, it } from "bun:test"; import { applyTeamPick, + applyTeamPickInTree, consolidateTeamHierarchy, + findCombinedSectionPaths, flattenTeamHierarchy, flattenTeamSections, groupByTeamHierarchy, groupByTeamPrefix, moveRepoToSection, + moveRepoToSectionInTree, + rebuildTeamHierarchy, rebuildTeamSections, undoPickedRepo, + undoPickedRepoInTree, undoSectionPick, + undoSectionPickInTree, } from "./group.ts"; import type { RepoGroup, TeamSection } from "./types.ts"; @@ -494,6 +500,320 @@ describe("flattenTeamHierarchy", () => { }); }); +// ─── rebuildTeamHierarchy ────────────────────────────────────────────────────── + +describe("rebuildTeamHierarchy", () => { + it("round-trips a 2-level tree through flattenTeamHierarchy", () => { + const groups = [ + makeGroup("org/a", ["gamme-client", "squad-dashboard"]), + makeGroup("org/b", ["gamme-client", "squad-billing"]), + ]; + const original = groupByTeamHierarchy(groups, [["gamme-", "squad-"]]); + const rebuilt = rebuildTeamHierarchy(flattenTeamHierarchy(original)); + expect(rebuilt).toEqual(original); + }); + + it("round-trips a tree where a node has both own groups and children (overlap parent)", () => { + const groups = [ + makeGroup("org/a", ["gamme-lead-client"]), + makeGroup("org/b", ["gamme-lead-client-p1"]), + ]; + const original = groupByTeamHierarchy(groups, [["gamme-"]]); + const rebuilt = rebuildTeamHierarchy(flattenTeamHierarchy(original)); + expect(rebuilt).toEqual(original); + }); + + it("round-trips multiple independent top-level chains", () => { + const groups = [ + makeGroup("org/a", ["gamme-client", "squad-dashboard"]), + makeGroup("org/b", ["chapter-backend"]), + makeGroup("org/c", []), + ]; + const original = groupByTeamHierarchy(groups, [["gamme-", "squad-"], ["chapter-"]]); + const rebuilt = rebuildTeamHierarchy(flattenTeamHierarchy(original)); + expect(rebuilt).toEqual(original); + }); + + it("returns an empty array for an empty input", () => { + expect(rebuildTeamHierarchy([])).toEqual([]); + }); +}); + +// ─── applyTeamPickInTree ──────────────────────────────────────────────────────── + +describe("applyTeamPickInTree", () => { + it("behaves like applyTeamPick for a top-level (depth-1) path", () => { + const flatSections: TeamSection[] = [ + { label: "squad-frontend", groups: [makeGroup("org/a", ["squad-frontend"])] }, + { + label: "squad-frontend + squad-mobile", + groups: [makeGroup("org/shared", ["squad-frontend", "squad-mobile"])], + }, + ]; + const viaFlat = applyTeamPick(flatSections, "squad-frontend + squad-mobile", "squad-frontend"); + const viaTree = applyTeamPickInTree( + flatSections, + ["squad-frontend + squad-mobile"], + "squad-frontend", + ); + expect(viaTree).toEqual(viaFlat); + }); + + it("reassigns a nested combined section to a sibling at the same depth", () => { + const groups = [ + makeGroup("org/shared", ["gamme-client", "squad-a", "squad-b"]), + makeGroup("org/a", ["gamme-client", "squad-a"]), + ]; + const tree = groupByTeamHierarchy(groups, [["gamme-", "squad-"]]); + const updated = applyTeamPickInTree(tree, ["gamme-client", "squad-a + squad-b"], "squad-a"); + const gamme = updated.find((s) => s.label === "gamme-client")!; + const childLabels = (gamme.children ?? []).map((c) => c.label); + expect(childLabels).not.toContain("squad-a + squad-b"); + const squadA = gamme.children!.find((c) => c.label === "squad-a")!; + expect(squadA.groups.map((g) => g.repoFullName).toSorted()).toEqual(["org/a", "org/shared"]); + }); + + it("tags moved repos with pickedFrom = joined path", () => { + const groups = [makeGroup("org/shared", ["gamme-client", "squad-a", "squad-b"])]; + const tree = groupByTeamHierarchy(groups, [["gamme-", "squad-"]]); + const updated = applyTeamPickInTree(tree, ["gamme-client", "squad-a + squad-b"], "squad-a"); + const gamme = updated.find((s) => s.label === "gamme-client")!; + const squadA = gamme.children!.find((c) => c.label === "squad-a")!; + expect(squadA.groups[0].pickedFrom).toBe("gamme-client > squad-a + squad-b"); + }); + + it("creates a new sibling section when the chosen team has none yet", () => { + const groups = [makeGroup("org/shared", ["gamme-client", "squad-a", "squad-b"])]; + const tree = groupByTeamHierarchy(groups, [["gamme-", "squad-"]]); + const updated = applyTeamPickInTree(tree, ["gamme-client", "squad-a + squad-b"], "squad-b"); + const gamme = updated.find((s) => s.label === "gamme-client")!; + expect(gamme.children!.map((c) => c.label)).toContain("squad-b"); + }); + + it("is a no-op when a path segment is not found", () => { + const groups = [makeGroup("org/shared", ["gamme-client", "squad-a", "squad-b"])]; + const tree = groupByTeamHierarchy(groups, [["gamme-", "squad-"]]); + const result = applyTeamPickInTree(tree, ["nope", "squad-a + squad-b"], "squad-a"); + expect(result).toEqual(tree); + }); + + it("preserves the picked section's own children (does not drop the subtree)", () => { + // Regression: a top-level combined section ("gamme-a + gamme-a-security-p1") + // that was already subdivided by the next chain level (squad-) must keep + // its nested children when picked — only its own (now empty) `groups` + // were carried over before the fix, silently dropping every repo nested + // underneath. + const groups = [ + makeGroup("org/tools-mobile", [ + "gamme-lead-mobile", + "gamme-lead-mobile-security-p1", + "squad-core", + "squad-mobile", + ]), + makeGroup("org/wizard-mobile", ["gamme-lead-mobile", "gamme-lead-mobile-security-p1"]), + ]; + const tree = groupByTeamHierarchy(groups, [["gamme-", "squad-"]]); + const combined = tree.find((s) => s.label.includes(" + "))!; + expect(combined.label).toBe("gamme-lead-mobile + gamme-lead-mobile-security-p1"); + expect(combined.groups).toEqual([]); // fully subdivided by squad- before the pick + expect(combined.children).toHaveLength(2); // "squad-core + squad-mobile" and "other" + + const updated = applyTeamPickInTree(tree, [combined.label], "gamme-lead-mobile"); + + expect(updated.map((s) => s.label)).not.toContain(combined.label); + const picked = updated.find((s) => s.label === "gamme-lead-mobile")!; + expect(picked).toBeDefined(); + expect(picked.children).toHaveLength(2); + const squadChild = picked.children!.find((c) => c.label === "squad-core + squad-mobile")!; + expect(squadChild.groups.map((g) => g.repoFullName)).toEqual(["org/tools-mobile"]); + const otherChild = picked.children!.find((c) => c.label === "other")!; + expect(otherChild.groups.map((g) => g.repoFullName)).toEqual(["org/wizard-mobile"]); + // Every repo in the moved subtree is tagged, not just the top node's own groups. + expect(squadChild.groups[0].pickedFrom).toBe(combined.label); + expect(otherChild.groups[0].pickedFrom).toBe(combined.label); + }); + + it("merges the picked subtree's children into an existing target section's children", () => { + const groups = [ + makeGroup("org/existing", ["gamme-lead-mobile", "squad-existing"]), + makeGroup("org/tools-mobile", [ + "gamme-lead-mobile", + "gamme-lead-mobile-security-p1", + "squad-core", + ]), + ]; + const tree = groupByTeamHierarchy(groups, [["gamme-", "squad-"]]); + const combined = tree.find((s) => s.label.includes(" + "))!; + const updated = applyTeamPickInTree(tree, [combined.label], "gamme-lead-mobile"); + const picked = updated.find((s) => s.label === "gamme-lead-mobile")!; + const childLabels = picked.children!.map((c) => c.label).toSorted(); + expect(childLabels).toEqual(["squad-core", "squad-existing"]); + }); + + it("returns sections unchanged for an empty combinedPath", () => { + const groups = [makeGroup("org/a")]; + const tree = groupByTeamHierarchy(groups, [["squad-"]]); + expect(applyTeamPickInTree(tree, [], "squad-a")).toBe(tree); + }); +}); + +// ─── undoSectionPickInTree ────────────────────────────────────────────────────── + +describe("undoSectionPickInTree", () => { + it("restores every repo tagged with the matching pickedFrom back to the combined section", () => { + const groups = [ + makeGroup("org/shared", ["gamme-client", "squad-a", "squad-b"]), + makeGroup("org/a", ["gamme-client", "squad-a"]), + ]; + const tree = groupByTeamHierarchy(groups, [["gamme-", "squad-"]]); + const picked = applyTeamPickInTree(tree, ["gamme-client", "squad-a + squad-b"], "squad-a"); + const restored = undoSectionPickInTree(picked, "gamme-client > squad-a + squad-b"); + const gamme = restored.find((s) => s.label === "gamme-client")!; + const childLabels = gamme.children!.map((c) => c.label).toSorted(); + expect(childLabels).toEqual(["squad-a", "squad-a + squad-b"]); + const combined = gamme.children!.find((c) => c.label === "squad-a + squad-b")!; + expect(combined.groups.map((g) => g.repoFullName)).toEqual(["org/shared"]); + expect(combined.groups[0].pickedFrom).toBeUndefined(); + }); + + it("drops a section left empty after the restore", () => { + const groups = [makeGroup("org/shared", ["gamme-client", "squad-a", "squad-b"])]; + const tree = groupByTeamHierarchy(groups, [["gamme-", "squad-"]]); + const picked = applyTeamPickInTree(tree, ["gamme-client", "squad-a + squad-b"], "squad-a"); + const restored = undoSectionPickInTree(picked, "gamme-client > squad-a + squad-b"); + const gamme = restored.find((s) => s.label === "gamme-client")!; + // squad-a only ever held the moved repo — it must be gone after the restore. + expect(gamme.children!.map((c) => c.label)).not.toContain("squad-a"); + }); + + it("is a no-op when no repo has a matching pickedFrom", () => { + const groups = [makeGroup("org/a", ["squad-a", "squad-b"])]; + const tree = groupByTeamHierarchy(groups, [["squad-"]]); + expect(undoSectionPickInTree(tree, "nope")).toBe(tree); + }); + + it("behaves like undoSectionPick for a top-level (depth-1) path", () => { + const flatSections: TeamSection[] = [ + { + label: "squad-frontend", + groups: [{ ...makeGroup("org/shared"), pickedFrom: "squad-frontend + squad-mobile" }], + }, + ]; + const viaFlat = undoSectionPick( + flattenTeamSections(flatSections), + "squad-frontend + squad-mobile", + ); + const viaTree = flattenTeamHierarchy( + undoSectionPickInTree( + rebuildTeamHierarchy(flattenTeamSections(flatSections)), + "squad-frontend + squad-mobile", + ), + ); + expect(viaTree.map((g) => g.repoFullName)).toEqual(viaFlat.map((g) => g.repoFullName)); + }); +}); + +// ─── moveRepoToSectionInTree ──────────────────────────────────────────────────── + +describe("moveRepoToSectionInTree", () => { + it("moves a repo to a sibling under the given parent path", () => { + const groups = [ + makeGroup("org/shared", ["gamme-client", "squad-a", "squad-b"]), + makeGroup("org/a", ["gamme-client", "squad-a"]), + ]; + const tree = groupByTeamHierarchy(groups, [["gamme-", "squad-"]]); + const picked = applyTeamPickInTree(tree, ["gamme-client", "squad-a + squad-b"], "squad-a"); + const moved = moveRepoToSectionInTree(picked, "org/shared", ["gamme-client"], "squad-b"); + const gamme = moved.find((s) => s.label === "gamme-client")!; + const squadB = gamme.children!.find((c) => c.label === "squad-b")!; + expect(squadB.groups.map((g) => g.repoFullName)).toEqual(["org/shared"]); + const squadA = gamme.children!.find((c) => c.label === "squad-a")!; + expect(squadA.groups.map((g) => g.repoFullName)).toEqual(["org/a"]); + }); + + it("creates the target section when it doesn't exist yet", () => { + const groups = [makeGroup("org/shared", ["gamme-client", "squad-a", "squad-b"])]; + const tree = groupByTeamHierarchy(groups, [["gamme-", "squad-"]]); + const picked = applyTeamPickInTree(tree, ["gamme-client", "squad-a + squad-b"], "squad-a"); + const moved = moveRepoToSectionInTree(picked, "org/shared", ["gamme-client"], "squad-c"); + const gamme = moved.find((s) => s.label === "gamme-client")!; + expect(gamme.children!.map((c) => c.label)).toContain("squad-c"); + }); + + it("is a no-op when the repo is not found anywhere in the tree", () => { + const groups = [makeGroup("org/a", ["squad-a"])]; + const tree = groupByTeamHierarchy(groups, [["squad-"]]); + expect(moveRepoToSectionInTree(tree, "org/does-not-exist", [], "squad-b")).toBe(tree); + }); +}); + +// ─── undoPickedRepoInTree ─────────────────────────────────────────────────────── + +describe("undoPickedRepoInTree", () => { + it("restores a single picked repo back to its original combined section", () => { + const groups = [ + makeGroup("org/shared", ["gamme-client", "squad-a", "squad-b"]), + makeGroup("org/a", ["gamme-client", "squad-a"]), + ]; + const tree = groupByTeamHierarchy(groups, [["gamme-", "squad-"]]); + const picked = applyTeamPickInTree(tree, ["gamme-client", "squad-a + squad-b"], "squad-a"); + const restored = undoPickedRepoInTree(picked, "org/shared"); + const gamme = restored.find((s) => s.label === "gamme-client")!; + const combined = gamme.children!.find((c) => c.label === "squad-a + squad-b")!; + expect(combined.groups.map((g) => g.repoFullName)).toEqual(["org/shared"]); + expect(combined.groups[0].pickedFrom).toBeUndefined(); + // The other repo that was also moved stays picked. + const squadA = gamme.children!.find((c) => c.label === "squad-a")!; + expect(squadA.groups.map((g) => g.repoFullName)).toEqual(["org/a"]); + }); + + it("is a no-op for a repo with no pickedFrom", () => { + const groups = [makeGroup("org/a", ["squad-a"])]; + const tree = groupByTeamHierarchy(groups, [["squad-"]]); + expect(undoPickedRepoInTree(tree, "org/a")).toBe(tree); + }); + + it("is a no-op when the repo is not found", () => { + const groups = [makeGroup("org/a", ["squad-a"])]; + const tree = groupByTeamHierarchy(groups, [["squad-"]]); + expect(undoPickedRepoInTree(tree, "org/does-not-exist")).toBe(tree); + }); +}); + +// ─── findCombinedSectionPaths ─────────────────────────────────────────────────── + +describe("findCombinedSectionPaths", () => { + it("finds a top-level combined section", () => { + const groups = [makeGroup("org/a", ["squad-a", "squad-b"])]; + const tree = groupByTeamHierarchy(groups, [["squad-"]]); + expect(findCombinedSectionPaths(tree)).toEqual([["squad-a + squad-b"]]); + }); + + it("finds a nested combined section with its full ancestor path", () => { + const groups = [makeGroup("org/a", ["gamme-client", "squad-a", "squad-b"])]; + const tree = groupByTeamHierarchy(groups, [["gamme-", "squad-"]]); + expect(findCombinedSectionPaths(tree)).toEqual([["gamme-client", "squad-a + squad-b"]]); + }); + + it("returns an empty array when there is no combined section", () => { + const groups = [makeGroup("org/a", ["squad-a"])]; + const tree = groupByTeamHierarchy(groups, [["squad-"]]); + expect(findCombinedSectionPaths(tree)).toEqual([]); + }); + + it("finds multiple combined sections across different branches", () => { + const groups = [ + makeGroup("org/a", ["gamme-x", "squad-a", "squad-b"]), + makeGroup("org/b", ["gamme-y", "chapter-a", "chapter-b"]), + ]; + const tree = groupByTeamHierarchy(groups, [["gamme-", "squad-"], ["gamme-"]]); + // Both repos start with a different top-level "gamme-" match, so this + // exercises two independent combined sections at the same nested depth. + const paths = findCombinedSectionPaths(tree); + expect(paths).toContainEqual(["gamme-x", "squad-a + squad-b"]); + }); +}); + // ─── flattenTeamSections ────────────────────────────────────────────────────── describe("flattenTeamSections", () => { diff --git a/src/group.ts b/src/group.ts index 8ff5357..205f722 100644 --- a/src/group.ts +++ b/src/group.ts @@ -439,6 +439,319 @@ function firstDivergingIndex(a: PathEntry[], b: PathEntry[]): number { return i; } +// ─── Hierarchical (path-addressed) pick-team ────────────────────────────────── +// +// A section `label` alone is not unique across a `groupByTeamHierarchy` tree +// (e.g. `"other"` can appear under multiple parents), so hierarchy-aware pick +// operations address a section by its full root-to-node `path` (an array of +// ancestor labels ending with the section's own label). The same path, +// joined with `" > "`, is stored in `pickedFrom` so `undoSectionPickInTree` +// can find every repo picked from that exact section later. For a top-level +// section (`path.length === 1`), this is behaviourally identical to the flat +// `applyTeamPick` / `undoSectionPick` (same joined string as the bare label). + +const PATH_SEPARATOR = " > "; + +/** + * Reconstructs a `groupByTeamHierarchy` tree from a flat `RepoGroup[]` + * produced by `flattenTeamHierarchy`. The inverse of `flattenTeamHierarchy`. + * + * Walks the flat list maintaining a "current node per depth" stack; each + * `sectionPath` entry pushes (or replaces, if shallower) a node onto that + * stack, and repos are appended to whichever node is deepest on the stack + * at the time they're encountered — correctly handling nodes that own repos + * directly *and* have nested children (see `TeamSection`). + */ +export function rebuildTeamHierarchy(groups: RepoGroup[]): TeamSection[] { + const roots: TeamSection[] = []; + let stack: TeamSection[] = []; + + for (const g of groups) { + if (g.sectionPath !== undefined && g.sectionPath.length > 0) { + stack = stack.slice(0, g.sectionPath[0].level); + for (const entry of g.sectionPath) { + const node: TeamSection = { label: entry.label, groups: [], level: entry.level }; + if (stack.length === 0) { + roots.push(node); + } else { + const parent = stack[stack.length - 1]; + parent.children = [...(parent.children ?? []), node]; + } + stack.push(node); + } + } + const { sectionPath: _removed, ...rest } = g; + void _removed; + const repo = rest as RepoGroup; + if (stack.length === 0) { + // No section context at all — shouldn't happen for hierarchy-produced + // input, but keep the repo visible as its own top-level entry rather + // than silently dropping it. + roots.push({ label: repo.repoFullName, groups: [repo] }); + continue; + } + const leaf = stack[stack.length - 1]; + leaf.groups = [...leaf.groups, repo]; + } + + return roots; +} + +/** + * Navigates to the node at `parentPath` (ancestor labels, NOT including the + * target section's own label) and replaces its children — or the top-level + * `sections` array when `parentPath` is empty — with `updater`'s result. + * + * If a segment of `parentPath` is missing (e.g. an ancestor was pruned by + * `removeMatchingRepos` because moving its only repo away left it with no + * groups and no children), it is recreated fresh rather than silently + * no-op-ing — otherwise undoing/re-picking the *last* repo under a branch + * could make that branch permanently unreachable. + */ +function updateSiblingsAtPath( + sections: TeamSection[], + parentPath: string[], + updater: (siblings: TeamSection[]) => TeamSection[], +): TeamSection[] { + if (parentPath.length === 0) return updater(sections); + + const [head, ...rest] = parentPath; + const idx = sections.findIndex((s) => s.label === head); + if (idx === -1) { + const children = updateSiblingsAtPath([], rest, updater); + // Only materialize the missing ancestor if the update actually produced + // something inside it — otherwise this is a genuine no-op (e.g. a + // typo'd path) and adding an empty node here would pollute the tree. + return children.length === 0 ? sections : [...sections, { label: head, groups: [], children }]; + } + + const updatedChildren = updateSiblingsAtPath(sections[idx].children ?? [], rest, updater); + return sections.map((s, i) => (i === idx ? { ...s, children: updatedChildren } : s)); +} + +/** + * Merges `repos` into the sibling named `label` under `parentPath`, creating + * it — inserted before an `"other"` sibling, or appended — if it doesn't + * already exist. Shared by every hierarchical pick/undo/move operation below. + */ +function mergeOrCreateAtPath( + sections: TeamSection[], + parentPath: string[], + label: string, + repos: RepoGroup[], +): TeamSection[] { + return updateSiblingsAtPath(sections, parentPath, (siblings) => { + const idx = siblings.findIndex((s) => s.label === label); + if (idx !== -1) { + return siblings.map((s, i) => (i === idx ? { ...s, groups: [...s.groups, ...repos] } : s)); + } + const newSection: TeamSection = { label, groups: repos }; + const otherIdx = siblings.findIndex((s) => s.label === "other"); + return otherIdx === -1 + ? [...siblings, newSection] + : [...siblings.slice(0, otherIdx), newSection, ...siblings.slice(otherIdx)]; + }); +} + +/** + * Removes every repo matching `predicate` anywhere in the tree, dropping + * sections left with no groups and no children afterward. Returns the + * stripped tree; matched repos (untouched, `pickedFrom` included) are + * appended to `collected` in the order encountered. + */ +function removeMatchingRepos( + nodes: TeamSection[], + predicate: (g: RepoGroup) => boolean, + collected: RepoGroup[], +): TeamSection[] { + return nodes + .map((node) => { + const kept: RepoGroup[] = []; + for (const g of node.groups) { + if (predicate(g)) collected.push(g); + else kept.push(g); + } + const children = node.children + ? removeMatchingRepos(node.children, predicate, collected) + : undefined; + return { ...node, groups: kept, ...(children ? { children } : {}) }; + }) + .filter((node) => node.groups.length > 0 || (node.children?.length ?? 0) > 0); +} + +/** Strips `pickedFrom` from a repo (pure — returns a new object). */ +function stripPickedFrom(g: RepoGroup): RepoGroup { + const { pickedFrom: _p, ...rest } = g; + void _p; + return rest as RepoGroup; +} + +/** + * Tree-aware equivalent of `applyTeamPick`: reassigns the ENTIRE subtree of + * the combined section identified by `combinedPath` (e.g. + * `["gamme-client", "squad-a + squad-b"]`) — its own `groups` *and* any + * nested `children` (e.g. it was already subdivided by a further chain + * level) — to a sibling section named `chosenTeam` at that same depth + * (merged into it if it already exists, otherwise created in its place). + * Every repo in the moved subtree (own groups and every descendant) is + * tagged with `pickedFrom = combinedPath.join(" > ")` so + * `undoSectionPickInTree` can find all of them later. + * + * No-op (returns `sections` unchanged) if any segment of `combinedPath` does + * not resolve to an existing node. Pure — does not mutate `sections`. + */ +export function applyTeamPickInTree( + sections: TeamSection[], + combinedPath: string[], + chosenTeam: string, +): TeamSection[] { + if (combinedPath.length === 0) return sections; + const parentPath = combinedPath.slice(0, -1); + const combinedLabel = combinedPath[combinedPath.length - 1]; + const pathKey = combinedPath.join(PATH_SEPARATOR); + + return updateSiblingsAtPath(sections, parentPath, (siblings) => { + const idx = siblings.findIndex((s) => s.label === combinedLabel); + if (idx === -1) return siblings; + + const picked = tagPickedFrom(siblings[idx], pathKey); + const remaining = siblings.filter((_, i) => i !== idx); + + const targetIdx = remaining.findIndex((s) => s.label === chosenTeam); + if (targetIdx !== -1) { + return remaining.map((s, i) => + i === targetIdx + ? { + ...s, + groups: [...s.groups, ...picked.groups], + ...(picked.children && picked.children.length > 0 + ? { children: [...(s.children ?? []), ...picked.children] } + : {}), + } + : s, + ); + } + + const newSection: TeamSection = { + label: chosenTeam, + groups: picked.groups, + level: siblings[idx].level, + ...(picked.children && picked.children.length > 0 ? { children: picked.children } : {}), + }; + const result = [...remaining]; + result.splice(idx, 0, newSection); + return result; + }); +} + +/** + * Recursively tags every repo in `node` (its own `groups` and every + * descendant's, through `children`) with `pickedFrom`, preserving the + * subtree's shape. Pure — returns a new tree, does not mutate `node`. + */ +function tagPickedFrom(node: TeamSection, pathKey: string): TeamSection { + return { + ...node, + groups: node.groups.map((g) => ({ ...g, pickedFrom: pathKey })), + ...(node.children ? { children: node.children.map((c) => tagPickedFrom(c, pathKey)) } : {}), + }; +} + +/** + * Tree-aware equivalent of `undoSectionPick`: restores every repo anywhere in + * the tree whose `pickedFrom` matches `combinedPathString` (the joined path + * produced by `applyTeamPickInTree`) back to the section at that path, + * recreating it if it no longer exists. `pickedFrom` is stripped from + * restored repos. Sections left empty after the removal are dropped. + * + * No-op (returns `sections` unchanged) if no repo has a matching `pickedFrom`. + * Pure — does not mutate `sections`. + */ +export function undoSectionPickInTree( + sections: TeamSection[], + combinedPathString: string, +): TeamSection[] { + const collected: RepoGroup[] = []; + const stripped = removeMatchingRepos( + sections, + (g) => g.pickedFrom === combinedPathString, + collected, + ); + if (collected.length === 0) return sections; + + const combinedPath = combinedPathString.split(PATH_SEPARATOR); + const parentPath = combinedPath.slice(0, -1); + const combinedLabel = combinedPath[combinedPath.length - 1]; + return mergeOrCreateAtPath(stripped, parentPath, combinedLabel, collected.map(stripPickedFrom)); +} + +/** + * Tree-aware equivalent of `moveRepoToSection`: moves the repo identified by + * `repoFullName` (wherever it currently sits in the tree) to a sibling named + * `targetTeam` under `parentPath` (created in place if absent). The repo's + * `pickedFrom` is preserved so a later undo can still restore it. + * + * No-op (returns `sections` unchanged) if the repo isn't found in the tree. + * Pure — does not mutate `sections`. + */ +export function moveRepoToSectionInTree( + sections: TeamSection[], + repoFullName: string, + parentPath: string[], + targetTeam: string, +): TeamSection[] { + const collected: RepoGroup[] = []; + const stripped = removeMatchingRepos(sections, (g) => g.repoFullName === repoFullName, collected); + if (collected.length === 0) return sections; + return mergeOrCreateAtPath(stripped, parentPath, targetTeam, collected); +} + +/** + * Tree-aware equivalent of `undoPickedRepo`: restores a single previously + * picked repo (identified by `repoFullName`) back to its original combined + * section (read from its own `pickedFrom`), recreating that section if it no + * longer exists. `pickedFrom` is stripped from the restored repo. + * + * No-op (returns `sections` unchanged) if the repo isn't found or has no + * `pickedFrom`. Pure — does not mutate `sections`. + */ +export function undoPickedRepoInTree(sections: TeamSection[], repoFullName: string): TeamSection[] { + const collected: RepoGroup[] = []; + const stripped = removeMatchingRepos( + sections, + (g) => g.repoFullName === repoFullName && g.pickedFrom !== undefined, + collected, + ); + if (collected.length === 0) return sections; + + const combinedPathString = collected[0].pickedFrom!; + const combinedPath = combinedPathString.split(PATH_SEPARATOR); + const parentPath = combinedPath.slice(0, -1); + const combinedLabel = combinedPath[combinedPath.length - 1]; + return mergeOrCreateAtPath(stripped, parentPath, combinedLabel, collected.map(stripPickedFrom)); +} + +/** + * Returns the full path (ancestor labels, root first) of every combined + * (`"a + b"`) section anywhere in the tree — used to resolve an unqualified + * `--pick-team` label (auto-pick when exactly one match) or report the + * available candidates when it's ambiguous or not found. + */ +export function findCombinedSectionPaths(sections: TeamSection[]): string[][] { + const paths: string[][] = []; + + function visit(nodes: TeamSection[], ancestors: string[]): void { + for (const node of nodes) { + const path = [...ancestors, node.label]; + if (node.label.includes(" + ")) paths.push(path); + if (node.children) visit(node.children, path); + } + } + + visit(sections, []); + return paths; +} + // ─── Internal helpers ───────────────────────────────────────────────────────── /** diff --git a/src/render.test.ts b/src/render.test.ts index f193c2f..c184415 100644 --- a/src/render.test.ts +++ b/src/render.test.ts @@ -2109,6 +2109,82 @@ describe("renderGroups — hierarchical section headings (sectionLevel)", () => }); }); +// ─── renderGroups — team pick mode section bar ──────────────────────────────── +// Regression: getSectionPath must be a real local import (not just re-exported) +// for this branch to even run — see the crash reported on issue #181. + +describe("renderGroups — team pick mode section bar", () => { + it("shows the pick bar on a flat (sectionLabel) combined section", () => { + const groups = [ + { ...makeGroup("org/repoA", ["a.ts"], true), sectionLabel: "squad-a + squad-b" }, + ]; + const rows = buildRows(groups); + const out = renderGroups(groups, 0, rows, 40, 0, "q", "org", { + termWidth: 80, + teamPickMode: { + active: true, + sectionLabel: "squad-a + squad-b", + candidates: ["squad-a", "squad-b"], + focusedIndex: 0, + }, + }); + const stripped = out.replace(/\x1b\[[0-9;]*m/g, ""); + expect(stripped).toContain("[ squad-a ]"); + }); + + it("shows the pick bar on the hierarchical section whose full path matches", () => { + const groups = [ + { + ...makeGroup("org/repoA", ["a.ts"], true), + sectionPath: [ + { label: "gamme-client", level: 0 }, + { label: "squad-a + squad-b", level: 1 }, + ], + }, + ]; + const rows = buildRows(groups); + const out = renderGroups(groups, 0, rows, 40, 0, "q", "org", { + termWidth: 80, + teamPickMode: { + active: true, + sectionLabel: "squad-a + squad-b", + sectionPath: ["gamme-client", "squad-a + squad-b"], + candidates: ["squad-a", "squad-b"], + focusedIndex: 0, + }, + }); + const stripped = out.replace(/\x1b\[[0-9;]*m/g, ""); + expect(stripped).toContain("[ squad-a ]"); + }); + + it("does not show the pick bar on a same-label section at a different path", () => { + const groups = [ + { + ...makeGroup("org/repoA", ["a.ts"], true), + sectionPath: [ + { label: "gamme-other", level: 0 }, + { label: "squad-a + squad-b", level: 1 }, + ], + }, + ]; + const rows = buildRows(groups); + const out = renderGroups(groups, 0, rows, 40, 0, "q", "org", { + termWidth: 80, + teamPickMode: { + active: true, + sectionLabel: "squad-a + squad-b", + // Targets a DIFFERENT parent than the one actually rendered above. + sectionPath: ["gamme-client", "squad-a + squad-b"], + candidates: ["squad-a", "squad-b"], + focusedIndex: 0, + }, + }); + const stripped = out.replace(/\x1b\[[0-9;]*m/g, ""); + expect(stripped).not.toContain("[ squad-a ]"); + expect(stripped).toContain("squad-a + squad-b"); + }); +}); + // ─── renderGroups — re-pick mode hints bar ──────────────────────────────────── describe("renderGroups — re-pick mode hints bar", () => { diff --git a/src/render.ts b/src/render.ts index 21122f9..5719edc 100644 --- a/src/render.ts +++ b/src/render.ts @@ -2,7 +2,7 @@ import * as style from "./style.ts"; import type { FilterTarget, RepoGroup, Row, TextMatchSegment } from "./types.ts"; import { highlightFragment } from "./render/highlight.ts"; import { buildFilterStats, type FilterStats } from "./render/filter.ts"; -import { rowTerminalLines } from "./render/rows.ts"; +import { getSectionPath, rowTerminalLines } from "./render/rows.ts"; import { buildMatchCountLabel, buildSummaryFull } from "./render/summary.ts"; import { renderTeamPickHeader } from "./render/team-pick.ts"; import { visibleWidth, stripAnsi, clipToWidth } from "./render/terminal.ts"; @@ -23,6 +23,7 @@ export { buildRows, isCursorVisible, normalizeScrollOffset, + getSectionPath, } from "./render/rows.ts"; export { buildMatchCountLabel, @@ -274,6 +275,11 @@ interface RenderOptions { teamPickMode?: { active: boolean; sectionLabel: string; + /** Full ancestor path (root first, ending with `sectionLabel`) used to + * unambiguously match the target row in a `groupByTeamHierarchy` tree, + * where the same label can appear under multiple parents. Falls back to + * matching on `sectionLabel` alone when absent (flat sections). */ + sectionPath?: string[]; candidates: string[]; focusedIndex: number; }; @@ -532,8 +538,16 @@ export function renderGroups( // Emit the blank separator only when there are rows above in the viewport. if (usedLines > 0) lines.push(""); // Feat: team pick mode — show pick bar when active for this section — see issue #85. + // Fix: match by full ancestor path (not just label) when available, so + // the same label at a different tree branch isn't targeted by mistake + // in a groupByTeamHierarchy tree — see issue #181. const pickMode = opts.teamPickMode; - if (pickMode?.active && pickMode.sectionLabel === row.sectionLabel) { + if ( + pickMode?.active && + (pickMode.sectionPath !== undefined + ? pickMode.sectionPath.join(" > ") === getSectionPath(rows, i).join(" > ") + : pickMode.sectionLabel === row.sectionLabel) + ) { // Fix: clip pick bar to (termWidth - 3) so "── " + bar never wraps — see issue #121. const bar = renderTeamPickHeader( pickMode.candidates, diff --git a/src/render/rows.ts b/src/render/rows.ts index e540033..59d0add 100644 --- a/src/render/rows.ts +++ b/src/render/rows.ts @@ -134,6 +134,37 @@ function firstDivergingPathIndex( return i; } +/** + * Reconstructs the full ancestor path (root-to-node labels) for the section + * row at `rowIndex`, by scanning backward through `rows` for the most recent + * section row at each decreasing `sectionLevel`. Used to identify a + * combined-label section unambiguously in a `groupByTeamHierarchy` tree, + * where the same label (e.g. `"other"`) can appear under multiple parents. + * + * Returns `[]` if the row at `rowIndex` is not a section row. For a flat + * `groupByTeamPrefix` section (`sectionLevel` 0 or unset), the path is just + * the row's own label. + */ +export function getSectionPath(rows: Row[], rowIndex: number): string[] { + const row = rows[rowIndex]; + if (row?.type !== "section" || row.sectionLabel === undefined) return []; + + const path: string[] = [row.sectionLabel]; + let neededLevel = (row.sectionLevel ?? 0) - 1; + for (let i = rowIndex - 1; i >= 0 && neededLevel >= 0; i--) { + const r = rows[i]; + if ( + r.type === "section" && + r.sectionLabel !== undefined && + (r.sectionLevel ?? 0) === neededLevel + ) { + path.unshift(r.sectionLabel); + neededLevel--; + } + } + return path; +} + /** * Normalises scrollOffset downward so the viewport is always packed from the * bottom. After a fold, a filter change, or navigating near the end of the diff --git a/src/tui.ts b/src/tui.ts index 8c691f1..3dfc5f5 100644 --- a/src/tui.ts +++ b/src/tui.ts @@ -6,6 +6,7 @@ import { buildFileUrl, buildFilterStats, buildRows, + getSectionPath, isCursorVisible, normalizeScrollOffset, renderGroups, @@ -14,10 +15,15 @@ import { import { buildOutput } from "./output.ts"; import { applyTeamPick, - moveRepoToSection, - undoSectionPick, + applyTeamPickInTree, + flattenTeamHierarchy, flattenTeamSections, + moveRepoToSection, + moveRepoToSectionInTree, + rebuildTeamHierarchy, rebuildTeamSections, + undoSectionPick, + undoSectionPickInTree, } from "./group.ts"; import { parseMouseEvent } from "./render/mouse.ts"; import { @@ -218,14 +224,20 @@ export async function runInteractive( // ─── Team pick mode state ─────────────────────────────────────────────────────── // Feat: team pick mode — resolve multi-team section ownership — see issue #85 + // Fix: track the full ancestor sectionPath (not just the bare label) so a + // pick on a groupByTeamHierarchy tree unambiguously targets the row the + // cursor was actually on — see issue #181. let teamPickMode = { active: false, sectionLabel: "", + sectionPath: [] as string[], candidates: [] as string[], focusedIndex: 0, }; - /** Maps combined section label → chosen team; pre-seeded with CLI --pick-team flags - * so they are included in the replay command even if no additional interactive picks are made. */ + /** Maps combined section path (joined with " > ", or the bare label for a + * flat top-level section) → chosen team; pre-seeded with CLI --pick-team + * flags so they are included in the replay command even if no additional + * interactive picks are made. */ const confirmedPicks: Record = { ...initialPickTeams }; // ─── Team re-pick mode state ────────────────────────────────────────────────── @@ -460,18 +472,42 @@ export async function runInteractive( } else if (key === KEY_ENTER_CR || key === KEY_ENTER_LF) { // Enter — confirm pick, reassign repos, exit pick mode const chosen = teamPickMode.candidates[teamPickMode.focusedIndex]; - const sections = rebuildTeamSections(groups); - const updated = applyTeamPick(sections, teamPickMode.sectionLabel, chosen); - groups = flattenTeamSections(updated); - confirmedPicks[teamPickMode.sectionLabel] = chosen; - teamPickMode = { active: false, sectionLabel: "", candidates: [], focusedIndex: 0 }; + // Fix: branch on sectionPath depth so a pick on a nested + // groupByTeamHierarchy section reassigns within the tree instead of + // (incorrectly) treating the tree as a flat groupByTeamPrefix list — + // see issue #181. A depth-1 path (top-level section) behaves exactly + // like the flat path, since a hierarchy tree's top level is the same + // shape as groupByTeamPrefix's flat sections. + if (teamPickMode.sectionPath.length > 1) { + const sections = rebuildTeamHierarchy(groups); + const updated = applyTeamPickInTree(sections, teamPickMode.sectionPath, chosen); + groups = flattenTeamHierarchy(updated); + } else { + const sections = rebuildTeamSections(groups); + const updated = applyTeamPick(sections, teamPickMode.sectionLabel, chosen); + groups = flattenTeamSections(updated); + } + confirmedPicks[teamPickMode.sectionPath.join(" > ") || teamPickMode.sectionLabel] = chosen; + teamPickMode = { + active: false, + sectionLabel: "", + sectionPath: [], + candidates: [], + focusedIndex: 0, + }; // Clamp cursor after row count may have changed const newRows = buildRows(groups, filterPath, filterTarget, filterRegex); cursor = Math.min(cursor, Math.max(0, newRows.length - 1)); scrollOffset = Math.min(scrollOffset, cursor); } else if (key === "\x1b" && !key.startsWith("\x1b[") && !key.startsWith("\x1b\x1b")) { // Esc — cancel with no change - teamPickMode = { active: false, sectionLabel: "", candidates: [], focusedIndex: 0 }; + teamPickMode = { + active: false, + sectionLabel: "", + sectionPath: [], + candidates: [], + focusedIndex: 0, + }; } redraw(); continue; @@ -500,7 +536,18 @@ export async function runInteractive( // Enter — confirm re-pick, move repo to the focused candidate team const targetTeam = repickMode.candidates[repickMode.focusedIndex]; const g = groups[repickMode.repoIndex]; - groups = moveRepoToSection(groups, g.repoFullName, targetTeam); + const pickedFrom = g.pickedFrom ?? ""; + // Fix: a hierarchical pickedFrom ("parent > combined") must move the + // repo within the tree, at the same parent depth it was picked from + // — see issue #181. + if (pickedFrom.includes(" > ")) { + const parentPath = pickedFrom.split(" > ").slice(0, -1); + const sections = rebuildTeamHierarchy(groups); + const updated = moveRepoToSectionInTree(sections, g.repoFullName, parentPath, targetTeam); + groups = flattenTeamHierarchy(updated); + } else { + groups = moveRepoToSection(groups, g.repoFullName, targetTeam); + } const newRows = buildRows(groups, filterPath, filterTarget, filterRegex); cursor = Math.min(cursor, Math.max(0, newRows.length - 1)); scrollOffset = Math.min(scrollOffset, cursor); @@ -513,7 +560,13 @@ export async function runInteractive( const combinedLabel = groups[repickMode.repoIndex]?.pickedFrom; if (combinedLabel) { delete confirmedPicks[combinedLabel]; - groups = undoSectionPick(groups, combinedLabel); + if (combinedLabel.includes(" > ")) { + const sections = rebuildTeamHierarchy(groups); + const updated = undoSectionPickInTree(sections, combinedLabel); + groups = flattenTeamHierarchy(updated); + } else { + groups = undoSectionPick(groups, combinedLabel); + } } const newRows = buildRows(groups, filterPath, filterTarget, filterRegex); cursor = Math.min(cursor, Math.max(0, newRows.length - 1)); @@ -706,7 +759,17 @@ export async function runInteractive( // Feat: team pick mode — resolve multi-team section ownership — see issue #85 if (key === "p" && row?.type === "section" && row.sectionLabel?.includes(" + ")) { const candidates = row.sectionLabel.split(" + "); - teamPickMode = { active: true, sectionLabel: row.sectionLabel, candidates, focusedIndex: 0 }; + // Fix: capture the full ancestor path so the pick targets the exact + // tree node under the cursor, not just any row sharing the same + // label — see issue #181. + const sectionPath = getSectionPath(rows, cursor); + teamPickMode = { + active: true, + sectionLabel: row.sectionLabel, + sectionPath, + candidates, + focusedIndex: 0, + }; redraw(); continue; } @@ -715,12 +778,13 @@ export async function runInteractive( // different team. Otherwise cycle the filter target: path → content → repo → path. // Feat: re-pick mode — see issue #87 if (key === "t") { - const isPickedRepo = - groupByTeamPrefix && row?.type === "repo" && !!groups[row.repoIndex]?.pickedFrom; + const isPickedRepo = row?.type === "repo" && !!groups[row.repoIndex]?.pickedFrom; if (isPickedRepo) { // Enter re-pick mode — candidates come from the original combined label + // (its last path segment for a hierarchical pick — see issue #181). const pickedFrom = groups[row!.repoIndex].pickedFrom!; - const candidates = pickedFrom.split(" + ").map((c) => c.trim()); + const combinedLabel = pickedFrom.split(" > ").at(-1)!; + const candidates = combinedLabel.split(" + ").map((c) => c.trim()); repickMode = { active: true, repoIndex: row!.repoIndex, candidates, focusedIndex: 0 }; } else { // Cycle filter target when not on a picked repo