edgar_read: list 13F holdings instead of returning None (#1337)

[wolfgang-aura]

Closes #1337.

edgar_read returned holdings: None for every 13F-HR, while the summary section of the same response reported Total Holdings: 211.

_extract_13f_section(obj, "holdings") in edgar/ai/mcp/tools/reader.py tested obj.holdings for truth. ThirteenF.holdings is a DataFrame, so that raised ValueError: The truth value of a DataFrame is ambiguous. The except Exception caught it at DEBUG and the function returned None. With the guard removed, the loop underneath would still have been wrong: iterating a DataFrame yields its column names, and getattr(h, 'name') on a string reads nothing useful. #1136 was the same pattern in _get_fund_holdings.

The section now checks holdings is not None and not holdings.empty and reads the first 30 rows through a new _holding_rows helper in edgar/ai/mcp/tools/base.py. As you asked, that helper is the row loop _get_fund_holdings has used since #1136, moved out of ownership.py unchanged: iterrows() with _cell_text/_cell_number over Issuer, Cusip, SharesPrnAmount and Value. _get_fund_holdings now calls it too, so there is one copy. A missing issuer prints as Unknown, and a missing share count or value is left off the line. The failure log moves from debug to warning.

Before, on the issue's 211-row fixture: None. After:

Top holdings (30 of 211):
  ISSUER 0 | 1,000 shares | $100,000
  ...

How this was tested

Fixtures: an in-memory DataFrame with the ThirteenF.holdings columns, as in the issue. No network.

• New TestExtract13FHoldings in tests/ai/test_mcp_intent_tools.py: the header, the first and 30th lines, the 30-row cap, NaN cells, and None for an empty or missing frame.
• tests/issues/regression/test_mcp_ownership_cell_coercion.py (the MCP edgar_ownership(analysis_type="fund_portfolio") always returns holdings: [] #1136 regression tests for _get_fund_holdings): 15 passed after the move.
• pytest tests/ai -m "not (slow or network or performance or batch)": 169 passed, 1 failed. The failure is test_install_skill_uses_symlinks: creating symlinks needs a privilege this Windows account lacks, and it fails the same way at the base commit.
• Mailman's touched-tests gate ran the 9 test files that the diff changes or that import edgar.ai.mcp.tools, with no marker filter: 148 passed, 0 failed.

Run on Windows 11 / Python 3.14.3; CI covers the rest of the matrix. Changelog fragment: changelog.d/1337.fixed.md.

AI disclosure

This change was drafted with AI assistance: Claude Opus 5.5 wrote the patch and a second Claude Opus 5.5 pass reviewed it, both running under Mailman. The second commit, which moves the row loop into _holding_rows after your comment, was written by Claude Opus 5.5 in a later session and did not get a second review pass. The test results above come from the harness running the commands itself, not from either agent's account of its own work. The account filing this pull request answers for every line of it.

[dgunning]

Thanks, this is exactly what I hoped for. The #1136 loop now lives in one place (_holding_rows), _get_fund_holdings calls it, and the section checks is not None and not .empty. I also ran it against Berkshire's latest 13F-HR (Q2 2026): it lists the holdings correctly, starting with Apple at 227,917,808 shares worth $65,950,296,923.

Two non-blocking notes for a follow-up:

1. _holding_rows skips rows with no usable cell, so the header can say "30 of 211" while listing fewer lines. Real filings don't hit this.
2. The holdings section counts securities (Berkshire: "29 of 29"), while the summary's "Total Holdings" counts information-table lines (89). Both are right, but they read as contradictory next to each other, so it's worth labelling them. This isn't something this PR introduced.

Merging.