Layer 5.3: Palette, completion, and discovery filtering by aboimpinto · Pull Request #5255 · Hmbown/CodeWhale

aboimpinto · 2026-08-07T00:56:07Z

Summary

Verifies and consolidates the user-command integration in the command palette and slash-completion surfaces (Layer 5.3 of the command-boundary refactor, following Layer 5.2 in #4992).

The integration already shipped with Layer 5.1 upstream; this PR proves each acceptance criterion with tests (Gherkin + Rust unit), and fixes a real shadowing inconsistency found during the audit.

Audit finding — shadowing inconsistency

Check	Slash completion	Command palette (before)
Canonical shadow	checks user name AND aliases	checks canonical name only

A user command whose alias shadows a built-in canonical name was suppressed in completion but not in the palette. The refactor unifies both surfaces on the same shared predicate.

Changes

File	Change
`crates/tui/src/commands/discovery.rs`	+235 — shared discovery-shadowing contract (`user_command_shadows_builtin_canonical`, `user_command_shadows_builtin_alias`, unshadowed-alias computation)
`crates/tui/src/commands/epic_discovery_acceptance.rs`	+750 — Gherkin acceptance harness (cucumber worlds, `fail_on_skipped`)
`crates/tui/tests/features/feat-012-discovery-filtering.feature`	+43 — Gherkin scenarios for all six acceptance criteria
`crates/tui/src/tui/command_palette.rs`	+174/- — unified onto shared contract; canonical-shadow suppression incl. aliases
`crates/tui/src/tui/widgets/mod.rs`	+60/- — slash completion unified onto shared contract
`crates/tui/src/commands/mod.rs`	+5 — test module wiring
`docs/architecture/command-dispatch.md`	+1 — discovery contract documented
`structcopy.rs`, `latex_render.rs`, `subagent/tests.rs`	minor clippy/fixture repairs

Upstream fixes carried in this PR (origin/main was red on CI)

While rebasing onto latest main we hit gates that upstream main itself failed at the
time (last ci.yml run on main was red). All were re-baselined here with documented commits so
this PR is mergeable and CI is green:

Windows-lane truncation footer (8f2b622dc, 093186a1a) — adaptive_evidence_acceptance.rs:62
failed on Windows because the model-facing artifact footer used raw path.display() while
the test asserts /artifacts/. Introduced by fix(tui): honest large-output truncation + recovery path #5212 (f0a6898c3). Fixed by normalizing
the footer via format_artifact_relative_path; two unit tests (one from upstream fa7c4b055)
updated to assert the normalized form. Upstream later made the integration test
path-tolerant (21ed173cf) — both changes coexist.
Dead-code budget (f0e4d8a26, merge) — upstream WIP commits added test-only
#[allow(dead_code)] attributes without bumping the budget; upstream re-baselined to 452 in
21ed173cf, the merge resolved to the same number.
Runtime contract (43a55ce87, merge) — upstream ec5747f7d renamed work_update →
todo_write and prompt tightening grew identities; all drifted metrics synced to the
measured receipt, re-synced after 21ed173cf (schema growth).
Source-structure budget (24e4ac9b9, 0e6185574, merge) — re-baselined for FEAT-012's
+1,179 lines and again for upstream growth after each merge (currently 677636).
Web public-surface contract (5d93dd00e) — upstream renamed the tool and updated
docs/facts but missed web/lib/public-surface-contract.test.ts and the tools docs page;
synced to todo_write (250/250 web tests).

FEAT-012's own changes (discovery contract, Gherkin harness, palette/completion unification)
were untouched by all of this. If maintainers prefer landing the budget/contract re-baselines
upstream first, the corresponding commits can be dropped before merge.

Testing

cargo test -p codewhale-tui --tests -- feat012     # 7/7 Gherkin acceptance
cargo test -p codewhale-tui --tests -- discovery   # 64/64 shared contract
cargo test -p codewhale-tui --tests -- command_palette  # 34/34

Closes #2791
Refs #2870

Paulo Aboim Pinto

…t gate - structcopy.rs: simplify nonminimal boolean in next_absolute_path_start (no behavior change) - latex_render.rs: remove always-true '|| true' last-row guard in parse_rows, which emitted a spurious empty row after a trailing row separator; add regression test Boy Scout repairs of pre-existing issues on origin/main so the configured strict lint gate is green.

- New crates/tui/src/commands/discovery.rs: user_command_shadows_builtin_canonical, user_command_shadows_builtin_alias, unshadowed_builtin_aliases (order-preserving) - Semantics ported from slash-completion predicates (widgets/mod.rs) which are the explicit alias-aware reference; palette consumes them in Phase 3 - 13 unit tests: canonical claims, accepted-alias claims, hidden ownership, rejected-alias omission, alias projection order, registry-lookup agreement guard - Registered pub mod discovery in commands/mod.rs - Temporary #[allow(dead_code)] on the three predicates until Phase 3/4 consumers land (recorded in planning-analysis-report.md); MUST be removed there Generated with Claude Code

- command_palette.rs: canonical-shadow check now uses commands::discovery::user_command_shadows_builtin_canonical over a collected metadata slice instead of user_registry.get(command.name) - palette_description_for_unshadowed_aliases now consumes discovery::unshadowed_builtin_aliases (order-preserving projection) - Removed temporary #[allow(dead_code)] markers from discovery.rs (all three predicates now have consumers); module doc note removed - New palette tests: visible canonical shadow (exactly one user-owned /help row with user metadata/action), accepted-alias suppression of built-in canonical row, hidden canonical shadow (no discovery row), alias-only shadow preserving canonical row without the claimed alias - Palette suite: 34 passed (30 existing + 4 new); shared 13/13; completion 23/23 guard green; strict clippy 0 warnings Generated with Claude Code

- widgets/mod.rs: builtin_visible_for_completion_match and push_command_entry now consume commands::discovery predicates; local duplicate user_command_shadows_builtin_canonical/_alias definitions deleted - New completion test: slash_completion_accepted_user_alias_claims_builtin_canonical_token (user alias claiming a built-in canonical token suppresses the built-in suggestion and surfaces the user command) - Completion suite: 24 passed (23 existing + 1 new); discovery 13/13 and palette 34/34 guards green; strict clippy 0 warnings; no ranking/dedup/ file-move changes; slash_completion_hints stays in widgets/mod.rs Generated with Claude Code

- New tests/features/feat-012-discovery-filtering.feature: 7 scenarios (AC1-AC6 + AT-010 alias-aware unification) covering all six FEAT-012 acceptance criteria and EPIC AT-008/009/010 - New commands/epic_discovery_acceptance.rs: scenario-level cucumber worlds bound to live palette builder, live slash completion, and live dispatch; fail_on_skipped + non-zero passed-step assertions per scenario - Registered module in commands/mod.rs - docs/architecture/command-dispatch.md: module map row for shared discovery.rs owner - feat012 selector: 7/7 scenarios pass, zero skipped, non-zero steps each - Guards: discovery 13/13, palette 34/34, completion 24/24; strict clippy 0 warnings Generated with Claude Code

- git_repo_root_reports_attempted_paths_when_no_repo_found created its harness in the checkout's parent dir; when the checkout is nested inside another git repo (e.g. a workspace repo with sibling checkouts), the harness itself resolved to that parent repo and the no-repository path was never exercised - Use the system temp dir with deep nesting beyond the parent-search limit, mirroring the sibling create_isolated_worktree no-repo test Boy Scout repair found by the FEAT-012 Phase 8 full workspace gate; test-only, no behavior change to git_repo_root itself.

…eptance harness FEAT-012 adds the shared discovery-shadowing contract (discovery.rs, 235 lines) and the Gherkin acceptance harness (epic_discovery_acceptance.rs, 750 lines) mirroring the accepted FEAT-011 pattern. Aggregate owned Rust source 673375 -> 674554 (+1179 lines). No new 1000-line modules. Pay down in v0.9.5 per the existing budget TODO notes.

aboimpinto · 2026-08-07T01:42:40Z

CI status: pre-existing Windows failure, not a regression

All checks pass except Test (windows-latest), which fails on:

headless_bash_success_and_failure_are_distinct_bounded_exact_evidence
panicked at crates\tui\tests\adaptive_evidence_acceptance.rs:62

This failure is pre-existing on upstream main — verified against run 31121815609 (branch main, 2026-08-06): the exact same test fails with the same panic on the Windows lane without any of this PR's changes. Our PR touches commands/discovery.rs, command_palette.rs, widgets/mod.rs, and test fixtures — not the Bash truncation/artifact footer path asserted at adaptive_evidence_acceptance.rs:62.

Green lanes:

✅ Lint (incl. source-structure budget — re-baselined for the Layer 5.3 acceptance harness)
✅ Test (ubuntu-latest), Test (macos-latest)
✅ link, gate, version drift, DCO, GitGuardian, cargo check (aarch64)

Paulo Aboim Pinto

Fixes an upstream Windows-lane test failure introduced by Hmbown#5212 (commit f0a6898, "fix(tui): honest large-output truncation + recovery path"). Hmbown#5212 flipped the adaptive-evidence contract so the model-facing truncation footer MUST name the on-disk artifact path, but the footer kept building that path with absolute_path.display().to_string(), which on Windows emits backslashes (\artifacts\) while the acceptance test asserts the POSIX form (/artifacts/). - truncate.rs: the truncated_preview recovery_path (both the adaptive evidence path and the legacy spillover fallback) now goes through crate::artifacts::format_artifact_relative_path, which normalizes separators to '/' — the same normalization the artifact_relative_path metadata field already used. - Consumers that read the footer path back (tool_routing, retrieval, UI preview) construct PathBuf from the string, and PathBuf::from accepts forward slashes on Windows, so no behavior change beyond the model-facing text being platform-independent. The failing test (headless_bash_success_and_failure_are_distinct_ bounded_exact_evidence) fails identically on upstream main without this PR's changes; this commit lands the repair inside the Layer 5.3 PR to unblock the Windows lane. Paulo Aboim Pinto

aboimpinto · 2026-08-07T07:09:52Z

Windows-lane repair: upstream failure fixed inside this PR

Commit 8f2b622dc — fix(tui): normalize artifact footer path separators for Windows

Root cause: PR #5212 (f0a6898) flipped the adaptive-evidence contract so the model-facing truncation footer must name the on-disk artifact path (/artifacts/), but the footer kept building the path with absolute_path.display().to_string(), which emits backslashes on Windows — so the acceptance test headless_bash_success_and_failure_are_distinct_bounded_exact_evidence fails on the Windows lane only. The same failure reproduces on upstream main (run 31121815609).

Fix: the truncated_preview recovery path now goes through crate::artifacts::format_artifact_relative_path — the same normalization the artifact_relative_path metadata already used. PathBuf::from accepts forward slashes on Windows, so all consumers keep working; only the model-facing text becomes platform-independent.

Scope: 2 lines changed, 1 file (truncate.rs). No test-file changes, no behavior change on POSIX.

Paulo Aboim Pinto

…-3-palette-completion-and-discovery-filte # Conflicts: # scripts/source-structure-budget.json

Upstream commit b6585ea (WIP: 0.9.4 fence, turn liveness, model picker, budget, and contributor credit, merged 2026-08-06) added three #[allow(dead_code)] attributes without bumping the dead-code budget, leaving both origin/main and this branch 3 over the 451 ceiling: - stuck_guard.rs: StepFingerprint::waiting_for_subagents (test-only ctor) - turn_loop.rs: should_hold_turn_for_subagents (test-only, Hmbown#3216) - behavioral_tips.rs: enum BehavioralTip (3 of 6 variants unconstructed) None are removable without deleting test-only constructors or enum variants, so re-baseline 451 -> 454 per the gate's own guidance. Our FEAT-012 code contributes 0 net allows (Phase 2 added 3, Phase 3 removed them). Pay down in the Hmbown#4785 sweep.

…ename Upstream ec5747f ("fix: todo_write sole progress surface + §3d/4a test alignment", 0.9.4) renamed work_update -> todo_write in the tool catalog, and the 0.9.4 WIP prompt tightening (b6585ea) grew the system prompt and representative-stage identities. Neither re-baselined scripts/runtime-contract-budget.json, leaving origin/main itself red on this gate. Sync all 49 drifted contract metrics to the measured receipt: - tool_catalog tool_names/identity digests/bytes/tokens for plan, act, operate x active/full (work_update -> todo_write; +125 bytes per surface) - system_prompt bytes/tokens for all modes (prompt tightening) - representative_context stage identities and byte counts The contract budget is a snapshot of the code; this locks the new identity per the gate's own "explicit maintainer decision" rule. No FEAT-012 changes contribute to the drift.

8f2b622 normalized the model-facing artifact footer to forward slashes (platform-independent). Two unit tests still asserted the raw OS path against that footer, failing on the Windows lane: - truncate::adaptive_evidence_footer_names_artifact_path_and_recovery - subagent::subagent_tool_results_spill_to_disk_and_stay_bounded_inline (upstream fa7c4b0) Assert the normalized form via format_artifact_relative_path, matching what the footer actually emits.

The rustfmt pass on the footer-path test assertions (093186a) added 4 lines to production truncate.rs, pushing the aggregate to 677475, 4 over the 677471 ceiling. Tighten ceiling 677471 -> 677475 (measured).

…-3-palette-completion-and-discovery-filte # Conflicts: # scripts/dead-code-budget.json

Upstream 21ed173 renamed work_update -> todo_write (canonical; old names stay hidden replay-only aliases) and updated docs/public-surface-facts.json, docs/TOOL_SURFACE.md, and docs/RUNTIME_SIMPLIFICATION_DESIGN.md — but left web/lib/public-surface-contract.test.ts expecting work_update, breaking the Lint & Type Check gate on every branch: - defaultActive array: work_update -> todo_write (matches facts file) - RUNTIME_SIMPLIFICATION_DESIGN expectation: same rename - web docs tools page: user-visible copy now names todo_write Full web suite: 250/250 passing.

aboimpinto · 2026-08-07T11:36:42Z

✅ Ready for review — all CI green (2026-08-07)

All gates pass on the current head (5d93dd00e):

Lint (fmt + clippy + all budget gates) ✅
Lint & Type Check (web, 250/250 tests) ✅
Test ubuntu / windows / macos ✅ (incl. adaptive_evidence_acceptance and the spillover footer tests)
cargo check (aarch64), link, DCO, GitGuardian, version drift, Claude review ✅

What happened since the draft was opened

Merged latest main twice (2026-08-07: d57ce9d06, then 21ed173cf pre-release repair batch) — resolved budget/contract conflicts as they came.
Upstream fixes carried (origin/main itself was red on these gates):
- 8f2b622dc — Windows-lane truncation footer fix (introduced by fix(tui): honest large-output truncation + recovery path #5212); upstream later made the test path-tolerant, both coexist.
- f0e4d8a26/merge — dead-code budget re-baseline (452; upstream re-baselined to 452 in 21ed173cf — numbers now agree).
- 43a55ce87/3a27a2d81 — runtime-contract budget synced twice (todo_write rename + repair-batch schema growth).
- 24e4ac9b9/0e6185574/merge — source-structure budget re-baselined (677636).
- 5d93dd00e — web public-surface contract synced to todo_write (upstream renamed the tool but missed web/lib/public-surface-contract.test.ts and the tools docs page).
One pre-existing Windows flake observed (run_shell_command_op_allows_readonly_shell_in_auto_mode, shell detection) — passed on the rerun and on the final run.

The FEAT-012 changes themselves (discovery contract + Gherkin harness + palette/completion unification) were untouched by all of this. Ready for owner review.

Paulo Aboim Pinto

…-3-palette-completion-and-discovery-filte # Conflicts: # scripts/runtime-contract-budget.json # scripts/source-structure-budget.json

aboimpinto · 2026-08-07T17:36:45Z

✅ Conflicts resolved again — all CI green (2026-08-07, 2nd sync)

Upstream pushed 3 more commits (efcf47a1d, 4e3087b0e, 59e710271 — incl. their own 0.9.4 budget rebaselines). Resolved by merging origin/main again (eae3b7ffb):

scripts/runtime-contract-budget.json → took **upstreams official rebaseline** (they re-baselined it themselves in 59e7102`); it passes on the merged tree with FEAT-012 code, so our earlier copy was dropped. ✅
scripts/source-structure-budget.json → upstreams 0.9.4 ceiling (676604) + our FEAT-012 delta (+1183) = **677787**, documented in the _todo_2026_08_07_feat012` ledger entry.
subagent/tests.rs merged cleanly.

Current head eae3b7ffb — all gates green: Lint, Lint & Type Check (250/250), ubuntu/windows/macos tests, cargo check aarch64, link, DCO, GitGuardian, Claude review. PR is MERGEABLE/CLEAN, still open for review.

Paulo Aboim Pinto

Paulo Aboim Pinto added 6 commits August 7, 2026 02:52

aboimpinto mentioned this pull request Aug 7, 2026

EPIC: staged command-boundary refactor for #2791 #2870

Closed

12 tasks

Paulo Aboim Pinto added 4 commits August 7, 2026 09:30

Merge remote-tracking branch 'origin/main' into feat/FEAT-012-layer-5…

c9a45fc

…-3-palette-completion-and-discovery-filte # Conflicts: # scripts/source-structure-budget.json

aboimpinto force-pushed the feat/FEAT-012-layer-5-3-palette-completion-and-discovery-filte branch from 1b10a42 to 093186a Compare August 7, 2026 08:23

Paulo Aboim Pinto added 3 commits August 7, 2026 10:39

chore(budget): re-baseline source-structure ceiling after rustfmt reflow

0e61855

The rustfmt pass on the footer-path test assertions (093186a) added 4 lines to production truncate.rs, pushing the aggregate to 677475, 4 over the 677471 ceiling. Tighten ceiling 677471 -> 677475 (measured).

ci: retrigger platform test lanes after runner cancellation

1b36002

ci: retrigger windows lane (flaky pwsh detection, no code change)

ac07033

aboimpinto marked this pull request as ready for review August 7, 2026 10:25

aboimpinto requested a review from Hmbown as a code owner August 7, 2026 10:25

Paulo Aboim Pinto added 2 commits August 7, 2026 12:28

Merge remote-tracking branch 'origin/main' into feat/FEAT-012-layer-5…

3a27a2d

…-3-palette-completion-and-discovery-filte # Conflicts: # scripts/dead-code-budget.json

Merge remote-tracking branch 'origin/main' into feat/FEAT-012-layer-5…

eae3b7f

…-3-palette-completion-and-discovery-filte # Conflicts: # scripts/runtime-contract-budget.json # scripts/source-structure-budget.json

Navigation Menu

Layer 5.3: Palette, completion, and discovery filtering - #5255

Layer 5.3: Palette, completion, and discovery filtering#5255
aboimpinto wants to merge 18 commits into
Hmbown:mainfrom
aboimpinto:feat/FEAT-012-layer-5-3-palette-completion-and-discovery-filte

aboimpinto commented Aug 7, 2026 •

edited

Loading

Uh oh!

aboimpinto commented Aug 7, 2026

Uh oh!

aboimpinto commented Aug 7, 2026

Uh oh!

aboimpinto commented Aug 7, 2026

Uh oh!

aboimpinto commented Aug 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

aboimpinto commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Audit finding — shadowing inconsistency

Changes

Upstream fixes carried in this PR (origin/main was red on CI)

Testing

Uh oh!

aboimpinto commented Aug 7, 2026

CI status: pre-existing Windows failure, not a regression

Uh oh!

aboimpinto commented Aug 7, 2026

Windows-lane repair: upstream failure fixed inside this PR

Uh oh!

aboimpinto commented Aug 7, 2026

✅ Ready for review — all CI green (2026-08-07)

What happened since the draft was opened

Uh oh!

aboimpinto commented Aug 7, 2026

✅ Conflicts resolved again — all CI green (2026-08-07, 2nd sync)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

aboimpinto commented Aug 7, 2026 •

edited

Loading