AGENTS.md
Rama main · réplica de lectura del repositorio original.
# Working on foilstack
Notes for whoever — or whatever — is editing this repository. The README is for
people deciding whether to run it; this is for people changing it.
## Shape of the thing
Card photographs go in — loose files, or a `.zip` of them. Loose uploads are
packed into an archive in `api_import` before anything else touches them, so
the traversal checks, the expansion ceiling, the duplicate-name suffixing and
the image cap all stay in `extract_archive` with one implementation rather than
two. Each image is encoded by a DINOv3 service and searched against a
catalogue of reference images held in Postgres with pgvector. Confident
matches go straight to inventory; everything else waits in a review queue.
Inventory becomes a CSV the seller uploads to a marketplace themselves.
```
src/foilstack/
cli.py ingest, embed, sets, rematch, sync-prices, enrich, purge,
quota, migrate, plugins
config.py nearly every setting, read once from the environment
db.py the schema. One row in `inventory` is one physical card
search.py nearest-neighbour over card_embeddings (cosine, HNSW)
importing.py archive → scans → candidates. Also `scan_path`
inventory.py pricing rules and the floor, stock lines, totals, export
shaping. `Pricing` is the rule and the floor together.
`items` is the wide read, `index` the thin one the
inventory screen pages over, `position` the aggregate
prices.py price history, and the inline SVG that draws it
enrich.py backfilled days into history, without overwriting one
images.py display-sized copies of scans
embedding.py the client for the encoder. embedder/ is the service itself
plugins/ sources (tcgcsv), enrichers (mtgjson), exports (CSV formats)
web/
app.py the application object, the public pages, /healthz
routes/ accounts, scans, inventory, listings, media
chrome.py the Jinja environment, the topbar figures, _asset_version
deps.py the dependencies every route shares
auth.py accounts, sessions, the single-user escape hatch
ratelimit.py per-process counters on the routes strangers can reach
bodylimit.py the byte ceiling on a request, before a parser writes it down
joblog.py a short in-memory "did that button do anything", per account
proof.py the two catalogue cards the landing page argues with
migrations/ alembic. The schema lives here, not in create_all — and
inside the package so a pip install can run
`foilstack migrate` and build its own
scripts/
preview.py a throwaway instance, to serve or to photograph
shots.py screenshots of a running instance
demo.py the README GIF
landing.py the landing page stills
bump_version.py the version bump, as a pre-commit hook
check_categories.py every TCGplayer category id against its real name
check-tests.sh the suite, on push, failing on a skip
restore.sh a backup back into a database
docs/ accounts, backups, the encoder, plugins, prices
```
`app.py` used to be every route and had reached fourteen hundred lines. The
groups that stand alone are routers under `routes/` now, split by the thing
they act on rather than by whether they answer with HTML or JSON — the route
that confirms a scan and the screen offering the button are one decision. What
made that possible was moving settings out of a module global and into
`deps.settings_dep`: a global bound at import belongs to whichever module
imported first, so every route reading it had to live beside that binding.
## Before you say it works
**Look at the page.** Several bugs have shipped here that no test would have
caught and a glance caught instantly: a reference image rendering at its natural
672×936 because a `<span>` was `display: inline`; the import screen showing its
progress tiles and Start button before a file was chosen, because `[hidden]`
loses to `display: grid`; a "low confidence" warning on a 99% match; queue rows
wrapping at 1280 but not 1440.
```bash
uv run python scripts/preview.py # serve at :8099 until Ctrl-C
uv run python scripts/preview.py --shots ./shots # screenshot and exit
uv run python scripts/preview.py --bulk 30000 # ...as a shop would see it
uv run python scripts/preview.py --backlog 9 # ...with a queue too long to render
```
Either way it is a disposable database, a disposable data directory, an account
already signed in, and a slice of your catalogue so the screens have real cards
in them. It drops the database and removes the directory on exit — on `kill` as
well as on Ctrl-C — and never touches a real deployment. The photographs are
*copied* into it, so discarding a scan in a preview deletes a copy; only
`refs/` is shared, as a symlink, because it is a cache of upstream images keyed
by card id and the preview keeps those ids. Screenshots land in `shots/`, which
is gitignored.
Then actually open the PNGs. A screenshot you did not look at is worth nothing.
Playwright is a dev dependency; `uv run playwright install chromium` is enough,
and the `--with-deps` step that wants sudo is not needed.
**Re-record the README animation when you change a screen it shows.** It covers
the queue, the inventory table, a card page and the listing run, so a change to
any of those dates it:
```bash
uv run python scripts/preview.py --demo src/foilstack/web/static/demo
```
Same disposable database as `--shots`, so nobody's real inventory is ever on
camera. It writes one committed file — the GIF the README shows — plus a
gitignored webm, which is the one to post anywhere that takes real video. It
used to write two animated WebPs as well, for a landing page that played the
animation in its hero; the landing page uses stills now and those were deleted.
**Re-shoot the landing stills when you change a screen one of them shows.** The
same rule, for the same reason, on the other set of images:
```bash
uv run python scripts/preview.py --landing src/foilstack/web/static/shots
```
Four committed files: the review queue (the hero), a card page with its price
trend, the listing run, and `og.png` — which is not a screenshot but a composed
card, because a link preview is rendered about 500px wide in a feed and an
application screen at that size is a grey rectangle.
The stills replaced a 3.0 MB animated WebP that was 94% of the landing page's
weight and the largest thing painted inside the fold. It also carried its own
copy of the application nav bar, so it announced a stale version number in the
middle of the hero for as long as nobody re-recorded it. The whole set is now
about 450 KB, and each still goes out of date on its own rather than all at
once.
Then watch it. Every fault this has had was invisible in the code and obvious in
the output: scrolls that silently moved nothing and vanished from the GIF when
identical frames collapsed, a queue seeded with the same card twice, a price
trend panel reading "no price history" on the one card chosen to show it off.
The size ceiling is not the `check-added-large-files` hook, which is set to
2048 KB and does not apply: it only inspects files being *added*, and the GIF
has been tracked since the beginning. It currently sits at about 4.4 MB and
commits without complaint. The real ceiling is camo, GitHub's image proxy,
which stops serving somewhere near 5 MB — and a README whose only picture
silently fails to load is the failure that matters here. Buy headroom by
scrolling less rather than by dropping the frame rate, because a scroll at five
frames a second reads as broken.
Capture is at `device_scale_factor=2` and the GIF is written down to 1000px at
192 colours. Those are separate numbers on purpose: the browser is asked for
twice the CSS size so the downsample has real detail to work from, and 1000 is
what GitHub's content column actually renders. The palette is close to free —
at 800px, 192 colours came out fractionally *smaller* than 160 — so width is
the whole cost, and stopping at 1000 rather than 1200 is the camo margin again.
The preview seeds runners-up, not just the top match. It used to seed one
candidate per scan, which quietly made the demo argue less than the product
does: `also matched: ...` never rendered on a queue row, and the "wrong card?"
panel opened on a heading promising a choice above a single option.
## Formatting and types
Install both hook types before your first commit:
```bash
uv run pre-commit install --hook-type pre-commit --hook-type pre-push
```
On commit: formatting, linting, types, the version bump, and `gitleaks` —
deliberately a hook and not a CI job, because this repository is public and a
credential CI catches is one that has already been pushed and can only be
rotated.
The bump fires only when something under `src/` is staged — a README fix is not
a new version of the software, and a number that moves for everything means
nothing — and never during a merge, rebase, cherry-pick or revert, which replay
commits that already carry their own version. `uv run python
scripts/bump_version.py --dry-run` says what it would do without touching
anything.
On push: the test suite, which also fails if anything **skipped**. Not on
commit, and that is the whole design — `tests/test_isolation.py` needs Postgres
and skips cleanly without it, so a plain pytest hook on a machine with no
database reports green while the sixty-six tests proving one account cannot
read another's cards did not run. Requiring a database before every commit is
friction nobody keeps; before every push it is the right trade.
`ruff` formats and lints; `mypy` type-checks `src/`. All three run in
pre-commit and in CI, and the settings live in `pyproject.toml` so a local run
and the hook cannot disagree.
```bash
uv run ruff check --fix . && uv run ruff format . && uv run mypy
```
Two settings that look like laziness and are not:
* **B008 is exempted for FastAPI's `Depends`/`Query`/`Form`**, by name rather
than by switching the rule off. Those are function calls in argument
defaults on purpose; B006 still catches a real mutable default.
* **mypy is not `--strict`.** Strict reports 142 errors, nearly all "annotate
this". A number that size gets ignored wholesale rather than fixed, and then
the check means nothing. Tighten a module at a time as annotations arrive.
## Tests
```bash
docker compose up -d postgres && uv run pytest -q
```
`tests/test_isolation.py` builds its own database and drives the real
application. It **skips** when Postgres is unreachable, so a green run proves
nothing about account scoping unless the server was up — check that the skip
count is zero before trusting it. That has already caught one credential change
that silently disabled twelve security tests.
Two habits worth keeping:
* **Assert on behaviour, not markup.** A test matching `"<tr data-row>"` broke
the moment the row gained a class, and the failure said nothing useful.
* **Make tests self-contained.** Three tests once leaned on a fixture row that
earlier tests in the same module edited; they passed alone and failed in
sequence, which is the least useful way for a test to fail.
## Things that have bitten
* **Never store an absolute path in the database.** `scans.stored_path` is
relative to the scans directory. The compose file mounts `./data` at `/data`,
so a row written by the CLI on the host was unreadable from the container and
every thumbnail 404'd. Same rule for derived files: display copies are keyed
by the scan's path, not its row id, because a row id is unique in one database
and a data directory can be shared with another.
* **A preview is only disposable in the parts that were made disposable.**
`scripts/preview.py` built a throwaway database and then seeded it with real
`stored_path` values, because the matches it shows have to be real ones. So
every preview scan row pointed at a photograph in the live `data/scans`, and
`purge_scans` — which unlinks the scan and its display copy whenever one is
discarded — was aimed at the seller's own images. Pressing `Discard all` on a
seeded queue attempted five hundred unlinks against production data. Every
one failed, and only by luck: the per-scan directories happened to be owned
by another user, and `purge_scans` logs an unlink it could not do and carries
on, so nothing on screen said a word either way. The docs above claimed it
"never touches a real deployment", which is the sentence that makes the trap
worth naming — it invites exactly the click that springs it.
The preview owns a `FOILSTACK_DATA_DIR` of its own now and the photographs
are copied into it, re-pointed to a **relative** path on the way — an
absolute `stored_path` is handed straight back by `scan_path`, so a private
directory alone would not have been enough. `refs/` stays shared on purpose:
it is a card-id-keyed cache of images fetched from upstream, ids come across
with the catalogue rows, and nothing ever deletes from it, so the alternative
is re-downloading every reference image on every screenshot run.
Teardown had the same shape of hole. `finally` dropped the database, but only
Ctrl-C ever reached it — a `kill` ends the process without unwinding — and it
never stopped the uvicorn child, which a terminal's Ctrl-C was killing for it
by hitting the whole foreground group. Sent a signal directly, the script
exited while the server kept the port and kept the database open, and the
drop then failed with "is being accessed by other users". Both halves are
handled now; the symptom was preview databases nobody could name.
* **A card id is a row number, not a name.** `37` is Base Set Charizard in one
database and something else entirely in the next, so nothing outside a single
install may hardcode one. `web/proof.py` finds the landing page's two cards by
name and set, and shows no thumbnails at all when the catalogue has not been
ingested — which is the honest state for an install that cannot yet identify
anything.
* **Scope every read by `user_id`.** Ownership is `NOT NULL` and single-user
mode is *one account*, not *no account*, so there is one query shape and no
branch that could forget. `inventory.items()` takes `user_id` positionally on
purpose — a scoping argument with a default is one a caller can omit.
* **The job log is keyed by account too.** It is in-memory and deliberately
tiny, but its messages name filenames, SKUs, row counts and export sizes, and
every one of those is somebody's business data. A single process-wide log
reads fine on a self-hosted install and hands a hosted seller's activity to
whoever loads the listings page next.
* **A write a browser can send twice must survive being sent twice.** One row
in `inventory` is one physical card, and confirming a scan inserted one
every time the request arrived — so a double-click, a proxy retry, or the
same `scan_id` twice in one bulk payload made two cards out of one
photograph, which then counted, priced and exported. `_confirm` is
idempotent and returns whether it created anything; the partial unique index
on `inventory.scan_id` is the half that survives two requests racing, and
`FOR UPDATE` on the scan is what makes them queue instead of collide.
* **An import only advances while the web process is alive.** `run_import` is
a `BackgroundTasks` coroutine, not a queued job — nothing outside the process
knows it is running and nothing owns it across a restart. The archive was
staged in a `mkdtemp` the reboot took with it, and on the cohort pass the
candidate pool is only ever in memory, so there is nothing to resume. Left
alone the row keeps saying `matching`, and the import screen polls it every
700ms forever under a bar that sweeps for work nobody is doing. Startup
sweeps them to `failed` with a message naming what landed. Boot is the proof
and no timestamp is needed: if this process is starting, no import is running
anywhere — which stops being true the moment there are two replicas, so that
assumption is written at the call site. The sweep is also the first thing at
boot that opens a connection, `db.init` only building the engine, and it is
wrapped for that reason: housekeeping that can fail a boot fails harder than
the rows it was tidying.
The statuses live in one tuple, `importing.ACTIVE_STATUSES`, because the
screen's own copy was missing `grouping` — so a job killed during matching
hung the page and one killed during the cohort pass vanished from it in
silence. Two different wrong answers to the same event, from one list
written twice.
* **An `async def` that does synchronous work stops the whole process.** The
web app is one uvicorn process with one event loop, and `run_import` used to
do all of its work on it: `extract_archive`, every Pillow resize, every
search and commit, with the encoder request the only `await` in the loop.
While one seller's archive unpacked, nothing else on the install was
answered — other accounts, that import's own progress poll, `/healthz`.
Nothing failed, so nothing said so; it reads as a slow site.
`run_import` is still a coroutine, because `BackgroundTasks` awaits it, but
all it does is hand `_run_import` to a pool of `IMPORT_WORKERS` threads. Its
own pool and not Starlette's: an import holds a thread for minutes, and one
waiting its turn in the forty-thread pool the `def` routes share would hold
a thread doing nothing. A job waits as `queued`, which the screen says in
words, because `pending` is what it reads as "unpacking scans".
`test_an_import_does_not_stop_the_process_answering` holds an import inside
extraction and needs the loop to release it, so a regression times out
there rather than passing.
This paragraph used to end on three routes "left alone" with the same
shape. By the time anyone went to fix them there were six, plus the
TCGplayer round trip resolving a whole selection in SQL on the loop and
`/api/import` itself still `async` around its quota sum and job insert. The
list in this file was the only thing tracking them, and new routes never
got added to it. The six awaited nothing but `request.json()`, which is no
reason to be `async`. They are `def` now and take the body as
`payload: dict[str, Any] = Body(...)`, and the other two read their
already-spooled uploads through `.file` from the threadpool.
`tests/test_routes_off_loop.py` is the rule now: it walks the route table
and fails on any `async def` endpoint not in `ASYNC_ROUTES`, which names the
I/O each exception actually waits on. Two are left — the reference-image
proxy and `/plugins`, both waiting on another server — and both hand their
queries to `run_in_threadpool`. An `async` route waiting on an empty
connection pool holds the loop for the pool's whole thirty-second timeout,
so a point lookup on the loop is not small, however cheap the query.
* **A floor is the seller's number, not the software's.** `list_price` used to
clamp at a module constant, which is one answer for every account on the
install — right for a dealer clearing boxes at pennies and wrong for a shop
that will not put a card in an envelope under a dollar. It is
`users.price_floor` now, and deliberately *not* an environment variable: a
hosted server holds sellers whose bulk policies genuinely differ, and one
setting in `.env` makes the busiest of them answer for the rest.
The rule and the floor travel as one `inventory.Pricing`, positionally and
with no default, for the same reason `items()` takes `user_id` that way. A
caller that remembers the rule and forgets the floor raises nothing — it
prices a seller's cards at 35c when they said a dollar, and every number it
produces looks like a price. `deps.pricing_dep` is the one place a route
gets one and `Pricing.for_user` the one place that knows where a floor
lives.
Applying a floor and saving one are two presses, and that is the whole
design. `/listings?floor=` prices one run; `POST /api/account/floor` is what
changes every screen. A figure typed to see what a shelf of bulk would come
to must not become the seller's policy because they hit Enter in a field —
so the screen always states which of the two is in force, and offers to save
it only when they differ.
The two ends also disagree about a bad value on purpose. `parse_floor`
raises; `Pricing.of` turns that into a fallback, because a stale bookmark
should price the run the ordinary way rather than 400 at somebody holding a
file they need. The save route does not fall back — a seller who meant to
set a dollar and silently got the shipped 35c would find out from a payout.
And an unusable `?floor=` falls back to *the account's* floor, not the
shipped one, which is a distinction `Pricing.of` cannot make because it has
no user; `for_user` makes it.
* **Inventory browses; listings lists.** The inventory bar used to carry an
`eBay CSV` and a `TCGplayer CSV` button, and the second of them handed the
seller a file that cannot be uploaded — `tcgplayer.toml` blanks the SKU id
and `Total Quantity` on purpose, and only the round trip through
`/export/tcgplayer/match` fills them. Styled identically to the eBay button,
which does work, it was a one-click path to a dead file. Both are gone from
that screen. `/export/{name}` stays as a route; what changed is that
reaching it goes through `/listings`, where the pricing rule, the channel
and the upload the round trip needs are all on screen. Anything that turns
inventory into a marketplace file belongs there, and the bridge is the row
selection and `List selected →`.
* **A guard against a bad filter value must not cancel a good one.** Picks
arriving in the querystring are narrowed to the values the account actually
owns, so a stale bookmark naming a set they sold out of is ignored rather
than emptying the screen. Narrowed to the values *currently on screen*
instead — which is what it did first, and what the screenshot caught — the
same line silently drops a live filter: pick LP, search for a card with no LP
copy, and LP is no longer "present", so the pick is discarded, its chip
disappears, and the screen answers with the near-mint copies. A filter that
stops applying with nothing on screen to say so is worse than one that
matches nothing. `present_values` takes the copies, not the rows, for that
reason, and `facet_options` paints a picked value at zero so there is always
something to click to undo it.
The same rule is why `_fold_filters` never folds a filter that has something
picked. The facets are menus now rather than rows of chips — a row painted
what it could fit and then admitted to hiding the rest, which at a hundred
sets is a `+90` nobody can click — and the tail of that row folds behind
`More filters` because five of them wrap at 1000px. A filter in force behind
a disclosure is the same silent filter as before and worse: there is nothing
on screen to suggest looking. A picked filter claims a standing slot rather
than being exempt from the budget, which is also what lets the disclosure be
forgotten between page loads — the filter just picked from comes back
standing because it now has a pick. `More filters` is a link before it is a
script, so the folded pair is reachable with JavaScript off, and it is the
one control on that screen that keeps the page number, because it is the one
that does not change the result.
A menu also has to say what it is doing while shut, which a row of chips
never had to: `any_label` is the unfiltered case in words, and a picked
filter reads its own value. That is the cost of the trade and it is the half
that goes wrong quietly.
* **A quota is only real if something gives the bytes back.** `usage_bytes`
sums `scans.size_bytes`, and discarding used to move a status and nothing
else — so storage only ever grew, and the 413 telling a full account to
"discard some scans first" asked for something that could not work.
Discarding now deletes the image and zeroes the row's `size_bytes`. The row
and its candidates stay: they are the record of *why* a scan was thrown
away, and they cost almost nothing next to the photograph. Deleting an
inventory row deliberately does **not** purge — bulk delete refuses sold
rows on the stated grounds that an in-stock row is recoverable "because its
scan is on disk", and that promise has to stay true. `foilstack purge` is
where an operator asks for those.
* **A route cannot limit a body it has already been handed.** `api_import`
counted every byte against the archive cap and the account's quota, and the
TCGplayer round trip against its own ceiling, and both counts ran after the
bytes were on disk. FastAPI resolves `File(...)` before it calls the route,
and Starlette spools each file part to a temporary file with no limit:
`max_part_size` is for ordinary fields and skips files by design. Measured,
a 20 MB body against a 1 KB cap was refused at 1 KB after 20 MB had been
written to a `/tmp` on the database's disk — by any signed-in account, or by
anybody at all in single-user mode.
`web/bodylimit.py` answers in front of the router: a declared
`Content-Length` past the ceiling is refused unread, and a body that
declares none is counted as it streams. The route checks stay and are still
the exact ones, because only a route knows whose quota is being spent. Two
ways the outer check goes wrong quietly: keyed by path, it goes stale when a
route is renamed, which is what `test_every_upload_ceiling_names_a_real_route`
is for; and without room for multipart framing — one part per loose image —
it breaks the size the import screen promises by arithmetic.
* **`--forwarded-allow-ips '*'` trusts the visitor, not the proxy.** Under
`*`, uvicorn takes the *left-most* `X-Forwarded-For` entry, and Cloudflare
appends to a header the client already sent rather than replacing it — so a
request to the live site carrying `X-Forwarded-For: 203.0.113.9` was logged,
and rate-limited, as 203.0.113.9. Given a real list, uvicorn takes the
right-most address it does not trust, which is the one the proxy added.
`FORWARDED_ALLOW_IPS` defaults to Docker's container range and travels as an
environment variable, because a flag on the command line beats it.
It was hiding a second bug. The per-address registration budget was spent
only by failures, so a limit described as tight "because every success costs
a row" never counted a success, and one honest address could open accounts
indefinitely. Every attempt spends it now, and `_signups` bounds the
accounts created across every address — the one budget nobody can refresh
by arriving from somewhere new, whatever the proxy reports.
* **A quota has to be enforced on what an upload becomes.** Both checks in
`api_import` measure the archive as it arrives on the wire, and what fills a
disk is what comes back out of the zip. `.tif` is in `IMAGE_SUFFIXES` and an
uncompressed TIFF deflates about 1000:1, so 2.2 MB accepted against the
quota wrote 2.2 GB under it; the only ceiling on the way out was
`MAX_TOTAL_BYTES`, one number for the whole disk that says nothing about
whose account is spending it, and registration is open by default, so that
4 GiB was per signup. `extraction_ceiling` is the account's number and
`extract_archive` takes it as an argument — the constant answers for the
disk, the caller answers for the account.
The slack is deliberate. It is room-left *plus one* `max_archive_mb`,
because the extracted size cannot be known before extracting and a hard edge
would truncate an ordinary batch mid-archive for landing near the line. A
seller slightly over is a better answer than a seller holding half a shelf
of cards. For the same reason the ceiling goes to the log and not to the
seller: it is a sum they cannot reconcile with the quota they were told
about.
What made this worth more than a one-line change is the failure path.
Nothing removed the files already written when extraction refused, and those
bytes have no `Scan` row — which is the only thing `usage_bytes` counts and
the only thing `foilstack purge` can find, so they are charged to nobody and
reclaimable by nothing. Survivable at 4 GiB, where reaching it meant a
hostile archive. Not survivable once the ceiling is the account's own quota,
because then the *ordinary* over-quota import is the one that leaks and an
account refused for being full fills the disk being refused. Lowering a
ceiling turns its rejection path from an edge case into the common case, so
the rejection path is the half to read first.
* **A page that reads all of something has a size it stops working at.** The
inventory screen built every row the account owned — three times, through
`items()`, which is the wide dictionary a card page needs — and `_chrome`
built it a fourth time on *every* screen in the application to draw two
numbers in the topbar. At 150,000 rows that was fifteen seconds and 1.5 GB
for one page, and four seconds on screens that never mention inventory.
It did not degrade into that, either. `_prices_for` fetched with
`card_id.in_(...)`, which renders one bind parameter per id against a
protocol that carries 65,535 — so an account holding more distinct cards
than that got a 500, on every page at once, with nothing on the way there
getting slower to warn anyone. A seller buying collections reaches 65k.
**Never pass a list of ids to `.in_()`.** Use `db.id_in(column, ids)`, which
sends `= ANY(:ids)` — one array parameter, no ceiling, and faster where the
list is large. This was a `BIND_CHUNK` constant in `inventory.py` first,
applied at two call sites out of seven; the other five included `purge_scans`,
which `foilstack purge` hands every discarded scan on the install, and a
second chunk size in `importing.py` tuned separately from the first. A rule
you have to remember at every call site is one a call site will forget, which
is the same reason `items()` takes `user_id` positionally and `Pricing` has no
default floor. `.in_()` is still right for a fixed set of statuses or
sub-types — the rule is about lists that grow with what a seller owns.
Three things answer it, and the order matters. `position()` is the topbar as
one aggregate query. `index()` is the thin per-copy read the screen folds
over — same key names as `items()`, minus what only a card page uses.
Paging is last and is worth least on its own: lazy-loading rows into a table
fixes nothing while the server still folds the whole inventory before
rendering the first one.
**`priced_printing()` is the only definition of which printing prices a
row.** `position()` and the sort keys need a market price per row without
materialising the inventory, so the rule has to exist as SQL. It existed as
Python too for a while — `resolve_printing` and `pick_printing`, for the
callers that already held a price map — and that duplication was the real
cost of the change above, of the quiet kind: a topbar disagreeing with the
table beneath it by a few dollars reads as rounding.
The Python copy is gone. What made it worth deleting rather than guarding
was that the split fell along no line that meant anything — `/inventory`,
`/listings` and the topbar on every page read the lateral, so the Python
half served the card page, the edit panel and the export while carrying the
whole of the drift risk. `index()` and `items()` are one `_read()` now,
differing only in whether the sixteen detail keys get built, so they agree
by construction rather than by a test.
The cost is that the rule can no longer be driven without a database. Its
cases moved from `tests/test_inventory.py`, where they were pure-function
tests against `pick_printing`, into `tests/test_inventory_scale.py`, which
skips when Postgres is unreachable — which is survivable only because the
pre-push hook fails on a skip. `finish_of` is the half that is still a
plain function and still tested as one. `position()` remains a third
expression of the rule as a grouped aggregate, and is still held to the
other two.
Look at it with `--bulk`. A pager cannot be reviewed on one page of results,
and the bugs paging introduces — a facet click that keeps the page number, a
select-all that silently means "this page", a row count reporting the window
as the answer — are all invisible until there is a second page. `--bulk`
widens the catalogue before padding inventory for that reason: the screen
groups by card, so padding rows alone against the seeded 152 cards buys 152
lines however many rows go in.
* **The queue holds one upload at a time, and says so about the rest.** Every
waiting upload is a heading on the screen, oldest first, each naming its own
size from a count in SQL; exactly one of them is open and its cards are the
only rows built. `_waiting_jobs` is the headings, `_open_job` is which one
opens, `_queue_rows` takes that one job. A shut heading is a **link**, so
opening it is a page load and a backlog of nine uploads costs the same page
as one — and works with JavaScript off, and puts the batch in the URL where
it can be reloaded onto.
This replaced a row cap, and the cap is worth remembering because both of
its shapes were wrong. It first read the newest 400 waiting scans by id
while the screen presented them oldest-upload-first, so it took the front of
the backlog — the exact section the seller is sent to work. Nine archives
waiting showed twenty cards of the fifty in the oldest; confirming those
twenty freed twenty slots and the same section came back with the next
twenty, which reads as a screen loading a page at a time and is really a
screen hiding work. Worse, every count came from the truncated list, so
nothing on the page admitted to it: the heading said `20 cards` over a batch
of fifty, the tile said 400 with 430 waiting, and the Commit button named
the same wrong number. Whole sections up to a budget fixed the lying counts
and left the rest — a screen that renders some batches, holds others back,
and has to print a footnote explaining that they appear "once the batches
above are cleared". One batch at a time needs no footnote, because nothing
is held back: the uploads that are not open are all on the screen, with
their real sizes, one click from their cards.
Which one opens: the batch asked for by `?job=`, then the one this browser
last asked for, then the newest. Newest last rather than oldest, though the
backlog is worked front to back — an import returns the seller here and
finishing one to somebody else's batch reads as an import that failed, which
is why `poll()` names the job it just finished on the way back. What makes
that safe is that the older batches never leave the screen. An id naming
nothing waiting is what a cleared batch looks like — the last row is
confirmed, the page reloads, and the id in hand is finished — so it advances
to the next upload down the queue rather than throwing the seller back to
the newest.
The memory is a cookie and is written **only when a batch was asked for by
name**, which is the same distinction the match panel draws: a screen
reached from the nav bar should not re-pin it to whatever happened to be
open. It has to be a cookie rather than localStorage for a stronger reason
than the folds it replaced had — the answer decides what gets *built*, not
just what gets shown, so restoring it in the browser would mean rendering
one batch's cards and then fetching another's.
The counts split cleanly and have to: `waiting_total` and the headings are
about the whole queue, and the filter chips, `Commit` and `Discard all` are
about the open batch — those build their payload from the DOM, so captioning
them with a queue-wide number would promise more than they do.
Shut heads carry no `#job-N` fragment, though it is the obvious addition:
landing on the heading pins it to the top of the viewport and scrolls the
bar above the list — the filter chips and the Commit button — off the
screen. Arriving at a batch with its own controls missing is worse than
arriving above a few lines of headings.
Look at it with `--backlog`. Two uploads and a dozen cards is a screen where
every one of these decisions looks the same as any other, which is why the
seeded preview never caught the truncation and cannot show this either.
* **A selection stopped being a list of ids the moment the screen was paged.**
The inventory form submits one `id=` per *copy*, and one page of a hundred
lines is around 740 of them — a 6.4 KB querystring, already inside the 8 KB
a proxy will commonly accept, before a shop with twenty copies of a staple
is considered. "Select all 3,884 matching" would have been a quarter of a
megabyte. So the filter travels instead and `/listings` resolves it, which
took the same run from 6,443 bytes of URL to 84.
`deps.Selection` carries three modes and the mode is always **stated**.
`sel=""` means the ids and only those; `sel=page` and `sel=all` mean a
filter. That is deliberate: `/listings` used to read "no ids" as "everything
the account owns" while the filter was nowhere on the form, so a screen
reading `Not listed 75` handed off a run priced over the whole inventory.
An unrecognised `sel` reads as the ids, never as everything.
Both ends narrow through **one** `inventory.narrow`. Not two implementations
tested for agreement — one implementation, because the failure here is a
seller listing a hundred lines they never saw with nothing on either screen
to say so. `sel=page` therefore has to carry the sort as well as the page
number: a window means nothing without the ordering it is a window on, and
the first version left `sort` out of the template context, where Jinja
rendered the unknown name as `""` and the far end read that as
"unrecognised" and fell back to the default. Page 2 of market-descending
resolved page 2 of name-ascending, silently, on both sides.
A filter run is resolved against the data as it stands when it is priced,
not as it stood when the page was drawn — so `/listings` states its own
terms ("all 3,884 lines · unlisted") rather than printing a bare count.
Bulk delete is deliberately **not** offered for a filter selection: it takes
ids, and inventing a meaning there would put the widest possible selection
one click from the most destructive button.
* **Route declaration order survives into the route table.** FastAPI matches
routes in the order they are declared and routers in the order they are
included, so a same-shape pair still depends on which came first. Declare
`/api/inventory/{item_id}` ahead of `/api/inventory/delete` and a POST to
`delete` is captured by the id route, `"delete"` fails to parse as an integer,
and bulk delete answers 422. The extraction out of `app.py` preserved the
original order for exactly that reason.
* **Settings resolve per call, not at import.** `deps.settings_dep` reads
`get_settings()` on each request — a cached dict lookup, and the reason
`get_settings.cache_clear()` takes effect at all. Bound at import it does not:
a module imported before a fixture repoints the application at a throwaway
database keeps the first settings object it ever saw, and the suite then fails
against the developer's own database with an error that names a password
rather than an ordering. That cost hours, twice.
* **A `NOT NULL` column needs a `server_default` in the migration**, dropped
afterwards. Autogenerate will not add one, and the migration fails outright on
a table that already has rows.
* **`flex-wrap: wrap` wraps on `flex-basis`, not `min-width`.** Items only
shrink after wrapping, within a line.
* **A setting has to be named in three places.** `config.py` reads it, the
`web` service in `docker-compose.yml` passes it through, and `.env.example`
documents it — and compose forwards only what that block lists, so a setting
added to the first and the third but not the second is a knob that silently
does nothing on the one install path the README describes. The settings
that live outside `config.py` are easier still to miss: `MAX_IMAGES` in
`importing.py` and `FOILSTACK_EMBED_CONCURRENCY` in `cli.py` are module
constants read at import, and the first of them is printed on the import
screen as a promise to the seller. `FORWARDED_ALLOW_IPS` is read by uvicorn
itself, and `.env` names it `FOILSTACK_FORWARDED_ALLOW_IPS` so compose can
pass it through.
* **Auto-accept is off unless the seller turns it on.** It is the one control
on the import screen that puts a card into inventory with nobody having
looked at it, and inventory is what gets priced, exported and sold against —
so it ships with an `Off` chip selected and no percentage offered as the
default. That is a different answer from "fall back to the configured
threshold", and the difference lives in one place: `ImportJob.auto_accept`
is NULL for a job that was never given one, and `_job_accepts` reads NULL as
never rather than as the default. The browser omits the form field entirely
when Off, so there is no value on the wire that means off and no way for a
dropped field to become a permissive one. `_may_auto_accept` keeps its own
`settings.auto_accept` fallback because it is the matching policy and its
tests are about the three rules, not about who asked.
`FOILSTACK_AUTO_ACCEPT` now names *the threshold on offer*, not one already
running. It is still a chip — the grid is the three standard percentages
plus the configured value, so an operator who set 0.90 can pick 0.90 and a
remembered threshold always has a chip to paint on — and both sides round to
two places, so a configured 0.945 lands on the 94% chip rather than on
nothing. The row had a real bug before any of this: it offered 88/92/96
against a shipped default of 94, so a stock install painted nothing while a
threshold was very much running.
* **The match panel is remembered per account, in a cookie.** A finished
import returns to `/app` through `location.href`, which is a fresh page and
a fresh `settings` object, so a seller working through a shelf of boxes
re-answered the whole panel for every zip. `_match_prefs` reads it at render
time for the reason `_folded_jobs` does — the chips paint right the first
time instead of flicking from the default once the script parses. Every
field is validated against what the screen offers, one field at a time: a
cookie survives a year and a condition that is not a condition, or a
threshold that is not one of the chips, would import under a setting nothing
on screen admits to. Every road that is not "the seller picked this chip"
leads to the safe answer — for the threshold that is Off, which cannot cost
anyone a card. The cohort tick is remembered too, which is the one
place the "the seller is asserting something about the pile in their hand"
rule gets stretched — it holds only because the chip is painted on above the
Start button before a file is chosen, so a carried-over assertion is one
they can see and untick.
* **Bump `_asset_version()` inputs when you add a static file.** It hashes the
files it knows about; one it does not know about ships behind a stale query
string, and the deploy then looks like it did nothing. `tests/test_assets.py`
now walks the templates for `?v=` and fails on any asset missing from the
hash — it found two that had been wrong since the beginning.
* **`finish` and `sub_type` are both on `inventory`, deliberately.** "Foil or
not" is answerable once for a whole batch on the import screen; "1st Edition
Holofoil or Holofoil" — $10,000 or $855 for the same Charizard — is not. The
coarse one drives bulk entry and the fallback guess; the precise one, once a
person sets it, is what pricing uses. Anything that drops `sub_type` on an
edit silently restores a guess the seller had already corrected.
* **A batch default may not answer for a card that has only one answer.**
More than a third of the catalogue has no foil printing and a fifth has
nothing else, so a batch imported as non-foil always contains cards that
exist only as foil. `inventory.resolve_finish` drops the default on those and
takes the match's side of the line; the queue and the auto-accept path both
go through it, and `import.html` repeats the rule in JavaScript for rows
re-pointed after load — the two have to agree or a correction moves a row
somewhere a reload puts back. It used to be flagged instead, chip and price
in a warning colour, which asked the seller to click away the one thing the
catalogue was certain about. The warning still fires, and now only where it
means something: a finish somebody set by hand that has no printing behind
it. Resolving is only for where there is exactly one honest answer — priced
both ways, or not priced at all, and the default stands.
The queue's finish chip is filled when the row is showing **what the import
asked for**, not when it is showing what it was seeded with. So a resolved
row reads as a plain white chip: it still stands out, because departing from
the seller's own answer is exactly what is worth a second look, and hiding
that because the departure was automatic would make the reason for it the
reason to say nothing about it. Three states, and they have to stay
distinguishable — filled dark is the default, white is a deviation that is
priced, warning colour is a finish with no printing behind it at all.
* **"Has a printing" and "has a price" are different facts.** `ingest` keeps a
printing whose `marketPrice` is null on purpose — it still names the
sub-type, and an unpriced card is worth recording — so about one price row
in seventy carries no money, and 2,700 cards in a real catalogue have a
printing on one side of the foil line that nothing will pay for.
`resolve_finish` used to take a bare list of names, which left every caller
to decide for itself which list that was. They split: `importing._accept`
passed the raw price map and the review queue passed one with the nulls
stripped for the browser. So a foil batch containing one of those cards
auto-accepted as a foil and was confirmed by hand as a non-foil — same
photograph, same match, same batch default, two rows, two prices, two
`Condition` columns in the upload file. Nothing raised, both answers looked
like answers, and it was 2,736 of 142,202 cards. It is the `Pricing`/floor
shape again: an argument a caller can get quietly wrong.
So they take the price map, not a list of names, and `priced_printings` is
the single place the two facts are told apart. `_price_map` — what the queue
ships to the browser as `data-prices`, and what `import.html` re-resolves a
corrected row against — is *derived* from it rather than filtering again
beside it, so the third copy of the rule agrees by construction.
A price also outranks the foil line in `priced_printing()`. An unpriced
Holofoil beside a priced Normal used to win on being the foil and then price
off `cards.market`, while `finish_unpriced` told the seller the row was
"priced off the other finish" — which was not what had happened. Only where
nothing at all is priced does the whole list come back, so a card with no
money behind it still names the printing it holds. That rule lives in the
lateral's `ORDER BY` and nowhere else; `tests/test_inventory_scale.py`
pins it, case by case, to the printing it must name.
* **A scan has three answers, and they are three columns.** `candidates` is
what the encoder saw in one image. `cohort_card_id` is what the rest of the
batch implies about it, when the seller ticked "batch is all one game/set" on
the import screen. `chosen_card_id` is a person saying they are holding the
card and it is this one. The queue prefers them in that order, backwards.
Folding the middle one into either neighbour was the first attempt and is
wrong both ways: reordering the candidates erases the ranking that is the
evidence for the move, and writing `chosen_card_id` tells the queue a human
decided something no human has looked at — which shows the "you picked this"
badge over nobody.
Two rules hold that pass together. **Nothing auto-accepts until the batch has
seen all of itself**, because accepting mid-batch and re-pointing afterwards
means writing inventory and taking it back, and a card that reached inventory
has been priced and exported against. And **a row the batch moved never
auto-accepts at all**, however well it scored: a guess about the neighbours
is a good reason to put a card in front of a person and a bad reason to skip
them. `rematch_scan` clears `cohort_card_id` for the same reason it keeps
`chosen_card_id` — the pick was made from a search result it is about to
replace.
The cohort is chosen by **presence, not first places**. Each scan gives each
game or set its own best score under it, once. Counting top matches is the
obvious rule and fails on the exact batch this is for: a Magic card reprinted
eight times scatters its first place across eight sets, so the set the seller
actually opened can win a fifth of them while appearing in nearly every
scan's candidate list. Counting every candidate instead fails the other way —
one scan of a common creature can put seven near-identical printings in one
core set and hand it the batch.
* **Price history cannot be backfilled**, except for Magic and only ninety
days of it. TCGCSV mirrors the current day only, so anything that stops
`sync-prices` running costs history permanently — treat a broken sync as data
loss rather than a stale cache. `foilstack enrich` recovers what MTGJSON
holds, which is one game, three months, and a market price with no
low/mid/high beside it. It is a repair, not a backup, and it may only ever
*add* a day: a day `sync-prices` recorded carries the full spread from the
authoritative source, and the writer excludes it before composing the insert
and then says `ON CONFLICT DO NOTHING` anyway. Being safe to run twice
matters more than being efficient — the operator reaching for it is usually
the one unsure whether the last attempt finished.
* **A TCGplayer upload is checked header-first, and its ids are SKU ids.** The
uploader compares the whole header row against the one its own export writes
and answers `Headers are not valid!` — no column named, no row examined. So
`tcgplayer.toml` carries all sixteen columns in order, four of them
deliberately empty, and a test pins the row against a real export. The
second half is worse: `TCGplayer Id` identifies a product *and* a condition
*and* a printing. 10th Edition Abundance is product 15023 and SKU 4519 near
mint, 4521 near mint foil. TCGCSV states it does not publish SKUs, so the
exporter writes that column blank — and the obvious shortcut of putting
`source_ref` there is the bad kind of wrong, because the id spaces overlap
and a row that resolves to some other card's SKU edits that listing instead
of failing. What makes a file uploadable is `foilstack.tcgplayer`, which
starts from the seller's own export and takes the ids from there.
* **`Add to Quantity` is a delta, and our stock is a total.** The round trip
wrote the whole stock line into that column, so every run after the first
added the cards again: three copies with one already listed went out as a
`3`, landed on their `1`, and offered four cards that cannot all be shipped
— an oversell, a cancellation and a mark against the seller. It is our stock
*minus* their own `Total Quantity`, which is also why that column is read at
all when nothing else in their file is, and why a value in it that will not
parse drops the row rather than counting as zero. The column takes a
negative, so the file is a sync and not an append: a copy sold here comes
down there on the next run, and the same file uploaded twice does nothing
the second time. `tcgplayer.toml` still writes the bare stock line, which is
safe only because it leaves `Total Quantity` blank and the file it produces
is not uploadable at all.
* **A placeholder is a promise.** The inventory search box offered `name, set,
sku` from the beginning and `narrow` never looked at a SKU — it reads the
name, game, set, conditions, finishes and collector number off the folded
line. A SKU is what a marketplace hands back when something sells, so
pasting one in to find the card is the search that box most owes a seller,
and what it answered was "nothing matches" — which reads as inventory that
is not there rather than as a box that was not looking. It reads the copies'
own SKUs now. Anything added to that haystack has to go in `narrow` and
nowhere else: `/listings` resolves a filter selection through the same
function, and a search that meant one thing on the screen and another at the
far end would list cards the seller never saw.
* **`cards.name` is the cleaned spelling and cannot be joined on.** TCGCSV
gives both: `cleanName`, which is what `name` holds and what reads and
searches properly, and `name`, which is what every TCGplayer CSV carries.
They differ exactly where it matters — `Ancestor's Chosen` is stored as
`Ancestors Chosen` — so a listing file matched on `name` loses about one card
in ten, and loses them silently. `source_name` holds the raw one. It is
nullable and only `ingest` writes it, on insert **and on update**, because a
field written only on insert is a field an existing catalogue never gets;
backfilling an install means re-running `ingest`, which is a catalogue pull
and not a re-encode.
Reverse-engineering `cleanName` instead was tried and abandoned at 98.5%
across four games. Every further rule — periods vanish, `&` becomes "and" —
is a guess, and a wrong guess is not an error but a card that quietly fails
to be listed.
* **`sync_state` is keyed per source *and* game.** It was keyed per source
once, and a successful Magic sync made every other catalogue believe its own
first run was already up to date.
* **Respect upstream's guidelines.** TCGCSV asks for a custom User-Agent, 100ms
between requests, and at most one full pull per daily rebuild — checked via
`last-updated.txt`. Exceeding it earns a throttle, then a ban. It is a free
service mirroring data we would otherwise have to buy.
* **A wrong TCGplayer category id is invisible.** It is a real category
returning a real catalogue of real cards — from a different game. Three of the
eleven ids here were wrong for months: `gundam` fetched hololive,
`dragonball` fetched Neopets Battledome, `finalfantasy` fetched Godzilla, and
the comment above them saying they had been read from the categories endpoint
rather than guessed is how they went unexamined. `scripts/check_categories.py`
checks every id against the name upstream gives it. It is deliberately not in
the suite — it needs the network, and a test that fails because upstream is
down cries wolf. Run it when you add a game.
* **The wheel has to contain more than Python.** Templates, static files and
migrations are none of them code, and any of them can quietly stop being
packaged; a release that imports fine and 500s on its first page is worse than
one that fails to build, so `release.yml` unzips the wheel and looks. The
static files are derived from the templates rather than listed, because a
hand-kept list only holds what somebody remembered — `app.js` was missing from
it for a whole release, and a missing script is the quiet kind of broken too:
every page renders and every button is dead.
* **The encoder's vectors are numpy.** `search.as_literal` forces `float()`
because NumPy 2 renders `np.float32(-0.02)` in list repr, which Postgres
cannot parse as a `halfvec`.
## Conventions
* Comments explain **why**, and are worth writing where the reason is not
recoverable from the code — a threshold, a trade-off, a bug that is now a
rule. Do not narrate what the next line does.
* Schema changes are Alembic revisions. `create_all` beside migrations is how a
database reaches a state no migration describes.
```bash
uv run alembic upgrade head # apply
uv run alembic revision --autogenerate -m "what" # write one
uv run alembic downgrade -1 # step back
```
* Nearest-neighbour search runs **in Postgres**, against an HNSW index, rather
than over a numpy array held in memory. That is what made accounts possible —
a per-user filter is a `WHERE` clause — and what stops a large catalogue
reading every vector on every scan.
* Numbers in the interface must be real or visibly marked. The analytics screen
computes what it can from inventory and labels the rest `demo data`; fees and
shipping are absent rather than estimated, because foilstack never sees them.
* Nothing posts to a marketplace. "Channels" are CSV formats, and the button
that records a sale says `Mark sold`, not `Push`.
* **Front end: no build step, no framework.** Behaviour shared by more than one
screen belongs in `static/app.js`, which exposes `$`, `$$`, `postJSON`,
`postForm` and `wireCardSearch` as plain globals. A page's own script goes in
a `scripts` block, which `base.html` emits *after* `app.js`; a script written
inline in a screen's markup instead runs before the helpers exist. Redeclaring
one of those names in a page script is a `SyntaxError`, not a shadow — two
top-level `const`s of one name in classic scripts collide.
* **A route module must not import `app.py`.** That is a cycle, and it is what
kept every route in one file. What the shell offers is in `web/chrome.py` —
the Jinja environment and its filters, the topbar figures, `_asset_version` —
and what a request needs is in `web/deps.py`. `foilstack.web` has no
`__init__.py`, so a constant two of those modules share is defined in the one
whose job it is rather than hoisted into the package.
* Jinja parses its tags inside HTML comments, so an `<!-- -->` comment that
names a block opens one, and the template dies at the next `endblock`. Use a
`{# ... #}` comment when the text needs to mention a tag.
## Deployment
Deploy with the commit in hand, so the running build can say what it is:
```bash
GIT_SHA="$(git rev-parse --short HEAD)$(git diff --quiet HEAD || echo -dirty)" \
docker compose up -d --build
curl -sS https://your-host/healthz # ok / foilstack 0.2.14 (a439f79)
```
Commit before you build. The `-dirty` suffix is there because a build from an
uncommitted tree labelled with a clean commit claims code it does not contain,
and that has already happened here: a deploy built before its own commit
reported the commit *before* the fix while running the fix. The version behaves
the same way — the bump hook runs at commit time, so a build made first carries
the previous number.
It catches staged and unstaged edits, not untracked files, which do still reach
the build context. Committing first is the habit; the suffix is the safety net
for when you forget.
**Do not name a service in the build.** `web` and `prices` are separate images
built from the same `Dockerfile.web`, so `docker compose build web` leaves the
price sidecar running whatever code it was last built with — and it fails
quietly, because a sidecar that rejects a game just logs and loops. That is how
a corrected category map sat in `web` for a day while `prices` still refused
`dragonballfusion` as an unknown game. `--build` with no service rebuilds both.
`docker compose up -d` runs `alembic upgrade head` before uvicorn, so a deploy
applies its own migrations. The `backup` service dumps on a schedule and writes
`BACKUP_FAILING` into the backup directory when a run produces nothing usable —
that file is the whole of the monitoring.
`FOILSTACK_MULTI_USER=true` requires a real `FOILSTACK_SECRET_KEY`; the app
refuses to start otherwise, because that key signs session cookies and the
shipped default is public.
A monitoring file has to be writable from wherever writes it. The `offsite`
container mounts the backup directory twice — `/backups` read-only, which is
what it replicates and must not be able to damage, and `/state` writable,
which is where `OFFSITE_FAILING` goes. It was written to `/backups` at first,
so a failing run could not raise the alarm and a good one could not clear a
stale marker: the monitoring was a no-op for as long as it existed.