Skip to content

fix(parser): accept asterisk list-marker on checkbox regex#36

Merged
bborbe merged 4 commits into
masterfrom
feat/checkbox-accept-asterisk
Jun 28, 2026
Merged

fix(parser): accept asterisk list-marker on checkbox regex#36
bborbe merged 4 commits into
masterfrom
feat/checkbox-accept-asterisk

Conversation

@bborbe

@bborbe bborbe commented Jun 28, 2026

Copy link
Copy Markdown
Owner

Summary

Unifies the seven checkbox regex sites across pkg/storage and pkg/ops so both - [ ] and * [ ] (and their [x] / [/] variants) are accepted as Markdown task lines, matching the linter's pre-existing tolerance for both markers. Closes the silent-skip bug where complete-goal / complete-task / update-task / defer-task / work-on-task ignored asterisk-prefixed checkboxes that the linter just declared fine.

The single replacement site in pkg/ops/complete.go captures the original list marker (([-*])) and reuses it via $1, so a * [/] line becomes * [x], not - [x].

8 vault files in the active personal vault use the asterisk form and become correctly visible to all ops.

Test plan

  • go test ./pkg/storage/... ./pkg/ops/... exits 0 (new asterisk cases added to each _test.go)
  • git grep -nE '\^\(\\s\*\)- \\\[' pkg/storage/base.go pkg/ops/update.go pkg/ops/complete.go pkg/ops/defer.go pkg/ops/workon.go returns zero matches
  • git grep -nE '\(\[-\*\]\) \[' pkg/ops/complete.go returns one match (the capture-group replacement site)
  • make precommit exits 0
  • Pre-existing dash-form test cases pass unchanged

Spec: dark-factory/specs/completed/020-unify-checkbox-regex-accept-asterisk-prefix.md.

bborbe and others added 4 commits June 28, 2026 13:10
All 7 acceptance criteria verified:
- AC1: All 7 checkbox regexes unified with [-*] pattern
- AC2: Replacement site captures and preserves list marker with $1
- AC3: Asterisk test cases added to all 5 test files
- AC4: Asterisk-prefixed lines round-trip with original marker
- AC5: Existing dash-prefixed behavior unchanged
- AC6: CHANGELOG.md has ## Unreleased entry
- AC7: make precommit exits 0

Dark-factory prompts 146-147 executed successfully; vault files using `* [ ]`,
`* [/]`, and `* [x]` are now correctly parsed and rewritten by
goal-completion, task-completion, task-update, task-work-on, and task-defer
operations.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…Note

The PR broadened the detector regex at complete.go:357 to also match
`* [...]` lines, but the inverse uncheck rewriter at line 365 still
hardcoded the dash form (`strings.Replace(line, "- [x]", "- [ ]", 1)`).
That asymmetry meant a `* [x]` line could be matched as a candidate for
unchecking but the rewrite would silently no-op (no `- [x]` substring to
replace).

Replace with the same capture-group preservation pattern already used on
the check branch: `regexp.MustCompile(`([-*]) \[x\]`).ReplaceAllString(
line, "$1 [ ]")`. Both branches now symmetric in marker handling.

Surfaced by local /coding:pr-review. No existing test exercises the
checked=false code path; not adding one in this commit to keep scope on
the regex fix.
@bborbe bborbe marked this pull request as ready for review June 28, 2026 12:18

@ben-s-pull-request-reviewer ben-s-pull-request-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Based on the complete diff analysis, here's my review:

PR Review: fix(parser): accept asterisk list-marker on checkbox regex

Verdict: approve

Summary

The PR correctly extends the checkbox regex to accept * as an alternative list marker alongside - across all 7 detection sites and 2 replacement sites. New tests guard against marker-rewrite regressions. No issues found.


Must Fix (Critical)

None.

Should Fix (Important)

None.

Nice to Have (Optional)

None.


Detailed Analysis

Regex changes (7 detection sites): All correctly changed from - \[ to [-*] \[, preserving capture group indices. No downstream code relies on the old group structure — matches[2] still holds checkbox state, matches[3] still holds task text across all callers.

complete.go replacement sites (2 branches):

  • Check branch: ([-*]) \[([ /])\]$1 [x] — correctly captures and reuses marker
  • Uncheck branch: ([-*]) \[x\]$1 [ ] — correctly captures and reuses marker
  • No cross-contamination: * [x] stays * [x], - [x] stays - [x]

workon.go: Uses matches[1] (the captured marker) in strings.Replace(line, marker+" [ ]", marker+" [/]", 1). The regex ^(\s*)[-*] \[([ x/])\] (.+)$ means matches[1] is exactly - or * (no whitespace), so replacement is correct.

lint.go: Confirmed untouched — it was excluded per spec and the diff shows no changes to it.

Test coverage: New Ginkgo contexts added to complete_test.go, workon_test.go, defer_test.go, update_test.go, and markdown_test.go. Each guards specifically against marker rewrite (verifying * [/] becomes * [x], not - [x]).

CHANGELOG.md: ## Unreleased section added with correct fix description.


{
  "verdict": "approve",
  "summary": "The PR correctly extends the checkbox regex to accept '*' as an alternative list marker across all detection and replacement sites. Tests guard against marker-rewrite regressions. No issues found.",
  "comments": [],
  "concerns_addressed": [
    "correctness: complete.go replacement regex correctly captures and reuses marker via $1 — no cross-contamination",
    "correctness: workon.go uses matches[1] (the captured marker) correctly — marker is '-' or '*' only, not whitespace",
    "correctness: base.go checkboxRegex changed from '-' to '[-*]' — all callers use consistent capture group indices",
    "tests: complete_test.go new context verifies '* [x]' is produced, not '- [x]'",
    "tests: workon_test.go new context verifies '* [/]' preserves asterisk from '* [ ]'",
    "correctness: lint.go NOT modified — confirmed by diff (excluded per spec)"
  ]
}

@bborbe bborbe merged commit 43a4e9c into master Jun 28, 2026
1 check passed
@bborbe bborbe deleted the feat/checkbox-accept-asterisk branch June 28, 2026 12:28
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