Skip to content

feat(classify): add --fail-fast + colored err#70

Open
MrDwarf7 wants to merge 4 commits into
afar1:mainfrom
MrDwarf7:mrdwarf7/cli-options-2
Open

feat(classify): add --fail-fast + colored err#70
MrDwarf7 wants to merge 4 commits into
afar1:mainfrom
MrDwarf7:mrdwarf7/cli-options-2

Conversation

@MrDwarf7
Copy link
Copy Markdown

@MrDwarf7 MrDwarf7 commented Apr 10, 2026

Summary

Add --fail-fast flag to ft classify command to stop on first error, plus colored error output for better CLI UX.

Benefits

  • --fail-fast allows users to see errors immediately without processing all bookmarks
  • Colored red errors make failures more visible in terminal output
  • .gitignore updated to exclude *.log files from classification output

Changes

  • src/cli.ts: Added --fail-fast boolean option to classify command
  • src/engine.ts: Implement fail-fast logic in classification engine
  • src/bookmark-classify-llm.ts: Add error output styling with chalk/red
  • src/bookmarks-viz.ts: Update to use new error display approach

Docs

Updated README with the new --fail-fast option documentation.

Stacked Note

This PR is stacked on top of my other submission ( #69 ) which adds the --format flag to the md command.

(noice)


Note

Medium Risk
Medium risk because it changes LLM process invocation/error handling (invokeEngine) and classification control flow, which could affect all LLM-backed commands and how failures are detected/handled.

Overview
Adds a --fail-fast option to ft classify and ft classify-domains, wiring it through the LLM classification pipeline to stop after the first batch error and printing clearer, colored error messages.

Updates LLM engine invocation to capture and surface stdout/stderr in a structured InvokeError and to treat engine output containing ERROR: as a failure.

Extends ft md with --format to control exported markdown filenames (new reverse-ISO-style default with day-of-week, plus legacy), and updates README/CLAUDE.md local-run guidance and .gitignore to ignore pnpm*.

Reviewed by Cursor Bugbot for commit 5018c74. Bugbot is set up for automated code reviews on this repo. Configure here.

@MrDwarf7 MrDwarf7 changed the title Mrdwarf7/cli options 2 feat(classify): add --fail-fast + colored err Apr 10, 2026
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 5018c74. Configure here.

Comment thread src/engine.ts
Comment thread src/bookmarks-viz.ts Outdated
Comment thread src/md-export.ts Outdated
@cursor
Copy link
Copy Markdown

cursor Bot commented Apr 10, 2026

You have used all of your free Bugbot PR reviews.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

@MrDwarf7 MrDwarf7 force-pushed the mrdwarf7/cli-options-2 branch from 4a38204 to 5018c74 Compare April 10, 2026 17:35
- engine.ts: replace execSync (returns string, not {stdout,stderr}) with
  spawnSync which properly captures both streams. Removes broken
  "as unknown as" cast and shell interpolation hack.
- bookmarks-viz.ts: define ANSI_RE as regex literal instead of
  constructing from ESC constant (literal [ opened a character class).
- md-export.ts: accept ISO dates with time components by checking
  dash positions (>=10 length) instead of strict length===10.

Addresses Cursor Bugbot comments on afar1#70.
@cursor
Copy link
Copy Markdown

cursor Bot commented Apr 10, 2026

You have used all of your free Bugbot PR reviews.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant