Wire hierarchical team grouping into the CLI end-to-end - #190
Open
shouze wants to merge 2 commits into
Open
Conversation
|
Coverage after merging feat/team-hierarchy-cli into feat/team-hierarchy-pick-team will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
There was a problem hiding this comment.
Pull request overview
Wires hierarchical team grouping, consolidation, and path-based team picking through the CLI and TUI.
Changes:
- Adds prefix-chain parsing and pick resolution.
- Integrates hierarchy grouping and consolidation end to end.
- Updates TUI handling, replay support, tests, and help formatting.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Summary | Findings |
|---|---|---|
src/tui.ts |
Tree-aware pick, re-pick, and undo handling | Critical: Picking a combined parent with children can drop those children. |
src/group.ts |
Prefix parsing and pick resolution | Moderate: Explicit paths are not validated and can record no-op assignments. |
src/group.test.ts |
Parser and resolver tests | No final comments. |
github-code-search.ts |
CLI grouping, consolidation, and pick integration | Critical: Consolidation before pick resolution can invalidate labels and paths. |
Suppressed comments (3)
github-code-search.ts:406
- A combined node can legitimately have children when a chain has another level (for example, a
gamme-a + gamme-broot subdivided bysquad-).applyTeamPickInTreemoves only the node's directgroups; if they are empty, this call removes the node and its descendants, dropping those repositories from the output. Picking a valid parent section must move or preserve the entire subtree (or explicitly reject non-leaf combined sections) before applying this for all hierarchy paths.
sections = applyTeamPickInTree(sections, resolution.path, resolution.chosen);
github-code-search.ts:396
- This mutates the tree used for every output format, so
--format json --group-by-team-prefix-consolidateemits synthetic(including …)labels and loses the original hierarchy in each result'ssectionpath. Consolidation is a rendering mode and the JSON contract needs the underlying full tree; retain the uncollapsed tree for JSON and derive a consolidated copy only for the relevant presentation.
if (opts.groupByTeamPrefixConsolidate) {
sections = consolidateTeamHierarchy(sections);
github-code-search.ts:200
src/completions.tskeeps a staticOPTIONSlist used to generate all installed shell completions, but this new flag is not added there. Consequently bash/zsh/fish users cannot complete--group-by-team-prefix-consolidateeven though Commander accepts it; add the flag to the shared completion metadata and its test.
"--group-by-team-prefix-consolidate",
[
"Collapse unambiguous single-branch nesting chains into one heading",
'with an "(including ...)" suffix instead of one heading per level.',
"Only applies with --group-by-team-prefix.",
"Docs: https://fulll.github.io/github-code-search/usage/team-grouping",
].join("\n"),
false,
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+395
to
+396
| if (opts.groupByTeamPrefixConsolidate) { | ||
| sections = consolidateTeamHierarchy(sections); |
Comment on lines
+809
to
+810
| if (combinedInput.includes(PATH_SEPARATOR)) { | ||
| path = combinedInput.split(PATH_SEPARATOR).map((s) => s.trim()); |
| groups = flattenTeamSections(updated); | ||
| } | ||
| const sections = rebuildTeamHierarchy(groups); | ||
| const updated = applyTeamPickInTree(sections, teamPickMode.sectionPath, chosen); |
shouze
force-pushed
the
feat/team-hierarchy-cli
branch
from
August 23, 2026 21:42
dbc3e06 to
12a397d
Compare
|
Coverage after merging feat/team-hierarchy-cli into feat/team-hierarchy-pick-team will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
1 similar comment
|
Coverage after merging feat/team-hierarchy-cli into feat/team-hierarchy-pick-team will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
shouze
added a commit
that referenced
this pull request
Aug 23, 2026
…tions Addresses Copilot review on PR #190: - Reordered CLI wiring so --pick-team assignments resolve against the raw, uncollapsed tree BEFORE --group-by-team-prefix-consolidate runs. Consolidating first changes (or erases) a combined section's label/ path, so pick-team could silently fail to find it or misparse a synthetic "(including ...)" label as candidate teams. - Consolidation is now skipped entirely for --format json (with a stderr warning when requested): JSON is a data contract and must reflect the real, uncollapsed hierarchy in each result's `section` path, not a display-only collapsed view. The replay command and TUI now receive the same consolidateApplied flag actually used, instead of the raw --group-by-team-prefix-consolidate request. - resolvePickTeamAssignment now validates an explicit "parent > combined" path actually resolves to a node in the tree before accepting it — previously a typo'd parent (or a path made stale by an earlier pick) was accepted, applyTeamPickInTree silently no-op'd, and the caller still recorded the assignment for replay as if it had succeeded. - Added the missing --group-by-team-prefix-consolidate entry to the shared shell-completion metadata (bash/zsh/fish) and its tests. The applyTeamPickInTree/tui.ts "children dropped on pick" findings in this review were already fixed in the previous commit on this stack (feat/team-hierarchy-pick-team) — verified still present here.
|
Coverage after merging feat/team-hierarchy-cli into feat/team-hierarchy-pick-team will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
4 tasks
- github-code-search.ts: --group-by-team-prefix now parses a chain grammar (/ for nesting depth, , for independent chains) via the new parseTeamPrefixChains, and always uses groupByTeamHierarchy (a 1-level chain behaves identically to the old flat groupByTeamPrefix, covered by parity tests). Added --group-by-team-prefix-consolidate to apply consolidateTeamHierarchy. --pick-team now resolves through the new resolvePickTeamAssignment (bare-label auto-resolve when unambiguous, or an explicit "parent > combined" path), replacing ~90 lines of inline validation with pure, tested logic. - src/group.ts: added parseTeamPrefixChains and resolvePickTeamAssignment (pure, fully unit-tested CLI-parsing helpers). - src/tui.ts: simplified pick/re-pick/undo handlers to always use the tree-aware functions (rebuildTeamHierarchy/applyTeamPickInTree/etc.), since the CLI now always produces sectionPath-tagged groups — the previous sectionPath.length/pickedFrom-based branching to the old flat functions was dead code that would have silently wiped all groups for a depth-1 (top-level) section pick (rebuildTeamSections finds no sectionLabel-tagged group, applyTeamPick no-ops on an empty array). Caught via a manual end-to-end smoke test before it could ship. Fixed along the way: a pre-existing Commander help-formatting bug where any option description containing a newline followed by whitespace (e.g. an indented example line) made Help.preformatted() treat the WHOLE description as manually formatted and skip aligning continuation lines to the option column — affected --exclude-repositories and --exclude-extracts too, not just the new options. Also removed a duplicated "(default: false)" on --include-archived/ --exclude-template-repositories (Commander already appends it). Closes #182
…tions Addresses Copilot review on PR #190: - Reordered CLI wiring so --pick-team assignments resolve against the raw, uncollapsed tree BEFORE --group-by-team-prefix-consolidate runs. Consolidating first changes (or erases) a combined section's label/ path, so pick-team could silently fail to find it or misparse a synthetic "(including ...)" label as candidate teams. - Consolidation is now skipped entirely for --format json (with a stderr warning when requested): JSON is a data contract and must reflect the real, uncollapsed hierarchy in each result's `section` path, not a display-only collapsed view. The replay command and TUI now receive the same consolidateApplied flag actually used, instead of the raw --group-by-team-prefix-consolidate request. - resolvePickTeamAssignment now validates an explicit "parent > combined" path actually resolves to a node in the tree before accepting it — previously a typo'd parent (or a path made stale by an earlier pick) was accepted, applyTeamPickInTree silently no-op'd, and the caller still recorded the assignment for replay as if it had succeeded. - Added the missing --group-by-team-prefix-consolidate entry to the shared shell-completion metadata (bash/zsh/fish) and its tests. The applyTeamPickInTree/tui.ts "children dropped on pick" findings in this review were already fixed in the previous commit on this stack (feat/team-hierarchy-pick-team) — verified still present here.
shouze
force-pushed
the
feat/team-hierarchy-cli
branch
from
August 24, 2026 00:14
45cd3c2 to
69c3bbb
Compare
|
Coverage after merging feat/team-hierarchy-cli into feat/team-hierarchy-pick-team will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
1 similar comment
|
Coverage after merging feat/team-hierarchy-cli into feat/team-hierarchy-pick-team will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Wires the hierarchical team-prefix grouping built in #177-#181 into the
production CLI end-to-end, replacing the flat
groupByTeamPrefixpipeline.--group-by-team-prefixnow accepts a chain grammar:/nests levelswithin one chain (
gamme-/squad-),,separates independent chains(
gamme-/squad-,chapter-). Parsed by the new, pure, fully-testedparseTeamPrefixChains(malformed segments are dropped with a warning,not silently propagated as empty prefixes).
groupByTeamHierarchy, evenfor a single flat prefix — a 1-level chain is behaviorally identical to
the old
groupByTeamPrefix(parity already covered by [1/7] Data model & core hierarchical grouping algorithm #177's tests), sothis isn't a second code path to maintain.
--group-by-team-prefix-consolidateto applyconsolidateTeamHierarchy(mechanism 3 from EPIC: Hierarchical team-prefix grouping (multi-level headings) #125/[2/7] Advanced consolidated rendering #178).--pick-teamnow resolves through the newresolvePickTeamAssignment:a bare label auto-resolves when it's unambiguous anywhere in the tree, or
an explicit
"gamme-client > squad-a + squad-b"=squad-apath when nestedor ambiguous. This single pure function replaces ~90 lines of inline
validation in
github-code-search.ts.consolidateTeamSectionsand path-qualifiedpickTeamsend to end.Bug caught before it shipped
While wiring this up,
src/tui.ts's pick/re-pick/undo handlers stillbranched on
sectionPath.length > 1/pickedFrom.includes(" > ")todecide between the new tree-aware functions and the old flat
rebuildTeamSections/applyTeamPick/flattenTeamSections. Since the CLInow always produces
sectionPath-tagged groups (never the old flatsectionLabelmarker), a pick on a depth-1 (top-level) section — thecommon case — would take the "flat" branch, where
rebuildTeamSectionsfinds no
sectionLabel-tagged group at all,applyTeamPickno-ops on anempty array, and the pick would silently wipe every group from the
screen. Caught with a manual end-to-end smoke test (mock data through the
full parse → group → pick → consolidate → flatten → output pipeline) before
it could reach a real user. Fixed by simplifying the TUI handlers to always
use the tree-aware functions — the old flat functions remain in
group.ts,fully tested, for any other consumer.
Also fixed (found while polishing
--help)A pre-existing Commander help-formatting bug: any option description
containing a newline immediately followed by whitespace (e.g. an indented
example sub-line) makes
Help.preformatted()treat the wholedescription as already manually formatted and skip aligning continuation
lines to the option column. This affected
--exclude-repositoriesand--exclude-extractstoo, not just the new options here. Fixed by removingthe leading indentation from example lines (still visually distinguished
via the existing
colorDescdimming) and removed a duplicated"(default: false)"on--include-archived/--exclude-template-repositories(Commander already appends it from theoption's default-value argument).
Closes #182
How to test
New
group.test.tstests coverparseTeamPrefixChains(flat, nested,multi-chain, whitespace, malformed input) and
resolvePickTeamAssignment(unambiguous bare label, nested bare label, explicit qualified path, all
error paths). Manually smoke-tested the full CLI pipeline with mock data
(chain parsing → hierarchy grouping → nested pick-team → consolidation →
markdown output → replay command round-trip).
Validation
bun test(961 pass)bun run lintbun run format:checkbun run knipbun run build.tsbunx tsc --noEmitsanity check (zero new errors)--helpoutput check for formatting