Conversation
… 0.9.1; differential test `python/libfec_parser/fecfile.py` was 22 lines re-exporting `src/fecfile.rs`, which returned strings only, used the wrong header keys and was missing half of `fecfile`'s names. It is now pure Python over `libfec_parser.open()`. Values come from each row's raw fields plus a byte-for-byte copy of `fecfile`'s own `types.json` (vendored as `_fecfile_types.json`, Apache-2.0, attributed in a new `NOTICE` that `license-files` now ships), not from libfec's typed accessors: the two typed-column sets differ, and only the vendored table is exact. Floats are floats, dates are tz-aware US/Eastern `datetime`s via `zoneinfo` (no new dependency; `tzdata` only named in an error message), and `as_strings` is "skip the converters". New: `iter_file`, `iter_lines`, `FecItem`, `FecParserMissingMappingError`, `FecParserTypeWarning`. Options are validated instead of silently ignored, lowercase filter prefixes work, itemization groups come out in file order, and `print_example` prints through `print`, so `capsys` sees it. `from_http` stays bound to the native function; ticket 22 replaces it and deletes `src/fecfile.rs`. One native hook: `_native.parser._column_names(row_type, version)`, for `parse_line`/`parse_header`, which have a line and no filing to open. `tests/test_fecfile_differential.py` compares every API against the real package on all five fixtures, by value, type and key order, with three documented differences: our `*_TODO_DUP` cover keys, real's `F99_text` (both deferred parser items) and the trailing newline real's `iter_file` leaves in the last field of every row, which is a bug in the real package. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V8jXKZ3DKfu41iLZhH71GY
… delete src/fecfile.rs Adds the `[http]` extra (httpx2>=2), a lazy `_client()` helper, and `from_http`/`iter_http` over docquery.fec.gov: try the electronic URL, fall back to the paper URL on 404, `None` on 404/404 (matching real fecfile), `FilingUnavailableError` on any other non-200. `iter_http` streams the response through ticket 21's `_IterReader` adapter into `open()` without buffering the whole body, and closes the response and client whether the generator runs to completion or is closed early. Deletes the now-unused `src/fecfile.rs` Rust compat layer (last used by the old `from_http`) and its `#[pymodule] mod fecfile` registration in src/lib.rs; `_native` no longer has a `fecfile` submodule. No crate dependencies were removable — `csv` and `jiff` are still used by row.rs/parser.rs. Verified: httpx2 2.13.0 on PyPI is httpx-shaped (Client/stream/ iter_bytes/MockTransport all present); docquery.fec.gov accepts both a custom `libfec_parser/<ver>` User-Agent and httpx2's default, so no Mozilla/5.0 fallback is needed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V8jXKZ3DKfu41iLZhH71GY
Review follow-up to the httpx2 commit. `_stream` drove the response's `__enter__`/`__exit__` by hand and `from_http` read `response.content` whole before parsing. Both now go through `_fetch`, a generator context manager that owns the client and the streamed response, so cleanup is the `with` statement's job on every path and `from_http` feeds the parser chunk by chunk too. The User-Agent takes the package's `__version__` rather than a second metadata lookup. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V8jXKZ3DKfu41iLZhH71GY
…e 91 MB filing; bench scenarios Lockstep against real fecfile 0.9.1 over all 408,162 items of 1805248.fec: zero mismatches, zero warnings on either side, no new allowlist entries. The existing allowlist -- (a) _TODO_DUP, (c) real's trailing newline -- is imported from tests/test_fecfile_differential.py rather than restated; (b) F99_text does not arise (no [BEGINTEXT] block). The filing is strict UTF-8 with no field starting in a quote, so neither the deferred lossy-encoding item nor the fec-parser .quoting(false) bug shows up here. Measured on an M4 Pro, release build, CPython 3.13: fecfile.from_file ours 2.70 s / 1,106 MB PyPI 7.14 s / 1,403 MB fecfile.iter_file ours 2.53 s / 32.6 MB PyPI 6.87 s / 36.4 MB 2.6x faster on both paths, and iter_file peaks at roughly what bare open() costs -- the per-row dicts never accumulate. The RSS test asserts 64 MB, the same threshold open() gets, well inside Phase 3's 100 MB done-when. No performance work was needed, so fecfile.py is unchanged. The 91 MB filing types cleanly end to end, so the differential's warning assertion compares two empty lists; test_type_warnings_match_real covers the FecParserTypeWarning message format instead, on a hand-built bad value, and confirms the line numbers agree verbatim (both sides count from 1 and print line_num + 1). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V8jXKZ3DKfu41iLZhH71GY
…s/README) Rewrite the `fecfile` API section around the drop-in claim (verified by tests/test_fecfile_differential.py and the 408,162-item benchmark differential), print the 3-entry allowlist verbatim, cover iter_file/iter_http streaming, the [http] extra install spelling (verified against a local wheel), and the tzdata note. Drop the stale "not yet a drop-in"/"strings only"/Phase 3 banners. Re-execute the notebook with typed-value fecfile cells and a new iter_file example; no warnings. Document the differential test and new dev deps in tests/README.md. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V8jXKZ3DKfu41iLZhH71GY
…e raw str Review follow-up to the compat docs: `fecfile`'s `getTyped`, and ours, return `None` and emit `FecParserTypeWarning` when a float/date/integer column does not parse; only untyped columns come back as the raw string. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V8jXKZ3DKfu41iLZhH71GY
… fecfile A few cover mappings name a column twice -- an F3X's col_a_total_receipts and five more, an F2's candidate_state -- and fec-parser disambiguates the second copy as `*_TODO_DUP`. Real fecfile has the name twice and assigns in a loop, so the LAST copy's value survives at the FIRST copy's key position; this layer kept the first copy's value under the real name and the second under a name real has never heard of. Filings whose two totals differ read the wrong number. The same mapping table also invents `TODO_UNKNOWN_BLANK` where the F3L layout leaves a column nameless and real uses ''. Both are now translated to the spec's names in one documented place, before the dict is built and before the type converter is looked up, so ordinary dict assignment reproduces real's semantics in the typed and the as_strings path alike. That retires allowlist entry (a): `without_todo_dup` is gone from the fixture differential and from the slow 408,162-item lockstep differential, which now compare the cover page column for column with nothing dropped. New: a differential test over the whole mapping space -- a concrete row type for each of the 58 patterns in real's mappings.json x versions 8.5-8.0 and 7.0, 6.4, 6.1, 5.3, 5.0, 3.0 -- comparing names, order and duplicates against real's getMapping, plus a regression test with unequal duplicate values. Without the translation that sweep reports 114 divergent (form, version) pairs; with it, 0. Found by adversarial review (findings 1 and 4). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V8jXKZ3DKfu41iLZhH71GY
Real fecfile's parse_line returns None for a line with fewer than two fields,
and iter_lines drops it. This layer did neither: a delimiter-free schedule
name on its own line ("SA11AI") came back as a one-field phantom itemization,
and a whitespace-only line raised FecParserMissingMappingError, because the
native reader raises MissingMappingError for a blank row type.
_iter_items now pulls rows with next() rather than `for`, so a blank row type's
MissingMappingError can be skipped and the reader -- which stays usable after
raising -- carries on; any other missing mapping is re-raised as before. A row
with fewer than two fields is skipped on the way out. The prefix-filtered path
already matched real (the filter runs before the mapping lookup), and now the
unfiltered one does too.
Also widens allowlist entry (b) from "the \n real's iter_file leaves in the
last field" to line terminators generally: CRLF content handed to real's
loads/iter_lines keeps a \r in the last field of every row, since real splits
on '\n' alone. A CRLF filing read from *disk* is clean on both sides -- real
opens it in text mode -- and is now compared with no slack at all, by a new
test that builds the CRLF bytes from each committed fixture.
Found by adversarial review (finding 6a, 6b).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V8jXKZ3DKfu41iLZhH71GY
The README called this layer a drop-in for fecfile 0.9.1 with three differences, and left out that the whole-filing APIs read FEC 8.0-8.5 only: every older generation real parses -- v3/v5 comma-delimited, 6.x, 7.x, paper P3.x, and version strings real matches by regex prefix such as "8.50" -- raises FecParseError here. That scope now sits next to the drop-in claim, with the v1/v2-header and name_delim bullets folded into it, and a test pins it so the claim and the code cannot drift apart. Also documented, each with a test rather than an allowlist entry, since the differential excludes nothing for them: - zoneinfo and pytz agree from 1901-12-14 to 2038-03-14 and not outside it (measured, not assumed: pytz's transition table starts at the 32-bit time_t minimum and stops before the 2038 DST change), so the README's "same instant, same UTC offset" now carries that range and names the two windows; - the first record must be a cover fec-parser recognises, with a filer name; - a form type with surrounding whitespace is unmapped here; - filter_itemizations prefixes match the row type, not the raw line; - an element of a loads/iter_lines iterable containing \n is several records; - a blank line shifts a FecParserTypeWarning's line number by one (found here, not in the review: fec-parser numbers records, real numbers input lines). The allowlist section now matches the test file entry for entry -- F99_text and line terminators -- and "Known issues" still tells the truth about _TODO_DUP and TODO_UNKNOWN_BLANK for the native API, where cover_row still shows them. Module docstring, .pyi docstring, tests/README and the notebook's two drop-in sentences follow. Found by adversarial review (findings 2, 3, 5, 6c, 7, 8). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V8jXKZ3DKfu41iLZhH71GY
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
1 participant
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.
🤖 Claude-generated PR description
What this is
Phase 3 of the Python bindings:
libfec_parser.fecfilebecomes a pure-Python drop-in for PyPIfecfile0.9.1, for FEC format 8.0–8.5 filings, built on Phase 2's streamingopen(). The 368-line Rust compat layer (src/fecfile.rs) is deleted.Stacked on #21 (base
python-phase2), which is stacked on #20. It is its own PR because of the D2 escape hatch ("exact, or delete the module") — the hatch was not needed.Tickets
todos/python/21–24, one subagent per ticket, each diff reviewed and re-gated on the branch before landing, then an adversarial review by codex whose findings are the last three commits.231116ffecfile.pyin pure Python overopen(); raw fields + a vendored copy of fecfile'stypes.json;iter_file,iter_lines,FecItem,FecParserMissingMappingError,FecParserTypeWarning; options validated; differential test vs the real package3a5d75afrom_http/iter_httpover optionalhttpx2([http]extra, lazy import, 30 s timeout, UA),FilingUnavailableError; deletesrc/fecfile.rsc9b2ad7_fetch()context manager behind both HTTP functions;from_httpstreams too9e4c70f39c724b,a89462ae794e14df8dd49f0d159aDesign
_fecfile_types.json, vendored byte-for-byte, Apache-2.0, attributed in a newNOTICEshipped vialicense-files) — not from libfec's typed accessors, whose typed-column set differs from fecfile's. Exact by construction;as_stringsis "skip the converters"._native.parser._column_names(row_type, version), forparse_line/parse_header. Nofec-parserchanges. No Rust left in the compat layer.zoneinfoinstead ofpytz;httpx2only via[http];tzdataonly named in an error message (Windows).Done-when (roadmap
05:83-85)Differential test green with an allowlist short enough to print — it is two entries:
F99_text/[BEGINTEXT]…[ENDTEXT]bodies are not surfaced (deferred parser item N14).\nfrom itsiter_file,\rfrom CRLF content given toloads) — bugs in the real package, not reproduced.Coverage: every API on all five fixtures, by value, type and key order; all 408,162 items of the 91 MB filing in lockstep — zero mismatches, zero warnings either side; column names for 1,380
(form, version)pairs (115 forms × 12 versions, 8.5 → 3.0) — zero divergences. Negative controls were run for both big tests (an injected fault produced 408 mismatches; removing the name translation produced 114 divergent pairs), so the green runs aren't vacuous.iter_fileon the benchmark filing under 100 MB — 32.6 MB (test asserts < 64).fecfilefecfile.from_filefecfile.iter_file~2.6× faster on both paths in pure Python;
iter_filecosts about what bareopen()does. (Old Rust compatfrom_file: 2.1 s but 3.2 GB.)Adversarial review (codex) — what it found
Eight confirmed findings, all reproduced before acting. The important one:
_TODO_DUP; real fecfile has the name twice and its dict build is last-write-wins. We kept the first copy under the real name —col_a_total_receipts23.0 vs real's 44.0 when a filer's two totals differ. The original allowlist entry claimed the values were equal; that was only true of the fixtures. Fixed by translating native placeholder names to fecfile's in one documented place (_spec_name), which also removed that allowlist entry and made both differentials stricter. The follow-up name comparison found the duplication is wider than believed (F2'scandidate_statetoo) andTODO_UNKNOWN_BLANKvs''on F3L.parse_line/parse_headerdo work on 7.0–3.0.zoneinfoandpytzagree from 1901-12-14 through 2038-03-14 (measured day by day) and diverge outside it (pytzhas no DST after 2037 and no transitions before 1901). Documented with a table; both sides pinned by tests.\n): documented under "Malformed input", each pinned by a test.FecParserTypeWarningline numbers by one (the parser numbers records, real counts lines). Documented + pinned.Codex checked and found correct: HTTP cleanup on every exit path (early close, GC, parse error, HTTP error, unstarted generator), gzip bodies, 404 fallback, options validated before any request, type-table lookup order, quoted fields, Rust locking, no stale references to the deleted module.
Decisions to confirm
_TODO_DUPtranslation (e794e14) bends the "no binding-side workarounds for_TODO_DUP" rule. That rule was written believing the duplicate values matched; they don't, and a wrong total under the canonical key seemed worse than the rule — D2's "exact" won. It is one self-contained commit; reverting it restores the old behaviour and needs the allowlist entry back. The native API is untouched:cover_row.keys()still shows_TODO_DUP.from_httpreturnsNoneon 404/404, like real fecfile — overriding Q20's wording, which assumed real raises. Any other non-200 raisesFilingUnavailableError;iter_httpraises on any non-200 (as real).FecParseErrorfrom the whole-filing APIs. Sinceparse_line's mappings match real's back to 3.0, a pure-Python fallback reader for older versions is feasible as a follow-up; not done here.API changes vs Phase 2
float, tz-aware US/Easterndatetime,int,None) — were allstr. Header keys are fecfile's (soft_name/soft_ver; empty →''). No morefield_Nfallback: unmapped →FecParserMissingMappingError.iter_file,iter_http,iter_lines,FecItem,FecParserMissingMappingError,FilingUnavailableError,FecParserTypeWarning.itemizationsgroups are in file order (was HashMap order).print_exampleis capturable.from_httpneeds the[http]extra (ImportErrornaming it otherwise); network failures now propagate instead of returningNone.ValueError/TypeError), case-insensitive filter prefixes,loadstakes bytes-like / any iterable of lines,from_filetakesos.PathLike, errors derive fromFecError.libfec_parser._native.fecfileno longer exists.Gates at the tip (
f0d159a), run on the branchmake test338 passed / 8 opt-in skips ·make test-slow6 passed ·make stubsclean ·make notebook-checkclean (no error/stderr cells) ·cargo clippy -p libfec_parserand-p fec-cliwarning-free ·make test-network2 passed against docquery.fec.gov (atc9b2ad7; HTTP code unchanged since).CI:
fecfileandhttpx2added to the pytest install line so the differential and HTTP tests run rather than skip (fecfilealso in the non-blocking 3.14t job — it installs cleanly there).Known, not this PR's
fec-parsercsv.quoting(false)bug (see py: bindings Phase 2 — native API v2 (streaming open(), typed Row, read()) #21's "Known issue") — still open, doesn't occur in the 91 MB filing.Headerhas noname_delim(v3–5 HDR column) →''fromparse_header.fec-parser's error for an unrecognised cover is literallyError parsing cover record: asdf.plans/python/notes were updated locally and aren't in the diff.🤖 Generated with Claude Code
https://claude.ai/code/session_01V8jXKZ3DKfu41iLZhH71GY