Skip to content

Commit 459aca0

Browse files
committed
Add bulk-data support plan
Document the plan to finish offline Scryfall bulk-data support: - download and index default_cards (English) or all_cards (all languages) - keep online-only fallback and image fetching - centralize rate limiting - integrate settings and tests Images and missing non-English printings remain online.
1 parent f6606ef commit 459aca0

1 file changed

Lines changed: 320 additions & 0 deletions

File tree

‎BULK_PLAN.md‎

Lines changed: 320 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,320 @@
1+
# Plan: Proper Scryfall Bulk Data Support
2+
3+
## Context
4+
5+
The MTG Organizer app already has a partial bulk-data implementation:
6+
7+
- `mtgorg/constants.py` defines `USE_BULK_FILES = False` and `DEFAULT_BULK_FOLDER_LOCATION = ~/.mtgo/scryfall_bulk`.
8+
- `mtgorg/lib/scryfall.py` has `getBulkData()`, `downloadBulkData()`, and `searchCardsLocal()`.
9+
- `mtgorg/mtgorg_app.py` creates the bulk folder on startup.
10+
- `readme.md` mentions "WIP, added basic support for bulk files".
11+
12+
However, the current implementation has important gaps that make it unreliable and slow. The goal is to finish the support so the app can work with Scryfall's card data (records, search, IDs, reprints, set printings) offline from one bulk file, without hitting API rate limits. Only card images are fetched from Scryfall on demand and cached locally. The online-only mode must remain available for users who cannot or do not want to download bulk files. In every case, the app must enforce Scryfall's rate limits (2 req/s for search/named, 10 req/s for other endpoints).
13+
14+
---
15+
16+
## 1. Bulk Data Selection
17+
18+
Scryfall offers several bulk files:
19+
20+
| Type | Contents | Use case |
21+
|------|----------|----------|
22+
| `default_cards` | Every printed card Scryfall has ever had. | Covers all offline lookups: names, IDs, reprints, set chooser. |
23+
| `oracle_cards` | One record per unique card (Oracle identity). | Smaller, but lacks reprint data; not used by this app. |
24+
| `all_cards` | Same as default but includes digital-only/arena cards. | Larger than `default_cards` for no benefit here. |
25+
| `rulings`, `unique_artwork` | Specialized. | Not needed for core features. |
26+
27+
### Decision
28+
29+
Offer the user a single bulk-file mode choice that maps directly to language coverage:
30+
31+
- **English** — download `default_cards`. Smaller and faster. Sufficient if the user works with English printings only. If a non-English printing is requested while this mode is active, the app falls back to Scryfall online with rate limiting.
32+
- **All languages** — download `all_cards`. Larger, but contains every printed card in every supported language. Required for fully offline non-English printing selection.
33+
34+
The user must be able to switch between these two modes and re-download when switching. The online-only mode remains available for users who do not want to download a bulk file at all.
35+
36+
### Tasks
37+
38+
- [ ] Add a `BULK_DATA_TYPE` constant in `constants.py` (default `"default_cards"`).
39+
- [ ] Add a user-facing setting `bulk_language_mode` with values `"english"` (maps to `default_cards`) and `"all"` (maps to `all_cards`).
40+
- [ ] Update `downloadBulkData()` to respect the selected mode.
41+
- [ ] When `bulk_language_mode = english` and a lookup requests a non-English printing, fall back to online with rate limiting.
42+
- [ ] Validate the selected type against the available list from `https://api.scryfall.com/bulk-data` and fall back to `default_cards` if unavailable.
43+
44+
---
45+
46+
## 2. Download, Update and Storage
47+
48+
### Current state
49+
50+
`downloadBulkData()` downloads the file with `requests`, streams it to disk, and names it after the URL segment (e.g. `default-cards-20230813090443.json`). There is no automatic update, no integrity check, and no handling if the download fails mid-way.
51+
52+
### Proposed design
53+
54+
1. **Metadata file**
55+
- Store a small JSON (`bulk_info.json`) next to the bulk file with:
56+
- `type`
57+
- `download_uri`
58+
- `updated_at` (ISO timestamp from Scryfall)
59+
- `size` (bytes)
60+
- `etag` (if available)
61+
- `local_path`
62+
2. **Update policy**
63+
- On startup, if `USE_BULK_FILES` is enabled, query the Scryfall `/bulk-data` endpoint (this is one lightweight request).
64+
- Compare `updated_at` from Scryfall with the local metadata.
65+
- If the local file is older than the remote file, download the new one.
66+
- Keep the previous bulk file until the new one is fully downloaded and validated.
67+
3. **Atomic download**
68+
- Download to a temporary file (`default-cards-*.json.tmp`).
69+
- Rename to the final name only after the download completes and JSON parses.
70+
- Remove old bulk files once the new one is validated, keeping at most two files.
71+
4. **Progress feedback**
72+
- Expose download progress so the GUI can show a progress bar.
73+
- Use `requests` streaming with chunked progress reporting.
74+
5. **Rate-limit enforcement during download**
75+
- The `/bulk-data` endpoint is one lightweight request; apply the 10 req/s rate limiter.
76+
- The actual bulk download is a single large `GET`; avoid additional requests around it.
77+
78+
### Tasks
79+
80+
- [ ] Create `BulkDataManager` class in `mtgorg/lib/scryfall.py` (or new `mtgorg/lib/bulk.py`) responsible for metadata, download, update, and cleanup.
81+
- [ ] Add `getBulkInfo()` to fetch `/bulk-data` and parse it.
82+
- [ ] Add `needsUpdate()` using `updated_at` comparison.
83+
- [ ] Add `downloadBulkData(type, progress_callback=None)` with atomic `.tmp` + rename.
84+
- [ ] Add `cleanupOldBulkFiles(keep=2)`.
85+
- [ ] Ensure the download uses the registered User-Agent and respects rate limits.
86+
87+
---
88+
89+
## 3. In-Memory Indexing and Caching
90+
91+
### Current state
92+
93+
`getBulkData()` reads the entire JSON file every time it is called. `searchCardsLocal()` does a linear scan over all cards for every lookup. That is slow and repeated across calls.
94+
95+
### Proposed design
96+
97+
1. **Lazy load once per process**
98+
- Keep the parsed bulk list in a module-level variable.
99+
- Reload only when the file on disk changes (compare mtime/size).
100+
- Both `default_cards` and `all_cards` are supported; the active file is determined by the user's language-mode setting.
101+
2. **Build indexes for common lookups**
102+
- `name -> list of cards` (case-insensitive, exact name matching).
103+
- `id -> card`.
104+
- `mtgo_id -> card`.
105+
- `oracle_id -> list of printings`.
106+
- `set_code -> list of cards` (optional, for set chooser).
107+
3. **TinyDB / local cache**
108+
- The comment `TODO load into a tinyDB object ?` exists but probably not needed if we keep the parsed JSON in memory and build dict indexes.
109+
- For persistence across app restarts, the JSON file itself is the cache.
110+
4. **Memory consideration**
111+
- `default_cards` is ~150 MB JSON / ~500 MB in Python memory.
112+
- `all_cards` is larger; evaluate memory usage after indexing, and consider `ijson` or `orjson` if needed.
113+
114+
### Tasks
115+
116+
- [ ] Add `_bulk_data_cache` and `_bulk_indexes` module-level variables in `scryfall.py`.
117+
- [ ] Add `_loadBulkData()` that reads the file, builds indexes, and stores mtime.
118+
- [ ] Add `_isBulkDataStale()` to detect file changes.
119+
- [ ] Refactor `getBulkData()` to return the cached data when fresh.
120+
- [ ] Refactor `searchCardsLocal()` to use the name index.
121+
- [ ] Refactor `getCardById()`, `getCardByMTGOId()` to use id/mtgo indexes.
122+
123+
---
124+
125+
## 4. Search Semantics in Bulk Mode
126+
127+
### Current state
128+
129+
- Exact search filters by `name.lower()`. Good.
130+
- Non-exact search is a slow linear substring scan with no ranking.
131+
- The `set:` filter in `searchCardsOnline` is ignored in bulk mode.
132+
133+
### Proposed design
134+
135+
1. **Exact search**
136+
- Use the name index.
137+
- Apply optional filters (`set`, `lang`, `collector_number`) after lookup.
138+
- Sort by `released_at` descending to keep deterministic behavior consistent with online mode.
139+
2. **Non-exact search**
140+
- Use substring matching against the name index keys, ranked by length/position.
141+
- Optionally add fuzzy matching with `fuzzywuzzy` but limit to top N results to keep it fast.
142+
3. **Set filtering**
143+
- Support `searchDict["set"]` in both local and online modes.
144+
4. **Return consistent API**
145+
- `searchCards()` already dispatches to local or online. Keep the same return shape: list of `Card` dicts.
146+
147+
### Tasks
148+
149+
- [ ] Implement `_filterCards(cards, searchDict)` for shared filters (set, lang, etc.).
150+
- [ ] Update `searchCardsLocal()` to call the name index and shared filters.
151+
- [ ] Add substring/ranked non-exact search using the index.
152+
- [ ] Ensure sorting matches online mode (newest printing first for exact matches).
153+
154+
---
155+
156+
## 5. Offline-aware Lookup Helpers
157+
158+
The following helpers currently have `USE_BULK_FILES` branches but with limitations:
159+
160+
### `getCardReprints(cardId)`
161+
162+
- Online: walks `prints_search_uri` paginated endpoint.
163+
- Offline: use the `oracle_id` index to find all printings and return their set codes. With `default_cards` this is English-only; with `all_cards` it includes every language.
164+
165+
### `getCardReprintId(cardId, setCode, lang)`
166+
167+
- Online: calls `prints_search_uri` and `ByCodeNumber`.
168+
- Offline: look up all printings by `oracle_id`, filter by `setCode` and `lang`, and return the matching printing.
169+
- If the active mode is `default_cards` (English) and the requested `lang` is not English or is not found locally, fall back to online lookup with rate limiting.
170+
171+
### `getCardById(id)`
172+
173+
- Already supports bulk mode via linear scan. Make it use the id index.
174+
175+
### `getCardByMTGOId(mtgoId)`
176+
177+
- Already supports bulk mode via linear scan. Make it use the mtgo_id index.
178+
179+
### Image fetching
180+
181+
- Bulk files do **not** include card images.
182+
- Card images, thumbnails, and back-face images are always requested from Scryfall on demand (`image_uris`, `card_faces[].image_uris`).
183+
- Cache downloaded images to disk (the app likely already does this). Enforce the 10 req/s limit on image requests.
184+
185+
### Online-only fallback
186+
187+
- If `USE_BULK_FILES` is false, every online call must go through the rate-limited request path.
188+
- When `USE_BULK_FILES` is true, card data lookups are served from bulk; only image requests go online, plus any non-English printing request that cannot be satisfied by `default_cards`.
189+
- Centralize rate-limiting in `utils.py` so both `scrython` requests and direct `requests` calls share the same token bucket.
190+
191+
### Tasks
192+
193+
- [ ] Implement `getCardReprintsLocal(cardId)` using `oracle_id` index.
194+
- [ ] Implement `getCardReprintIdLocal(cardId, setCode, lang)` using `oracle_id` + set filter.
195+
- [ ] Refactor `getCardById()` and `getCardByMTGOId()` to use indexes.
196+
- [ ] Ensure missing cards return `None` instead of raising `IndexError`.
197+
198+
---
199+
200+
## 6. GUI / Settings Integration
201+
202+
### Current state
203+
204+
- `USE_BULK_FILES` is a hard-coded boolean.
205+
- `mtgorg/config.py` exists but is not inspected.
206+
207+
### Proposed design
208+
209+
1. **Config section `[SCRYFALL]`**
210+
- `use_bulk_files = true/false`
211+
- `bulk_auto_update = true/false`
212+
- `bulk_language_mode = english` or `all` (maps to `default_cards` or `all_cards`)
213+
2. **Settings dialog**
214+
- Add a "Scryfall" tab with:
215+
- Checkbox "Use offline bulk data (card data only; images still online)"
216+
- Dropdown "Bulk language coverage" ("English" → `default_cards`, "All languages" → `all_cards`)
217+
- Checkbox "Auto-update bulk data"
218+
- Button "Download / Update now" with progress bar
219+
- Label showing last update and file size
220+
- Checkbox "Prefer online-only mode" (disables bulk, equivalent to `use_bulk_files = false`)
221+
3. **Startup behavior**
222+
- If `use_bulk_files` is true and no bulk file exists, prompt the user to download (or auto-download if `bulk_auto_update` is true).
223+
- If a download is required, show a modal progress dialog.
224+
- If the user cancels or is offline, keep `use_bulk_files = false` for the current session (online-only fallback) and log a warning.
225+
226+
### Tasks
227+
228+
- [ ] Read/write `USE_BULK_FILES` and bulk settings from config in `config.py`.
229+
- [ ] Add a settings UI section in the existing settings/preferences dialog.
230+
- [ ] Wire the "Download now" button to `BulkDataManager.downloadBulkData()`.
231+
- [ ] Update `mtgorg/mtgorg_app.py` startup flow to handle missing bulk data.
232+
233+
---
234+
235+
## 7. Tests and Rate-limit Avoidance
236+
237+
### Goal
238+
239+
Make import/parser tests independent of live Scryfall so they are fast and stable.
240+
241+
### Proposed design
242+
243+
1. **Use bulk data in tests**
244+
- Add a `conftest.py` fixture that downloads the `default_cards` bulk file once (if not present) and sets `constants.USE_BULK_FILES = True` for the test session.
245+
- Optionally support an env var to switch the test fixture to `all_cards`.
246+
- Cache the bulk file in `~/.mtgo/scryfall_bulk` (shared with the app) or `tests/data` (override via env var).
247+
- Mark network-heavy tests with `@pytest.mark.slow` or `@pytest.mark.online` so they can be skipped in CI.
248+
2. **Mock path for pure parser tests**
249+
- For tests that only validate CSV/MTGA/EDHRec parsing, mock `scryfall.searchCards` to return deterministic IDs.
250+
- Keep a small mapping of card name → fake UUID in `tests/test_imports.py`.
251+
3. **Integration tests**
252+
- Keep a small number of tests that verify the real bulk lookup works, but run them only when explicitly requested.
253+
254+
### Tasks
255+
256+
- [ ] Add `tests/conftest.py` with a bulk-data fixture and `USE_BULK_FILES = True`.
257+
- [ ] Add a pytest option `--no-bulk` to skip bulk setup.
258+
- [ ] Add `@pytest.mark.online` to tests that hit the network.
259+
- [ ] Convert parser-only tests to mock `scryfall.searchCards`.
260+
- [ ] Re-run `pytest tests/test_imports.py -v` and confirm 12/12 pass without rate limits.
261+
262+
---
263+
264+
## 8. Error Handling and Edge Cases
265+
266+
- [ ] Handle a corrupt bulk file (fail validation, re-download).
267+
- [ ] Handle interrupted downloads (`.tmp` file remains; resume or restart cleanly).
268+
- [ ] Handle missing bulk data when `USE_BULK_FILES = True` (prompt user, then fall back to online-only mode with a warning).
269+
- [ ] Handle unicode card names in the bulk file (the existing comment `! FIXME handle reading of unicode chars : 暴`).
270+
- [ ] Handle cards without `released_at` (use `""` as before).
271+
- [ ] Prevent `IndexError` in `importexport.py` when no card is found; log and skip instead.
272+
273+
---
274+
275+
## 9. Phased Implementation Order
276+
277+
| Phase | Work | Benefit |
278+
|-------|------|---------|
279+
| 1 | Build `BulkDataManager`, atomic download, metadata. | Reliable offline data source. |
280+
| 2 | Add in-memory indexes and refactor `searchCardsLocal`, `getCardById`, `getCardByMTGOId`. | Fast offline lookups. |
281+
| 3 | Implement offline `getCardReprints` / `getCardReprintId` using the active bulk file, with online fallback when `default_cards` cannot satisfy a non-English request. | Set chooser and reprints work offline in the selected language coverage. |
282+
| 4 | Centralize rate limiting for all online paths. | No more 429 errors. |
283+
| 5 | Config/GUI integration (online-only vs. bulk). | User can choose mode. |
284+
| 6 | Test migration to bulk data + mocks. | Eliminate rate-limit flakiness in tests. |
285+
| 7 | Edge cases and cleanup. | Robust offline/online hybrid mode. |
286+
287+
---
288+
289+
## 10. Files to Touch
290+
291+
- `mtgorg/constants.py`
292+
- `mtgorg/lib/scryfall.py`
293+
- `mtgorg/lib/utils.py` (for download progress/rate limiting helpers)
294+
- `mtgorg/config.py`
295+
- `mtgorg/mtgorg_app.py`
296+
- `mtgorg/widgets/` (settings/preferences dialog)
297+
- `tests/conftest.py`
298+
- `tests/test_imports.py`
299+
300+
---
301+
302+
## Design Decisions
303+
304+
1. **Bulk file choices**
305+
- Offer two modes that map to Scryfall bulk types:
306+
- `english` → `default_cards` (English printings, smaller).
307+
- `all` → `all_cards` (all languages, larger).
308+
- The user can switch and re-download; online-only mode remains available.
309+
310+
2. **What is offline vs. online**
311+
- **Offline**: card records, names, IDs, search, oracle text, legalities, prices, reprints, and set-specific printings in the selected language coverage (from `default_cards` or `all_cards`).
312+
- **Online**: card images only, plus missing non-English printings when `english` mode is active, plus everything when `USE_BULK_FILES` is false.
313+
314+
3. **Keep online-only mode**
315+
- Always available; do not force bulk downloads.
316+
- When bulk is enabled but missing/corrupt, fall back to online-only for the session and prompt the user to download later.
317+
318+
4. **Rate limiting**
319+
- Enforce everywhere: `scrython` endpoints (already has `SlowRateLimiter`), direct HTTP via `utils.getUrlData`, image fetches, and the `/bulk-data` metadata request.
320+
- Use a shared token bucket so the combined request rate never exceeds Scryfall's limits.

0 commit comments

Comments
 (0)