Skip to content

fix(formd,form144): three typed fields that were silently wrong (#1192, #1193, #1195) - #1203

Open
dgunning wants to merge 2 commits into
mainfrom
fix/xu3z-data-object-fields
Open

fix(formd,form144): three typed fields that were silently wrong (#1192, #1193, #1195)#1203
dgunning wants to merge 2 commits into
mainfrom
fix/xu3z-data-object-fields

Conversation

@dgunning

@dgunning dgunning commented Aug 31, 2026

Copy link
Copy Markdown
Owner

Fixes #1192, fixes #1193, fixes #1195.

Three field-level defects reported by @synfonia-llc on 2026-08-30, each with a self-contained offline reproduction and the source line. All three are a few lines of parsing; all three produced output that looked complete and was wrong.

The defects

Form D sales-compensation ZIP codes (#1193). SalesCompensationRecipient.from_xml called child_text(address_tag, "30361") — the sample ZIP from the docstring 40 lines above the call, pasted in where the tag name belongs. It looked for a child element named <30361>, which never exists, so every recipient's address.zipcode was None. The identical read for related persons (formd.py:372) was always correct, which is why nothing downstream ever looked broken.

FormD.is_new (#1192). The value parsed from <isAmendment> was assigned straight to is_new, which asks the opposite question. A base Form D reported is_new=False and rendered as FORMD/A; a Form D/A reported is_new=True and rendered as FORMD. submission_type stayed correct throughout, so a single object exposed two contradictory filing identities.

Form144.nothing_to_report (#1195). The raw SEC Y/N flag was stored against a bool annotation. bool("N") is True, so if form.nothing_to_report: read a notice that does report a sale as reporting none — accession 0001958244-23-000454 has one prior-sale row sitting right underneath the flag. The rows were never lost; only the flag lied.

The fixes

Two are direct. The other two are worth a note:

  • The Form D heading now comes from <submissionType> rather than from the corrected boolean. With is_new fixed, "/A" if not self.is_new else "" would have been correct, but it would still render /A when the element is absent and is_new is None, and it would still be a derived answer capable of disagreeing with submission_type beside it. Taking the SEC's own field removes the class of bug rather than this instance of it.

  • nothing_to_report is Optional[bool], not bool. The house idiom elsewhere is child_text(...) == 'Y', which silently maps a missing flag to False. For this field False means "there is something to report" — an invented answer, and exactly what XBRLS.from_filings() silently swallows unexpected parsing exceptions and returns partial results #1174/07lk.10 are trying to eliminate. _yes_no_flag maps Y/N to True/False and leaves anything unrecognized, absent included, as None.

This changes nothing_to_report's type. Callers comparing it against the string "N" need updating. Called out in the CHANGELOG.

Two existing tests were pinning two of these bugs

  • tests/test_regd_notice_contract.py:73 asserted offering.is_new is False against D.APFund.xml, which files <submissionType>D</submissionType> with <isAmendment>false</isAmendment> — a base notice.
  • tests/test_144_notice_contract.py:77 asserted sample['nothing_to_report'] == "N".

Both were written by asserting what the parser returned, so the suite agreed with the defects for as long as they existed. Both updated here, with a comment naming the filed value.

That is why every expectation in the new regression file is read out of the raw XML with xml.etree, never from the objects under test. tests/issues/regression/test_xu3z_data_object_fields.py, 11 tests, including:

  • a control asserting related-person ZIPs still parse (that read was never broken and must not regress while its sibling is fixed);
  • a test asserting a base notice and an amendment disagree with each other, which catches a "fix" that inverts the constant and makes every filing report the same thing;
  • the _yes_no_flag truth table, including that an unanswered flag stays None.

Verification

  • New: 11 tests, all passing.
  • Related suites (formd, form_d, 144, offering, regd, exempt): 137 passed.
  • Full fast suite: 6568 passed, 9 skipped, 1 xfailed.
  • ruff check clean on all three changed files (the 8 pre-existing S110/B007 findings in these modules are untouched and unrelated).

Adjacent, deliberately not fixed

data/D.Shepards.xml files the literal string "None" as a sales-compensation ZIP, and the parser now faithfully returns it. This module already strips that artifact for recipientName and recipientCRDNumber (formd.py:129-132) but not for address fields. Extending the cleaning to addresses is a second behaviour change with its own blast radius, so it is recorded as a follow-up rather than folded in here.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LN2NaNcXEuv5YvcKFntcaZ

dgunning and others added 2 commits August 31, 2026 04:39
@dgunning @claude
fix(formd,form144): three typed fields that were silently wrong (#1192,
488f8a0 
#1193, #1195)

All three were reported by synfonia-llc on 2026-08-30, each with an offline
reproduction and the source line. All three had existing tests over them, and two
of those tests asserted the parser's own answer back at itself, so the suite
agreed with the defect for as long as it existed.

Form D ZIP codes (#1193). `child_text(address_tag, "30361")` looked for a child
element named <30361> -- the sample ZIP from the docstring 40 lines above, pasted
in where the tag name belongs -- so every sales-compensation recipient's zipcode
was None. The identical read for related persons was always correct, which is why
nothing downstream looked broken.

FormD.is_new (#1192). The value parsed from <isAmendment> was assigned to is_new,
which asks the opposite question, so a base D reported is_new=False and rendered
as "FORMD/A" while a D/A rendered as "FORMD". The local is renamed for what it
holds and inverted at the point of use, preserving None for an absent element
rather than turning it into a claim. to_context() now takes the heading from
submissionType, the SEC's own field, so the heading cannot contradict
submission_type on the same object.

Form144.nothing_to_report (#1195). The raw 'Y'/'N' flag was stored against a bool
annotation, and bool("N") is True, so `if form.nothing_to_report:` read a notice
reporting a sale as reporting none. Now parsed through a _yes_no_flag helper:
'Y'/'N' become True/False and anything unrecognized -- including an absent
element -- stays None rather than defaulting, so "did not answer" and "answered
no" stay distinguishable. This changes the field's type; callers comparing it
against the string need updating, and the CHANGELOG says so.

The regression tests read every expectation out of the raw XML with the standard
library. Given that two existing tests pinned two of these bugs, an expectation
produced by the machinery under test is worth nothing here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LN2NaNcXEuv5YvcKFntcaZ
@dgunning @claude
test(xu3z): read the ground truth with no XML parser at all
b25c499 
CodeFactor flagged the stdlib `xml.etree` parse these tests used to read their
expectations (the ruff `# noqa: S314` silenced ruff, not CodeFactor). `defusedxml`
imports in this environment but is not declared in pyproject.toml, so reaching for
it would lean on an undeclared transitive dependency.

The expectations are now sliced straight out of the fixture text with two small
regex helpers. That is not a workaround: the point of the file is that the
expectation must not be produced by the machinery under test, and using no parser
at all is the strongest available form of that. The fixtures are small, checked in
and static, and the values wanted are leaf text.

Same 11 tests, same assertions, unchanged behaviour under test. This also drops
the `edgar.form144` import in favour of the canonical `edgar.ownership.form144`,
so the file no longer emits a DeprecationWarning of its own.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LN2NaNcXEuv5YvcKFntcaZ
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

1 participant

@dgunning