-
Notifications
You must be signed in to change notification settings - Fork 958
fix: use change-id as fallback title instead of "Untitled Change" #236
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Fixes #225 by addressing mismatch between proposal template and title extraction: - Updated proposal template to include `# Change: [description]` header - Changed extractTitle fallback from "Untitled Change" to change-id - Updated all extractTitle call sites to pass changeName parameter This ensures both new and existing proposals display meaningful titles.
WalkthroughRefactors title extraction to accept a changeName fallback and updates call sites; adds a "# Change: ..." header line to the agents proposal template so titles are discoverable by the extractor. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (change show/list)
participant ChangeCmd as ChangeCommand
participant Extract as extractTitle(content, changeName)
CLI->>ChangeCmd: request show/list for change-id
ChangeCmd->>Extract: extractTitle(fileContent, change-id)
alt file contains "# Change: Title" or "# Title"
Extract-->>ChangeCmd: return parsed Title
else no header found
Extract-->>ChangeCmd: return change-id (fallback)
end
ChangeCmd-->>CLI: respond with change metadata (includes title)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)src/commands/change.ts (1)
🔇 Additional comments (3)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/commands/change.ts (1)
261-263: LGTM! Fallback logic now uses change-id.The refactored
extractTitle()method correctly useschangeNameas the fallback instead of the hardcoded'Untitled Change'string. The regex pattern/^#\s+(?:Change:\s+)?(.+)$/mproperly matches both# Titleand# Change: Titleformats, and the.+ensures empty headers won't match.Optional: Consider case-insensitive matching for "Change:"
For more lenient parsing, you could make "Change:" case-insensitive:
- const match = content.match(/^#\s+(?:Change:\s+)?(.+)$/m); + const match = content.match(/^#\s+(?:Change:\s+)?(.+)$/im);This would match
# change: Titleor# CHANGE: Titleas well. However, since the template usesChange:with proper casing, the current implementation is acceptable.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/commands/change.ts(5 hunks)src/core/templates/agents-template.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/commands/change.ts (1)
src/core/schemas/change.schema.ts (1)
Change(42-42)
🔇 Additional comments (2)
src/core/templates/agents-template.ts (1)
163-164: LGTM! Template now includes title header.The addition of
# Change: [Brief description of change]to the proposal template ensures new proposals will include a title header thatextractTitle()can parse. This aligns with the fix and addresses the root cause from issue #225.src/commands/change.ts (1)
70-70: All call sites correctly updated.All three invocations of
extractTitle()have been properly updated to pass thechangeNameparameter:
- Line 70:
show()method with validatedchangeName- Line 127:
list()JSON mode within the map iteration- Line 162:
list()long format within the for loopEach call site has
changeNameavailable in scope, ensuring the fallback mechanism works correctly.Also applies to: 127-127, 162-162
Makes the extractTitle regex case-insensitive to handle variations like "# change:" or "# CHANGE:" in addition to "# Change:".
Summary
Fixes #225 - Changes now display their change-id instead of "Untitled Change" when no title header is present.
Changes
# Change: [description]header to proposal template inagents-template.tsextractTitle()to use the change-id (which is already meaningful and kebab-cased) instead of the generic "Untitled Change"extractTitle()call sites to pass thechangeNameparameterImpact
Test Plan
openspec show <change> --json --deltas-onlyoutput format🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements