Skip to content

Fix review findings: config bugs, parser performance, dead code, CI tests - #3

Merged
fanis merged 2 commits into
masterfrom
claude/project-review-optimization-4xcgkm
Jul 3, 2026
Merged

Fix review findings: config bugs, parser performance, dead code, CI tests#3
fanis merged 2 commits into
masterfrom
claude/project-review-optimization-4xcgkm

Conversation

@fanis

@fanis fanis commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Bugs:

  • Deleting (hiding) a bundled input/output config now actually disables it.
    The parser previously kept hardcoded copies of every bundled config and
    fell back to them even when hidden; bundled assets are now the single
    source of defaults (DefaultJson removed).
  • Grouped-thousands amounts without decimals parse correctly: "€1.234" and
    "1,234" are 1234, not a truncated 1.23.
  • Tapping Fill / Open Expense Manager no longer crashes when the output app
    is not installed, and a failed fill is not marked processed.
  • The dropZeroAmount global config switch is honored (was always on).
  • The config editor no longer corrupts regex patterns containing a literal
    backslash-u sequence on open + save.
  • "Clear local queue" asks for confirmation before deleting.
  • Capture dedupe keys use SHA-256 of the body instead of a 32-bit hash, so
    two different bank SMS can't collide and silently drop an expense.
  • A parser rule with an invalid regex is skipped instead of crashing the
    notification listener at parse time.

Performance:

  • Rule regexes compile once per config load, not on every notification.
  • Stored candidates re-parse only when the config revision changes, with the
    result written back, instead of running the full parser for every row on
    every list read (previously on the UI thread).
  • Notification parsing/DB writes run on a background executor; the review
    queue loads off the main thread; CandidateDb is a shared singleton.
  • The accessibility service filters events to the configured output app at
    the system level and expires an abandoned fill after 15 minutes; per-event
    diagnostics writes are batched.
  • Processed/skipped candidates older than 90 days are pruned.

Maintainability:

  • ExpenseParser split into focused files (ConfiguredParser, InputSource,
    InputRule, GlobalConfig, NotificationText) plus shared IoUtil/ConfigNames/
    JsonValues helpers, removing duplicated readers, file-name cleaning, and
    flag parsing.
  • Notification/accessibility enablement checks unified in BaseActivity with
    exact component matching.
  • Dead code removed (unused parse entry points, listNew, PayeeAliases.keys/
    isBlacklisted, unread automation prefs) and pre-minSdk-26 version guards
    dropped; status/type magic strings replaced with Candidate constants.

Testing:

  • New CI workflow runs the unit-test suite on every push and pull request.
  • JVM parser tests now run the real bundled asset configs; new tests cover
    hidden configs, config-revision reparsing, pruning, dedupe keys, amount
    normalization, dropZeroAmount, and unicode-escape decoding (52 tests).

Co-Authored-By: Claude Fable 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01GvLAiYoWnBfCvHxCUExC41

claude added 2 commits July 2, 2026 07:55
…ests

Bugs:
- Deleting (hiding) a bundled input/output config now actually disables it.
  The parser previously kept hardcoded copies of every bundled config and
  fell back to them even when hidden; bundled assets are now the single
  source of defaults (DefaultJson removed).
- Grouped-thousands amounts without decimals parse correctly: "€1.234" and
  "1,234" are 1234, not a truncated 1.23.
- Tapping Fill / Open Expense Manager no longer crashes when the output app
  is not installed, and a failed fill is not marked processed.
- The dropZeroAmount global config switch is honored (was always on).
- The config editor no longer corrupts regex patterns containing a literal
  backslash-u sequence on open + save.
- "Clear local queue" asks for confirmation before deleting.
- Capture dedupe keys use SHA-256 of the body instead of a 32-bit hash, so
  two different bank SMS can't collide and silently drop an expense.
- A parser rule with an invalid regex is skipped instead of crashing the
  notification listener at parse time.

Performance:
- Rule regexes compile once per config load, not on every notification.
- Stored candidates re-parse only when the config revision changes, with the
  result written back, instead of running the full parser for every row on
  every list read (previously on the UI thread).
- Notification parsing/DB writes run on a background executor; the review
  queue loads off the main thread; CandidateDb is a shared singleton.
- The accessibility service filters events to the configured output app at
  the system level and expires an abandoned fill after 15 minutes; per-event
  diagnostics writes are batched.
- Processed/skipped candidates older than 90 days are pruned.

Maintainability:
- ExpenseParser split into focused files (ConfiguredParser, InputSource,
  InputRule, GlobalConfig, NotificationText) plus shared IoUtil/ConfigNames/
  JsonValues helpers, removing duplicated readers, file-name cleaning, and
  flag parsing.
- Notification/accessibility enablement checks unified in BaseActivity with
  exact component matching.
- Dead code removed (unused parse entry points, listNew, PayeeAliases.keys/
  isBlacklisted, unread automation prefs) and pre-minSdk-26 version guards
  dropped; status/type magic strings replaced with Candidate constants.

Testing:
- New CI workflow runs the unit-test suite on every push and pull request.
- JVM parser tests now run the real bundled asset configs; new tests cover
  hidden configs, config-revision reparsing, pruning, dedupe keys, amount
  normalization, dropZeroAmount, and unicode-escape decoding (52 tests).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GvLAiYoWnBfCvHxCUExC41
Server-side equivalent of scripts/release.sh --push, runnable from the
Actions tab (or an API dispatch) with a patch/minor/major input: bumps the
version, folds the changelog into the release notes, tags, runs the unit
tests, builds the signed APK, and publishes the GitHub Release. The
version-bump commit and tag are only pushed after the build succeeds, so a
failed run leaves master untouched. The workflow-token tag push does not
re-trigger the tag-driven android-release.yml, so the two release paths
never double-publish.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GvLAiYoWnBfCvHxCUExC41
@fanis
fanis merged commit 190cec9 into master Jul 3, 2026
1 check passed
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.

2 participants