feat: add CLI issue state command#281
Open
duncanleo wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughThe CLI entrypoint is refactored from a monolithic module into a thin dispatcher that imports command handlers from separate modules. New argument parsing utilities extract flags, options, and positional tokens from argv. Nine command modules implement validate, list, show, create, id, issue state, manifest, pages-index, and schematic-map operations. The schematic-map module includes subcommands for version selection, coordinate counting, snapshot diffing with geometry/topology comparisons, and SVG preview rendering. ChangesCLI Refactoring and Feature Expansion
Sequence DiagramsequenceDiagram
participant CliUser as User
participant runSchematicMap
participant VersionOps as Version Selection
participant DiffEngine as Snapshot Diffing
participant SvgRenderer as SVG Preview
participant FileStore as Filesystem
CliUser->>runSchematicMap: schematic-map list/show/select/stats/diff/generate/preview
alt list or show
runSchematicMap->>FileStore: read manifest/versions/constraints
else select
runSchematicMap->>VersionOps: normalize date, find latest version
VersionOps->>FileStore: load selected version
else stats
runSchematicMap->>FileStore: read snapshot and optional constraints
runSchematicMap->>runSchematicMap: count coordinates by class
else diff
runSchematicMap->>FileStore: read two snapshots
runSchematicMap->>DiffEngine: compare frame, layers, entities, geometry
DiffEngine->>runSchematicMap: semantic diff result
else generate
runSchematicMap->>FileStore: generate snapshot (external)
runSchematicMap->>FileStore: optionally write snapshot and update manifest
else preview
runSchematicMap->>FileStore: read snapshot
runSchematicMap->>SvgRenderer: render SVG with segment geometry, stations, labels
runSchematicMap->>FileStore: optionally write SVG file
end
runSchematicMap->>CliUser: stdout result or file path
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
duncanleo
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/cli/src/commands/issue.ts (1)
10-23: 💤 Low valueOptional: clarify the fallback error message.
issue requires stateis thrown both when no action is given and when an unrecognized action is passed (e.g.mrtdown issue foo). A message echoing the invalid action would aid users.♻️ Suggested wording
- throw new Error('issue requires state'); + throw new Error( + `Unsupported issue action${action ? `: ${action}` : ''}. Expected 'state'.`, + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/commands/issue.ts` around lines 10 - 23, The fallback error message is ambiguous — update the final throw to include the actual invalid or missing action for clarity: inspect the local variable action (from const action = args.shift()) and throw a descriptive error such as "unknown issue action '<action>'" when action is present or "issue requires state: no action provided" when action is undefined; keep the existing branch that handles 'state'/'current-state' and reference variables action, readIssueBundle and deriveCurrentState to locate the logic to modify.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/commands/create.ts`:
- Around line 49-58: runId currently builds the issue id using only the title
(calling buildIssueId(date, title)) which diverges from runCreate that uses slug
?? title; fix by reading the optional --slug via readOption(args, '--slug')
(like runCreate does) and pass slug ?? title to buildIssueId so the printed id
matches what create issue will produce; update the use of readOption and the
call to buildIssueId accordingly (symbols: runId, readOption, buildIssueId,
--slug, --title).
In `@packages/cli/src/commands/schematicMap.ts`:
- Around line 29-38: The function effectiveDateFromDate currently accepts any
two-digit day in the YYYY-MM-DD branch and then slices off the day, allowing
impossible dates like 2026-02-99; update that branch to validate the full date
is a real calendar date before slicing. Specifically, in effectiveDateFromDate
when the /^\d{4}-(0[1-9]|1[0-2])-\d{2}$/ case matches, parse the year, month and
day and confirm they form a valid date (e.g., construct a Date and verify year,
month and day round-trip) and only then return value.slice(0,7); otherwise throw
the same error.
---
Nitpick comments:
In `@packages/cli/src/commands/issue.ts`:
- Around line 10-23: The fallback error message is ambiguous — update the final
throw to include the actual invalid or missing action for clarity: inspect the
local variable action (from const action = args.shift()) and throw a descriptive
error such as "unknown issue action '<action>'" when action is present or "issue
requires state: no action provided" when action is undefined; keep the existing
branch that handles 'state'/'current-state' and reference variables action,
readIssueBundle and deriveCurrentState to locate the logic to modify.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9d99c34c-ca40-482e-bfcf-1a54605534bb
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
packages/cli/package.jsonpackages/cli/src/args.tspackages/cli/src/commands/create.tspackages/cli/src/commands/data.tspackages/cli/src/commands/issue.tspackages/cli/src/commands/manifest.tspackages/cli/src/commands/schematicMap.tspackages/cli/src/index.test.tspackages/cli/src/index.tspackages/cli/src/types.tspackages/cli/src/usage.tspackages/triage/package.json
Comment on lines
+49
to
+58
| export async function runId(args: string[], io: CliIO): Promise<number> { | ||
| const kind = args.shift(); | ||
| if (kind !== 'issue') { | ||
| throw new Error('Only id issue is supported'); | ||
| } | ||
| const date = readOption(args, '--date', { required: true }) as string; | ||
| const title = readOption(args, '--title', { required: true }) as string; | ||
| io.stdout(buildIssueId(date, title)); | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
id issue ignores --slug, diverging from create issue.
runCreate derives the id via buildIssueId(date, slug ?? title), but runId only uses title. When an operator passes --slug, the id printed by id issue will not match the id that create issue actually produces, defeating the purpose of previewing the id.
🐛 Proposed fix to honor --slug
export async function runId(args: string[], io: CliIO): Promise<number> {
const kind = args.shift();
if (kind !== 'issue') {
throw new Error('Only id issue is supported');
}
const date = readOption(args, '--date', { required: true }) as string;
const title = readOption(args, '--title', { required: true }) as string;
- io.stdout(buildIssueId(date, title));
+ const slug = readOption(args, '--slug');
+ io.stdout(buildIssueId(date, slug ?? title));
return 0;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export async function runId(args: string[], io: CliIO): Promise<number> { | |
| const kind = args.shift(); | |
| if (kind !== 'issue') { | |
| throw new Error('Only id issue is supported'); | |
| } | |
| const date = readOption(args, '--date', { required: true }) as string; | |
| const title = readOption(args, '--title', { required: true }) as string; | |
| io.stdout(buildIssueId(date, title)); | |
| return 0; | |
| } | |
| export async function runId(args: string[], io: CliIO): Promise<number> { | |
| const kind = args.shift(); | |
| if (kind !== 'issue') { | |
| throw new Error('Only id issue is supported'); | |
| } | |
| const date = readOption(args, '--date', { required: true }) as string; | |
| const title = readOption(args, '--title', { required: true }) as string; | |
| const slug = readOption(args, '--slug'); | |
| io.stdout(buildIssueId(date, slug ?? title)); | |
| return 0; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/commands/create.ts` around lines 49 - 58, runId currently
builds the issue id using only the title (calling buildIssueId(date, title))
which diverges from runCreate that uses slug ?? title; fix by reading the
optional --slug via readOption(args, '--slug') (like runCreate does) and pass
slug ?? title to buildIssueId so the printed id matches what create issue will
produce; update the use of readOption and the call to buildIssueId accordingly
(symbols: runId, readOption, buildIssueId, --slug, --title).
Comment on lines
+29
to
+38
| function effectiveDateFromDate(value: string): string { | ||
| if (/^\d{4}-(0[1-9]|1[0-2])$/.test(value)) { | ||
| return value; | ||
| } | ||
|
|
||
| if (/^\d{4}-(0[1-9]|1[0-2])-\d{2}$/.test(value)) { | ||
| return value.slice(0, 7); | ||
| } | ||
|
|
||
| throw new Error(`Expected YYYY-MM or YYYY-MM-DD, got: ${value}`); |
There was a problem hiding this comment.
Reject impossible YYYY-MM-DD inputs.
The second branch accepts any two-digit day, so values like 2026-02-99 or 2026-04-00 are silently normalized to 2026-02 / 2026-04 instead of failing. That makes typos look valid and can select the wrong version. Tighten validation before slicing off the day.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/commands/schematicMap.ts` around lines 29 - 38, The function
effectiveDateFromDate currently accepts any two-digit day in the YYYY-MM-DD
branch and then slices off the day, allowing impossible dates like 2026-02-99;
update that branch to validate the full date is a real calendar date before
slicing. Specifically, in effectiveDateFromDate when the
/^\d{4}-(0[1-9]|1[0-2])-\d{2}$/ case matches, parse the year, month and day and
confirm they form a valid date (e.g., construct a Date and verify year, month
and day round-trip) and only then return value.slice(0,7); otherwise throw the
same error.
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
1 participant
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
index.tsas the thin dispatcher.mrtdown issue state <id>to display an issue bundle's current derived state as formatted JSON.@mrtdown/triage/helpers/deriveCurrentStateas a narrow package subpath and wired the CLI package/test script to build against it.Impact
CLI maintainability improves by moving command-specific logic out of the single large entrypoint. Operators can now inspect an issue's derived service/facility state directly from the data repository without writing an ad hoc script.
Validation
npm run build:clinpm run test:clinpm run lintnpm run check:boundariesSummary by CodeRabbit