Add Schedule 14D-9 data object for tender offer recommendations - #940
SurafelAnshebo wants to merge 4 commits into
Conversation
|
Sorry this sat — thanks for your patience, and for marking it draft and asking about the shape before growing it. Reviewed properly now. The structure is right.
Your 11 tests pass, and the Lisata ground truth is genuine. One real defect, in
|
Minor, not patch: edgar.settings is a new public module. Nothing is removed or renamed -- edgar.core re-exports the same objects -- and dropping the legacy parser fallback from TenK/TenQ/TwentyF.items changed the item list on zero of 115 corpus filings. Seventeen entries fold into ## [5.52.0] - 2026-08-22. The headline is one story: the modern parser now answers every item lookup that used to need the deprecated ChunkedDocument, back to 1996 filings, which is the gate for removing edgar.files in 6.0. Alongside it two data-correctness fixes that each returned a plausible wrong answer rather than an error -- get_operating_cash_flow() returning None for Apple (#1083), and every fund series and class name degrading to its bare identifier after SEC moved a dataset page (#1077). Four merged PRs had no changelog entry and were added while scoping: #1077, #1079, #1080, #959. The section was then trimmed from 185 words per entry to 115, back inside the range the rest of the file uses. Schedule 14D-9 (#940) is deliberately not in this release: still a draft, no review, no CI run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…poisoning Addresses review on PR dgunning#940: - classify_recommendation() now classifies against the recommendation statement only (cut at the "Background of the..." / "Reasons for the Recommendation" heading), not the full Item 4 section, which can run to 100,000+ characters of narrative that routinely restates an earlier, superseded board position - fixes a second real bug found while gathering ground truth: a recommendation split across an HTML line-wrap defeated the regex entirely (whitespace is now fully normalized before matching) - fixes a third gap: a real rejection phrased "recommends...not accept the Offer" (no "reject" or "tender") was previously missed - adds recommendation_text_truncated so callers can tell a complete short statement from a clipped long one - adds rich console rendering, matching Schedule13D's __rich__ pattern - adds 2 more real, hand-verified ground-truth filings (Moody National REIT II: one genuine reject, one genuine neutral), for 3 total Full fast suite: 2738 passed, 0 failed.
dgunning
left a comment
There was a problem hiding this comment.
Apologies for the long silence on this — you asked for a read on the object shape before growing it further and then waited five weeks. That's on us, not you.
Short version: the design is right, the code is better commented than most of what's already in the tree, and I'd take it. There are three CI gates it trips and one property whose behaviour doesn't match its docstring. None is structural.
What I checked first: does it still apply?
It's branched from 6ac6f4e5 (5.44.0) and main is now past 5.58, so I merged main into it before judging anything. It merges cleanly — one auto-merge in edgar/__init__.py, no conflicts — and your 20 tests pass unchanged against current main. Nothing has rotted.
The design
recommendation as a nullable classification alongside the raw text is the right call, and returning None on hedged language rather than guessing is the part I'd have pushed for if you hadn't done it. The comments carry their evidence — accessions for Woodbridge and Genco, the measured 722–1067 char statement range, the explicit note that reject/neutral are checked before accept because accept phrasing is the common case. That's the house style and it's rarer than it should be.
Registration is in the right place: both the form_map entry and the obj() if/elif chain in edgar/__init__.py. That's the dispatch people reliably get wrong (it is not the table in _filings.py). matches_form handles the amendment correctly — I checked SC 14D9/A matches and SC 14D1 / SC 14D9-X don't. Updating forms.yaml in the same PR covers the solvability half of the Definition of Done, which contributors almost never do unprompted.
One thing I want to be explicit about because it looks like a defect and isn't: assert filing.form in SC_14D9_FORMS is the established pattern here — ten_k.py, ten_q.py, current_report.py, schedule13.py, sixk.py, twenty_f.py, forty_f.py, form144.py and proxy/core.py all do exactly this. Don't change it; you'd be the odd one out.
Blocking: three CI gates
These all fail on the merged branch and all three are caused by the PR.
1. test_obj_still_returns_none_for_a_form_we_do_not_model uses form = "SC 14D9" as its example of a form we don't model. You now model it, so obj() routes into Schedule14D9.from_filing and dies on the stub:
AttributeError: '_Unmodelled' object has no attribute 'html'
edgar/tender_offers/schedule14d9.py:237
That test needs a different unmodelled form. It's a required companion change, not a sign anything is wrong with your code.
2. The raw-ValueError ratchet (tests/issues/regression/test_raw_valueerror_ratchet.py) goes 133 → 135, one for each raise ValueError in schedule14d9.py.
The ratchet's message suggests ValidationError, but I don't think that's the right one here — the caller's input wasn't invalid, the document failed to parse. DataObjectError in edgar/exceptions.py:497 is purpose-built for this: "filing.obj() could not build a data object for a form it supports", and it takes form= and accession_no=, which is exactly what both of your raises carry.
Worth knowing before you switch: DataObjectError is a ParsingError, not a ValueError, so your two pytest.raises(ValueError, match=...) assertions will need updating too. ValidationError would keep them working since it IS-A ValueError — but it would be the wrong name for the condition, and I'd rather have the right exception and an updated test.
3. No changelog fragment. Add changelog.d/922.added.md — a bold headline plus a sentence or two with one measured value, 500 chars max. Never touch CHANGELOG.md directly; concurrent PRs conflict on it.
One real finding: recommendation_text_truncated is inverted
The docstring promises "the recommendation statement itself may run longer than the calibrated window. Check this before treating recommendation_text as a complete statement." The implementation answers a different question — whether Item 4 as a whole exceeds the cap — and Item 4 is essentially always huge, so the flag fires when it shouldn't.
Measured on your own fixtures:
| fixture | Item 4 | window | cut by | reports truncated |
|---|---|---|---|---|
| lisata_therapeutics | 125,815 | 1,066 | marker | True |
| moody_national_reit_ii_neutral | 1,741 | 845 | marker | False |
| moody_national_reit_ii_reject | 1,588 | 692 | marker | False |
Lisata's window was cut at the Background of the heading, so the statement is complete — and the flag tells the caller to distrust it. Since your own docstring says Item 4 routinely runs past 100,000 characters, this will read True for nearly every real filing while being False only for unusually short ones, which is backwards from useful.
The question the docstring asks is "did the cap make the cut, rather than a heading?":
normalized = re.sub(r"\s+", " ", self.item4_text).strip()
lowered = normalized.lower()
hits = [m.start() for m in (re.search(p, lowered) for p in _SECTION_BOUNDARY_MARKERS) if m]
boundary = min(hits) if hits else len(normalized)
return boundary > _RECOMMENDATION_WINDOW_CHARSI tested it: all three fixtures report False (all cut cleanly at a heading), a synthetic 4,852-char Item 4 with no boundary heading reports True, and a short one with no heading reports False. test_recommendation_text_truncated_flag asserts is True for Lisata, so it locks the current behaviour in and would need to flip with the fix.
Not your problem
tests/xbrl/test_xbrl_unit_pointintime.py::TestPerformanceOverhead::test_reasonable_overhead_when_enabled also failed in my full -m fast run (7553 passed, 3 failed). It passes in isolation on both your branch and main, and your PR touches no XBRL files — it's load-sensitive. Ignore it.
Summary
Fix the two gates, add the changelog fragment, and decide on the truncation flag, and I'll take this. If you'd rather ship the object without recommendation_text_truncated and add it later once the semantics are settled, that's fine too — the rest stands on its own.
Thanks for the care you put into this, and sorry again for the wait.
|
Follow-up: I ran the parser against real filings rather than only the committed fixtures, and found something worth having before you touch anything else. It's a one-flag change and it moves the numbers a lot.
|
| before | after | |
|---|---|---|
| classified (accept/neutral/reject) | 13 (52%) | 20 (80%) |
parsed, recommendation is None |
3 | 4 |
raised ValueError |
9 (36%) | 1 (4%) |
Your 20 tests still pass unchanged. The table-of-contents entries do start matching too, but extract_item_section's longest-span rule discards them exactly as designed — nice that it already handled the case.
Worth adding a fixture with an uppercase heading so this can't regress; none of the three committed ones use that spelling, which is why the suite was green while a third of real filings failed.
The design question you asked about, with numbers
You offered to adjust the object shape, and here is the place it matters. Of the 229 SC 14D9 filings in 2025, 174 are SC 14D9/A. Sampling 20 amendments with the flag applied, 15 still raise: an amendment usually restates only the items it changes, so Item 4 genuinely isn't there.
Since both forms are registered, filing.obj() raises for the dominant filing type of the form. That's worse than it sounds, because obj() is the generic entry point people reach for without knowing the form. Two ways out:
- Make Item 4 optional — construct the object with
item4_text=Noneandrecommendation=None, and letis_amendmentexplain why. Keepsobj()total, at the cost of an object that sometimes carries nothing. - Register only
SC 14D9and leave amendments unmodelled.
I lean towards (1): an amendment still carries the subject company, date and amendment flag, and raising for a structurally normal document reads as a library bug to the caller. But it's your design and I'd rather you picked.
Where that leaves the PR
With the case fix, this is four small items from mergeable — the two CI gates and the changelog fragment from my earlier review, plus the truncation-flag semantics. Happy for you to take them; they're yours and you clearly know this form better than I do now.
If you'd rather not, say so or just go quiet for a week and we'll finish it on your branch with your authorship intact. No obligation either way after the wait we put you through.
The branch was cut at 5.44.0 and main is past 5.58. CI tests the pull-request merge commit, so the gates that fail on this PR (the raw-ValueError ratchet and test_exceptions_none_flips) live only on main — merging makes the local tree match what CI actually runs.
…I gates Maintainer changes on top of SurafelAnshebo's Schedule14D9 work, pushed to this branch rather than left as review comments because CI was red and the fixes are mechanical. The parser, the classification design and the ground truth are unchanged and are theirs. ITEM HEADINGS ARE MATCHED CASE-INSENSITIVELY. `_find_real_item_starts` compiled `Item\s*N\s*[.\-—]` without re.IGNORECASE, and a large share of filers set the heading in caps. In those documents the only remaining "Item 4" was a quoted cross-reference, which the quote filter then correctly discarded, so the whole filing raised. Over 25 SC 14D9 originals filed in 2025: 9 failed to parse before, 1 after, and classification went from 13 to 20. A HEADING AT OFFSET 0 IS NO LONGER READ AS QUOTED. `prev_char` is "" for a match at position 0, and `"" in _QUOTE_CHARS` is True because the empty string is a substring of every string, so that heading was skipped. Not reachable in a whole filed document, where Item 4 never starts at byte 0, but reachable from any already-sliced text. BOTH RAISES BECOME DataObjectError. The raw-ValueError ratchet (tests/issues/regression/test_raw_valueerror_ratchet.py) went 133 -> 135. The ratchet suggests ValidationError, but the caller's input was not invalid — the document failed to parse — and DataObjectError is the canonical exception for "filing.obj() could not build a data object for a form it supports", carrying form= and accession_no=. It is a ParsingError rather than a ValueError, so the two pytest.raises assertions move with it. test_obj_still_returns_none_for_a_form_we_do_not_model USES A DIFFERENT FORM. It used "SC 14D9" as its example of an unmodelled form; this PR models it, so obj() routed into Schedule14D9.from_filing and died on the stub. Now "SC 14D1", the withdrawn tender-offer form, with a comment saying why the choice matters. recommendation_text_truncated ANSWERS ITS OWN DOCSTRING. It promised "the statement may run longer than the window" and measured whether Item 4 exceeded the cap. Item 4 carries the whole background narrative and routinely passes 100,000 characters, so the flag read True for nearly every real filing — including Lisata, whose window ends cleanly at a heading after 1,066 chars. It now reports whether the cap, rather than a heading, made the cut. Also merges main (the branch was cut at 5.44.0) and adds the changelog fragment. Full fast suite: 7566 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@SurafelAnshebo I've pushed the fixes to this branch rather than leaving you another list — CI had gone red and all of it was mechanical. Your parser, the classification design and the ground truth are untouched and remain yours. Revert any of it if you disagree; none of it is precious. What CI was failing onBoth gates live only on
The one that actually mattered
Over 25 SC 14D9 originals filed in 2025:
Your A latent one your tests couldn't reachWriting a case-insensitivity test surfaced this: prev_char = text[start - 1] if start > 0 else ""
if prev_char in _QUOTE_CHARS: # "" is a substring of every string → TrueA heading at offset 0 was discarded as quoted. Not reachable in a whole filed document, where Item 4 never starts at byte 0, but reachable from any already-sliced text — and it's why my synthetic fixture failed on the title-case spelling too, which is what put me onto it. Guarded with The truncation flag
Plus the changelog fragment. Full fast suite: 7566 passed. Still yours to decideThe design question from your original post, with numbers behind it now: of the 229 SC 14D9 filings in 2025, 174 are Thanks again for the work, and for your patience with how long we took to look at it. |
Closes #922
Implements
Schedule14D9per the boundary agreed in #922 SC 14D-9 only, not the tender-offer side (SC TO-T/SC TO-I).Approach
lxml.htmlon the filing's primary document (SC 14D-9 has no structured XML cover page like Schedule 13D/G, so this parses rendered text directly).recommendationis a nullable classification ("accept"/"reject"/"neutral"/None) alongside the raw Item 4 text returnsNoneon ambiguous/hedged board language rather than guessing, per the design note in Add data objects for tender offer filings (SC TO-T, SC TO-I, SC 14D9) #922.SC 14D9/SC 14D9/Ainform_mapandobj()dispatch inedgar/__init__.py, plus the skill YAML (edgar/ai/skills/forms.yaml).Ground truth
Lisata Therapeutics, Inc. (CIK 320017), SC 14D-9 filed 2026-06-10, accession
0001140361-26-024737. Hand-verified against the primary document and the board unanimously recommended shareholders accept the offer, and.recommendation == "accept"matches.Testing
tests/test_tender_offers.py, 11/11 passingruff check/ruff formatclean on new filesMarking as draft per @dgunning's note in #922 and happy to adjust the object shape/API before it grows further.
Thanks!