Skip to content

fix(imports): persist the statement's amount sign convention - #14

Merged
aafre merged 3 commits into
mainfrom
fix/phase-0-ui-honest
Aug 29, 2026
Merged

aafre merged 3 commits into
mainfrom
fix/phase-0-ui-honest

Conversation

@aafre

@aafre aafre commented Aug 29, 2026

Copy link
Copy Markdown
Owner

Forwards PR #12 into main.

#12 was merged into fix/phase-0-ui-honest (its stacked base) rather than main, because #11 was merged without deleting its branch, so GitHub never retargeted it. The commits are identical and already reviewed in #12; this just carries them the last hop.

Contents: amount_sign column on ImportBatchModel, migration 0003_batch_amount_sign, persistence and sticky re-application in apply_patch, and the field on ImportBatchResponse — which is what makes the preview UI's sign-convention selector survive a refresh, and gives a committed batch a record of how it read its amounts.

Gate on this branch: ruff, ruff format, mypy, 103 tests passing.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RYgbBDgmuiRTGt5QNo6313

aafre and others added 3 commits August 29, 2026 22:03
`PATCH /imports/{id}` accepted `amount_sign` and applied it to the candidate
rows, then threw the answer away. Nothing recorded it and nothing returned it,
which broke the feature in two directions.

The browser was the visible half. `renderSignSelector` restores the selector
from `batch.amount_sign`, a field the API never sent, so the guard could never
fire: any re-render of the preview reset the control to unanswered and re-blocked
commit, even though the directions on screen were already correct. The user had
to re-answer a question they had answered, with no way to tell from the page
which convention the rows in front of them were actually read under.

The other half is provenance. A committed batch drops its staged rows and keeps
only the counts and the committed transaction ids as its receipt. If a
credit-card export was booked the wrong way round - every purchase filed as
income - there was nothing on the batch to say which convention produced it, so
an audit had to guess from the resulting signs.

The column is nullable: batches created before this migration, and any batch
whose owner never stated a convention, genuinely have no answer, and `as_written`
is the default behaviour rather than a recorded choice.

The stored convention is now also re-applied on every later patch, not only on
the patch that set it. Directions do survive in the candidate blob today, but
only because `validate()` skips rows whose amount it has already parsed - a
coupling nothing states and nothing tests. Re-applying makes the persisted value
the authority. `_apply_amount_sign` re-derives direction from the raw amount text
instead of flipping a sign, so repeating it is free; the call stays between
`validate()` (which sets direction) and `resolve_duplicates()` (whose fingerprint
covers the signed amount).

Gate: ruff, ruff format, mypy, 103 tests. Migration verified on a fresh database
through `pfa db migrate`, which honours PFA_DATABASE_URL where a direct alembic
invocation does not.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RYgbBDgmuiRTGt5QNo6313
fix(imports): persist the statement's amount sign convention
@aafre
aafre merged commit f0e92bf into main Aug 29, 2026
5 checks 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.

1 participant