feat: generic plan filename branch fallback to parent dir#310
feat: generic plan filename branch fallback to parent dir#310bronislav wants to merge 4 commits intoumputun:masterfrom
Conversation
955bff8 to
196d8f7
Compare
There was a problem hiding this comment.
Pull request overview
Updates plan.ExtractBranchName so that when a plan filename is “generic” (e.g., tasks.md, plan.md), the derived branch name can fall back to the parent directory basename (useful for OpenSpec-style layouts where identity is in the directory name), and tightens date-prefix stripping for directory basenames.
Changes:
- Add generic-filename detection and parent-directory fallback logic in
ExtractBranchName. - Introduce a stricter directory date-prefix regex and helpers to reuse stripping logic.
- Extend table-driven tests with OpenSpec layouts, numeric-prefix edge cases, and directory-date stripping scenarios; document the behavior in CLAUDE/docs.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/plan/plan.go | Implements generic-filename handling, parent-dir fallback, and directory date-prefix stripping helpers. |
| pkg/plan/plan_test.go | Adds extensive ExtractBranchName table tests covering new fallback/date-stripping behavior. |
| docs/plans/20260428-generic-plan-filename-branch-fallback.md | Adds a plan doc describing the motivation and intended behavior. |
| CLAUDE.md | Documents the new branch-name derivation behavior for generic plan filenames. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…implementation - stripDirDatePrefix now trims leading '-' after regex replace (mirrors stripDatePrefix) so paths like 2024-01-15--feature yield 'feature' not '-feature' - Add test case for double-dash directory separator - Plan doc: add 'plans' to generic-name set (matches code), remove stale line numbers from Context section, update Solution Overview accordingly
umputun
left a comment
There was a problem hiding this comment.
stepping back from line-level findings, I want to question the approach itself.
most of this is already solvable today, with zero code changes
if the user creates the branch first via a 3-line wrapper (e.g. git checkout -b "$(basename $(dirname $1))"; ralphex "$@"), preparePlanBranch returns early with "already on feature branch" (pkg/git/service.go:183) and CreateBranchForPlan exits without ever calling ExtractBranchName (line 229-230). Branch derivation simply doesn't run. The OpenSpec UX gap on the common path is a wrapper script away from being solved.
the only path where the heuristic actually changes anything is --worktree mode, which calls preparePlanBranch with requireDefault=true (line 285) and uses ExtractBranchName for the worktree dir name (line 272). That's a narrower slice of users.
why a --branch flag and not a heuristic?
a --branch=NAME flag is strictly more general:
- works for OpenSpec, any other tool's layout, custom namespacing (
feat/foo), capitalised variants,spec.md,description.md, etc - ~5 lines, 1 test, no regex, no edge cases
- self-documenting in
--help - covers worktree mode too (the case the heuristic was actually needed for)
the heuristic only helps when filename is in the hardcoded set (tasks, plan, plans, index, readme) AND parent dir is meaningful AND not also in the set. A user with spec.md or description.md gets nothing. So it isn't actually generic, it's five-name-OpenSpec-shaped.
why the heuristic approach is fragile, not just incomplete
the implementation already has bugs that fall directly out of the heuristic-based approach:
-
pkg/plan/plan.go:28-dirDatePrefixRe's compact-date alternative^\d{8}-?strips any 8-digit prefix, not just genuineYYYYMMDD. Soopenspec/changes/12345678-login/tasks.mdreturnsloginand loses the issue ID. The PR description claims "non-date numeric prefixes are left alone" but only the test cases for2fa-/404-exercise that, and they pass because they aren't 8 digits. Real 8-digit dir names (timestamps, ticket IDs) get falsely treated as dates. Fixing this drives towardtime.Parsevalidation or yet-tighter regex, which is the maintenance treadmill heuristics put you on. -
pkg/plan/plan.go:183-184- the godoc no longer describes the actual behaviour, and "generic" isn't defined anywhere a reader can reach without grepping forgenericPlanFilenames. A flag-based design is self-documenting in--help; this one needs prose to explain when each branch fires.
these aren't items to fix in this PR; they're examples of the cost of growing the heuristic. A --branch flag has none of them by construction.
Summary
ExtractBranchNamefalls back to the parent directory's basename when the plan filename is generic (tasks,plan,plans,index,readme— case-insensitive). OpenSpec-style layouts likechanges/add-dark-mode/tasks.mdnow yieldadd-dark-modeinstead oftasks.Directory date stripping uses a stricter regex (
dirDatePrefixRe) — only genuine ISO/compact dates (2024-01-15-,20260428-) are stripped, so non-date numeric prefixes like2fa-or404-are left alone.Fallback stays on the original filename when: path is bare/root, parent dir is also generic, or the stripped dir name is empty (e.g. pure-date dir
20260428).Key files:
pkg/plan/plan.go—ExtractBranchName,genericPlanFilenames,stripDirDatePrefixTest plan
make test— 22 new table-driven cases (OpenSpec layouts, date stripping, edge cases)make lintExtractBranchNametests