fix: resolve numeric reminder ID prefixes

[goutamadwant]

Closes #105

Summary

• keep valid numeric indexes taking precedence over ID-prefix matching
• fall back to the existing prefix resolver when a numeric token is outside the current view
• preserve the existing invalid-identifier error when neither interpretation matches
• document the precedence rule and cover both the regression and compatibility behavior

Validation

• swift format lint --recursive Sources Tests
• swift-format lint Sources/RemindCore/IDResolver.swift Tests/RemindCoreTests/IDResolverTests.swift
• node scripts/build-docs-site.mjs
• git diff --check
• independent correctness review: no remaining findings
• independent security review: no remaining patch-introduced findings

Limitations

• swift test --filter IDResolverTests could not run in the available command-line-tools environment because XCTest is unavailable; the repository's macOS CI is the authoritative Swift 6.2/Xcode validation.
• An in-range all-digit token intentionally remains an index even when it could also prefix-match an ID, preserving the existing command contract.

Disclosure: AI was used to understand the codebase and review the fix.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs real behavior proof before merge.

What this changes

The PR resolves out-of-range numeric reminder identifiers as ID prefixes while preserving valid numeric-index precedence.

Example: The default view has fewer than 4553 reminders, and reminder 45534C82-FE4E-4C30-ABE1-2243ED31ACC8 exists.

• Before: remindctl info 4553 reports Invalid identifier: "4553".
• After: remindctl info 4553 resolves the matching reminder, provided its prefix is unique.

Review scores

Measure	Result	What it means
Overall readiness	🧂 unranked krab (1/6)	The focused implementation appears correct and useful, but its regression tests have no passing execution evidence and required shipped-behavior proof is absent.
Proof confidence	🧂 unranked krab (1/6)	Real behavior proof is necessary before merge. See Before merge.
Patch quality	🦐 gold shrimp (3/6)	No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: A valid all-digit reminder prefix produces an invalid-identifier error instead of selecting its matching reminder.
Reason: The linked report explicitly requests this fallback and retains in-range numeric precedence; the patch repairs all consumers through their existing shared owner.

Merge readiness

⛔ Blocked before merge - 1 item remains

Keep open: this PR provides a focused repair for a current resolver bug, and current main still rejects matching numeric prefixes; no actionable correctness defect was found in the patch.

Priority: P2
Reviewed head: 2f1fe3c57c7f96a9858f31a9f912432977f3b1ff

Before merge

• Add real behavior proof - The changed IDResolver fallback has no supplied after-fix evidence through the shipped CLI: provide terminal screenshots, logs, or live output on macOS showing an out-of-range numeric prefix selecting the intended reminder through info and complete or delete --dry-run, with valid-index precedence retained; redact private reminder content, IPs, API keys, phone numbers, and non-public endpoints. After adding proof, update the PR body, then request a new review with @clawsweeper re-review. The PR author or someone with repository write access can request it.

Findings

None.

Tests

• Missing end-to-end proof: Show the numeric-prefix regression failing on base and passing on head, plus a macOS CLI run resolving that prefix to the intended full ID through info and complete or delete --dry-run.

Agent review details

How this fits together

CLI commands fetch accessible EventKit reminders and pass identifiers to a shared resolver, which returns unique reminder targets for display, links, edits, completion, or deletion.

flowchart TD
  A[CLI identifier] --> B[Command helper]
  C[EventKit reminders] --> B
  B --> D[Default view index lookup]
  D --> E[ID prefix fallback]
  E --> F[Unique resolved targets]
  F --> G[Display or reminder operation]

Loading

Technical review

Best possible solution:

Use the existing shared prefix resolver for out-of-range numeric tokens while preserving valid indexes, ambiguity rejection, overflow safety, and unique targets.

Do we have a high-confidence way to reproduce the issue?

Current-main source deterministically rejects 4553 before matching the reported UUID when the numeric view is smaller than 4553; no target code or tests were run during this review.

Is this the best way to solve the issue?

The patch fixes the shared root cause using the existing matcher and matches the linked report's explicit precedence requirement without introducing a parallel resolution path.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against 61fb13efbc25.

Provenance checked

• Numeric-index rejection and prefix fallback changes intended behavior with a stated reason (d8f9510: The initial CLI interpreted integers before prefixes and rejected indexes outside the list; the PR explains why matching numeric prefixes should now fall through.)
• Bounds validation and default-view numeric mapping keeps the original intent (fix: validate reminder indexes and honor parsing time zones #87: Validate bounds before subtraction to prevent Int.min overflow while retaining the separate default-view numeric source introduced in commit 883ae25.)
• Unique targets and complete input validation keeps the original intent (fix: deduplicate reminder targets before delete and complete #98: Resolve repeated identifiers once to prevent duplicate deletion failures while preserving first-input order and validating every input.)

Testing

Proof path: none.

Security

None.

Evidence

What I checked:

• Current-main failure: Current main parses 4553 as an integer and throws before prefix matching when the default numeric view contains fewer than 4553 reminders; GitHub still reports the pinned main SHA. (Sources/RemindCore/IDResolver.swift:22, 61fb13efbc25)
• Verified introduced change: The introduced diff routes invalid numeric indexes through the existing minimum-length, prefix, ambiguity, and deduplication logic while retaining invalidIdentifier when no numeric-prefix match exists. (Sources/RemindCore/IDResolver.swift:22, 2f1fe3c57c7f)
• Shared command integration: resolveShowIdentifiers supplies all reminders for prefix matching and today's default view for numeric indexes; info, edit, complete, delete, and link share this owner. (Sources/remindctl/CommandHelpers.swift:58, 61fb13efbc25)
• Linked report and intended precedence: All-digit reminder ID prefixes are unusable: any all-digit token is treated as an index #105 reports the concrete 4553 failure and explicitly requests out-of-range prefix fallback with valid-index precedence; this PR implements that expectation.
• Historical safety constraints: fix: validate reminder indexes and honor parsing time zones #87 established bounds-before-subtraction safety, and fix: deduplicate reminder targets before delete and complete #98 established unique targets and complete input validation; this patch preserves both. (Sources/RemindCore/IDResolver.swift:24, 61fb13efbc25)
• Feature history and local history limit: History identifies the initial resolver, the separate default-view numeric mapping, overflow validation, and deduplication. Some historical blobs were unavailable locally with lazy fetching disabled; GitHub REST supplied the initial resolver and the default-view commit patch. (Sources/RemindCore/IDResolver.swift:9, 883ae257911e)

Likely related people:

• Sebastien Tardif: Raw commit dc9fbe4 adds Sources/RemindCore/IDResolver.swift:16 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: dc9fbe4751aa; files: Sources/RemindCore/IDResolver.swift)

Review metrics

Metric	Value	Why it matters
Change size	Production +8/-5; tests +49/-0; documentation +2/-2	The small shared-resolver change is supported by focused numeric-prefix and filtered-view precedence cases.

Labels

Label changes:

• add P2: Numeric prefixes fail for a limited set of reminders, with longer identifiers providing a workaround.
• add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
• add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Label justifications:

• P2: Numeric prefixes fail for a limited set of reminders, with longer identifiers providing a workaround.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Supply the macOS CLI evidence described in realBehaviorProof.
• Run the focused resolver suite on macOS and demonstrate the new numeric-prefix regression failing on base and passing on head.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 17, 2026, 1:20 PM ET / 17:20 UTC.

[steipete]

Thanks @goutamadwant — I reproduced the numeric-prefix failure and incorporated your fix and regression tests into #107, with your co-author credit and changelog thanks preserved. That PR is merged, and the exact-head CI, merged-main CI and signed no-publish release rehearsal all passed. Closing this as superseded by the landed fix.

Proof: https://github.com/openclaw/remindctl/actions/runs/39249845665 and https://github.com/openclaw/remindctl/actions/runs/39251786113.