fix: repair broken core paths (CLI, YNAB import_id, dead code)#13
Open
demonno wants to merge 11 commits into
Open
fix: repair broken core paths (CLI, YNAB import_id, dead code)#13demonno wants to merge 11 commits into
demonno wants to merge 11 commits into
Conversation
Design for deleting dead/broken Revolut module and Transformer abstraction, repairing CLI config loading and the 'import' command, and adding a counter to prevent import_id collisions in the YNAB repository. Co-Authored-By: Claude Opus 4.7 <[email protected]>
Task-by-task TDD plan with six tasks: baseline check, Revolut deletion, Transformer deletion, import_id counter (with failing test first), CLI cleanup, and final smoke verification. Co-Authored-By: Claude Opus 4.7 <[email protected]>
The Revolut reader and transformer referenced fields that did not exist on the CSV model and were never wired into the factory. Removing them also drops the mypy exclude that was masking the broken signatures. Co-Authored-By: Claude Code <[email protected]>
README advertised Revolut as a supported bank and CLAUDE.md flagged Revolut amount parsing as a known issue; both are obsolete now that infra/revolut/ has been removed. Co-Authored-By: Claude Opus 4.7 <[email protected]>
Co-Authored-By: Claude Opus 4.7 <[email protected]>
The Transformer class had no callers after the Revolut removal. Its utility methods duplicate CSVReader's. Deletes the only (skipped) test that exercised it.
- Two supported banks (was: three) after Revolut removal. - Core modules: drop 'transformers' from description — module deleted. - Provider pattern: drop optional transformers.py step — no callers. - Known Issues: drop stale skipped-test entry (test was removed with the Transformer class). Co-Authored-By: Claude Opus 4.7 <[email protected]>
YNAB deduplicates by import_id. With only amount and date in the id, two identical-amount same-day transactions collide and one is silently dropped on import. Adds a per-repository Counter keyed on (account_id, amount, iso_date) that appends :N to the id.
Counter is internal bookkeeping, not a constructor argument. Using init=False/repr=False/compare=False removes it from the public dataclass surface (__init__, __repr__, __eq__). Renamed to _counter for signal-to-reader. New test locks in the per-instance-reset semantic that create_writer in setup.py depends on. Co-Authored-By: Claude Opus 4.7 <[email protected]>
Settings() already loads from env / .env via pydantic-settings; the --config Path option was passed positionally to Settings and raised ValidationError on every run. The handler swallowed the error and continued, so downstream commands crashed on an unset state['config']. The 'import' command now calls import_transactions, matching read-write. Renamed the Python-level function from cli to run_import to avoid the collision with the Typer app variable. Also fixes the pre-existing conifg→config typo in echo output.
run_import previously echoed 'Import transactions with config {settings.dict()}'
which printed ynab_api_key (stored as plain str) to stdout. read-write echoes
only 'Importing transactions' — match that to avoid leaking the token.
The underlying root fix (wrapping ynab_api_key in SecretStr) belongs to the
settings cleanup spec; this change removes the widened exposure introduced
by the import-command wiring in the previous commit.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
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
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
Executes spec 2026-04-19-fix-broken-core-paths-design / plan 2026-04-19-fix-broken-core-paths.
Transformerabstract base (no subclasses after Revolut was removed) and its only, skipped, test.YnabAPIBasedRepository._to_ynabsoimport_idis unique for same-day same-amount transactions — prevents YNAB from silently deduplicating genuine duplicates. NewCounterfield is private (init=False, repr=False, compare=False).--configoption (was passing aPathpositionally to pydantic-settings and raisingValidationErroron every invocation, then swallowing it and leavingstate["config"]unset); wire up theimportsubcommand to actually callimport_transactions.Test plan
make install— clean syncmake lint— ruff cleanmake check— mypy clean (15 files, Revolut exclude removed)make test— 4 passed, 0 skipped (was 2 passed, 1 skipped on master)uv run ynab-import --help—--configgone; four commands listedtest_import_id_counter_suffixes_collisions+test_counter_is_per_instancelock down counter semanticsuv run ynab-import importend-to-end against a real TBC or Swedbank CSV (not run locally — no valid.env)Deferred (explicit non-goals, tracked for future specs)
ynab_api_keyis still a plainstr—settings.dict()echoes fromcheckandreadstill print the token. Newrun_importdoes not echo settings (scoped mitigation on this branch).Settings()eagerly, so subcommand--helprequires valid env. Acceptable per spec.state["verbose"]is set but never read.InMemoryBasedRepositorykept (per spec decision) but still unused.🤖 Generated with Claude Code