Skip to content

feat(finance/uk_companies_house_handler): Document API flow v2d (#421) - #434

Merged
rosspeili merged 3 commits into
ARPAHLS:mainfrom
2409324124:feat/issue-421-document-api
Oct 15, 2026
Merged

rosspeili merged 3 commits into
ARPAHLS:mainfrom
2409324124:feat/issue-421-document-api

Conversation

@2409324124

@2409324124 2409324124 commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Description

Implements Phase v2d of finance/uk_companies_house_handler (parent #220): the Companies House Document API discover -> pick -> download flow. Bumps the skill to 1.4.0.

Fixes #421

Summary of changes

  • New actions (skill.py, manifest.yaml):
    • list_downloadable_documents: filters filing history to items with links.document_metadata and returns a pick list (transaction_id, date, description, category, metadata_url, downloadable). Supports category, limit (max 25), latest_only, and latest_per_category.
    • get_document_metadata: returns mime_types, content_length, total_pages, and created_at from the Document API.
    • fetch_document: streams bytes in chunks and returns bounded content_base64. max_bytes defaults to 5 MB with a hard ceiling of 10 MB. Also supports include_content: false and content_type.
  • Document API request path:
    • Basic auth is sent to document-api.company-information.service.gov.uk.
    • /content is requested with allow_redirects=False. The Location redirect is then followed without Authorization.
    • The declared content_length is checked before download, and a streaming byte counter aborts the download with document_too_large if it goes over.
  • Context: selected_transaction_id and document_candidates are carried forward. If a transaction_id is not in the candidates, the skill resolves it through GET /company/{cn}/filing-history/{tx}. Filing transaction ids are not Document API ids.
  • run_pipeline: list_downloadable_documents is supported as a step, including <from_resolve> substitution.
  • Constitution: rules 8-10 are new (no disk persistence, strip auth on redirect, 429 handling without sleep).
  • Docs and examples:
    • Updated instructions.md (Document Mode playbook), docs/skills/finance/uk_companies_house_handler.md (schemas and examples), the category hub, the docs/skills/README.md row, card.json, and data/api_index.json.
    • Added examples/uk_companies_house_document_flow_demo.py, indexed in examples/README.md and registered in tests/test_examples_smoke.py.
    • Added a CHANGELOG.md entry under [Unreleased].

Maintainer decisions followed

  1. Authorization is dropped when following redirects to external storage, because pre-signed S3 URLs reject extra auth.
  2. On HTTP 429, the skill returns a structured rate_limited envelope with retry_after and the Retry-After value in agent_hint. There is no sleep and no client-side throttling.
  3. The skill stays stateless. Document bytes are returned in the response envelope only and are never written to disk.

Mocked test coverage (no network in CI)

New tests in skills/finance/uk_companies_house_handler/test_skill.py:

  • Listing: the pick list, latest_only, the category filter, and needs_input when company_number is missing.
  • Metadata: metadata success; 404 -> not_found; 410 -> api_error; 429 -> rate_limited with retry_after.
  • Fetch:
    • Redirect handling, asserting no auth on the storage request.
    • Oversize from declared metadata and oversize during streaming.
    • include_content: false.
    • 404, 410, and 429 on /content.
    • Missing target and an invalid (non-Document-API) metadata_url.
  • Pipeline and context: the run_pipeline document flow; candidates surviving the pipeline context merge; unknown transaction_id resolved through the filing-history item; unknown transaction_id without a company number.
  • Manifest: version 1.4.0 and the new action enum values.

Live e2e (local only, against the real API, company 00445790 Tesco PLC)

Run from a local script outside the repo with a live REST key. The key was not committed or logged.

  • list_downloadable_documents (accounts): ready, 5 documents, HTTP 200/200.
  • get_document_metadata (confirmation statement, 2026-07-01): HTTP 200, application/pdf, content_length 82,803, 3 pages.
  • fetch_document:
    • /content with auth and allow_redirects=False -> 302 to s3.eu-west-2.amazonaws.com.
    • The S3 request was sent without Authorization -> 200 application/pdf.
    • The base64 decoded to 82,803 bytes starting with %PDF-1.4, filename 00445790_confirmation-statement_2026-07-01.pdf.
  • document_too_large from metadata alone (no /content request):
    • The group accounts PDF (2026-07-25) is 14,316,485 bytes, over the 10 MB hard ceiling.
    • The 82,803-byte PDF with max_bytes=1024 was also rejected.
  • Bogus document id: HTTP 404 -> not_found for both the metadata and fetch actions.
  • run_pipeline (resolve_company "Tesco PLC" -> needs_input, resume with 00445790 -> list_downloadable_documents with latest_only): ready. A following fetch_document with only transaction_id plus context downloaded the same 82,803-byte PDF.
  • Bug found live and fixed (second commit):
    • document_candidates from a pipeline step were dropped by the outer context merge.
    • The fallback then treated a filing transaction_id as a Document API id, which returned 404.
    • The fix keeps the candidates and resolves the id through the filing-history item. Regression tests were added.
    • The document_too_large and not_found messages were also clarified.

Not covered live: 410, 429, and the streaming byte-counter abort. The live API always declared content_length, so oversize was caught at the metadata step. All three are covered by the mocked tests only.

Type of Change

  • New Skill
  • Skill Upgrade: changes to an existing skill under skills/
  • Bug Fix
  • Documentation
  • Framework Feature
  • CLI
  • Examples: examples/uk_companies_house_document_flow_demo.py, examples/README.md
  • Packaging
  • RFC / meta

Checklist (all PRs)

  • Linked GitHub issue (Fixes #421)
  • Scope matches the issue, with no unrelated refactors
  • Contribution complies with Legal notice and code ownership (original work, MIT license grant, no malicious logic)
  • python -m black --check ., flake8, and ruff check . pass locally
  • pytest skills/ (539 passed, 1 skipped) and pytest tests/ (567 passed, 8 skipped) pass locally
  • CHANGELOG.md updated under [Unreleased]
  • examples/README.md updated for the new runnable script
  • Ran pytest tests/test_registry_docs.py (included in pytest tests/)
  • Ran pytest tests/test_skill_docs.py (included in pytest tests/)

New or updated skill

Bundle and metadata

  • Skill at skills/finance/uk_companies_house_handler/ (existing bundle)
  • manifest.yaml: version 1.4.0, new actions and parameters, constitution rules 8-10
  • requirements unchanged (requests); env_vars unchanged (the same COMPANIES_HOUSE_API_KEY is used for the Document API)

Effect, Directive, Assurance

  • Deterministic skill.py
  • instructions.md updated with the Document Mode playbook
  • card.json description updated; issuer unchanged
  • test_skill.py covers the new actions and error paths
  • SkillLoader.load_skill("finance/uk_companies_house_handler") succeeds

Documentation and catalog

  • Catalog page, finance hub row, and docs/skills/README.md row updated (existing skill, so no sitemap change)
  • Provider Usage Examples unchanged; the existing five-provider snippets still apply

Constitution and safety (skills only)

  • Read-only public registry data. Nothing is submitted or modified.
  • Document bytes are returned in-memory only; no disk persistence.
  • Authorization is never forwarded to external storage hosts.
  • metadata_url must be on a Companies House Document API host; other hosts are rejected with invalid_metadata_url.
  • The payload is bounded by max_bytes, with a 10 MB hard ceiling, enforced both from declared metadata and during streaming.
  • Rate limits are surfaced to the host through Retry-After; the skill does not sleep.

Related Issues

Fixes #421
Parent: #220

Upgrade uk_companies_house_handler to v1.4.0 with
list_downloadable_documents, get_document_metadata, and fetch_document.
Document downloads stream as bounded base64 without disk writes; auth is
dropped on redirects to external storage; 429 returns Retry-After in
agent_hint without client-side sleep. Includes mocked tests, demo,
docs, and CHANGELOG entry.
…ve filing transaction ids

Live testing against the Companies House Document API showed that
document_candidates from a pipeline list step were dropped by the outer
context merge, and the fallback treated a filing transaction_id as a
Document API id (404). Resolve the metadata link via the filing-history
item instead, and clarify document_too_large and not_found messages.
@2409324124
2409324124 marked this pull request as ready for review October 15, 2026 06:06

@rosspeili rosspeili left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is some of the most outstanding work I've seen on the framework @2409324124! I would even argue a mastercalss in open-source contribution, considering most back and forth reviews that stale PRs come from contributors not reading the basic docs.

Everything is spot on, and the fact you shared your rational with the bug you found during live testing just speaks of your character as a problem solver.

Merging as is, thanks a lot for this, and welcome to Skillware <3

Comment thread examples/uk_companies_house_document_flow_demo.py Fixed
Comment thread skills/finance/uk_companies_house_handler/test_skill.py Fixed
Comment thread skills/finance/uk_companies_house_handler/test_skill.py Fixed
Comment thread skills/finance/uk_companies_house_handler/test_skill.py Fixed
@rosspeili

Copy link
Copy Markdown
Contributor

Hey @2409324124, the codeql bot flagged 4 lines in test/demo code for Incomplete URL substring sanitization:

  • examples/uk_companies_house_document_flow_demo.py (line 110)
  • skills/finance/uk_companies_house_handler/test_skill.py (lines 2407, 2500, 2736)

CodeQL scans all repository files by default and flags if "domain" in url: because it mistakes the mock router for an unanchored production URL sanitizer (even though production skill.py is already using strict urlparse hostname checking).

I would push a patch that can be anchoring those 4 mock checks so CodeQL turns completely green, eg.

# In examples/uk_companies_house_document_flow_demo.py (line 110)
# and skills/finance/uk_companies_house_handler/test_skill.py (lines 2407, 2500, 2736):

-        if "s3.example.com" in url:
+        if url.startswith("https://s3.example.com/"):

Once these changes are in we can merge. <3

@2409324124

Copy link
Copy Markdown
Contributor Author

@rosspeili Done, all 4 mock checks now use startswith as suggested. tks!

@rosspeili
rosspeili merged commit 1ba0675 into ARPAHLS:main Oct 15, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Skill Upgrade]: finance/uk_companies_house_handler v2d: Document API flow (list, metadata, fetch) (Parent: #220)

3 participants