Repository navigation
feat(finance/uk_companies_house_handler): Document API flow v2d (#421) - #434
Conversation
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.
rosspeili
left a comment
There was a problem hiding this comment.
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
|
Hey @2409324124, the codeql bot flagged 4 lines in test/demo code for
CodeQL scans all repository files by default and flags 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 |
|
@rosspeili Done, all 4 mock checks now use startswith as suggested. tks! |
Description
Implements Phase v2d of
finance/uk_companies_house_handler(parent #220): the Companies House Document API discover -> pick -> download flow. Bumps the skill to1.4.0.Fixes #421
Summary of changes
skill.py,manifest.yaml):list_downloadable_documents: filters filing history to items withlinks.document_metadataand returns a pick list (transaction_id,date,description,category,metadata_url,downloadable). Supportscategory,limit(max 25),latest_only, andlatest_per_category.get_document_metadata: returnsmime_types,content_length,total_pages, andcreated_atfrom the Document API.fetch_document: streams bytes in chunks and returns boundedcontent_base64.max_bytesdefaults to 5 MB with a hard ceiling of 10 MB. Also supportsinclude_content: falseandcontent_type.document-api.company-information.service.gov.uk./contentis requested withallow_redirects=False. TheLocationredirect is then followed without Authorization.content_lengthis checked before download, and a streaming byte counter aborts the download withdocument_too_largeif it goes over.selected_transaction_idanddocument_candidatesare carried forward. If atransaction_idis not in the candidates, the skill resolves it throughGET /company/{cn}/filing-history/{tx}. Filing transaction ids are not Document API ids.run_pipeline:list_downloadable_documentsis supported as a step, including<from_resolve>substitution.instructions.md(Document Mode playbook),docs/skills/finance/uk_companies_house_handler.md(schemas and examples), the category hub, thedocs/skills/README.mdrow,card.json, anddata/api_index.json.examples/uk_companies_house_document_flow_demo.py, indexed inexamples/README.mdand registered intests/test_examples_smoke.py.CHANGELOG.mdentry under[Unreleased].Maintainer decisions followed
rate_limitedenvelope withretry_afterand the Retry-After value inagent_hint. There is no sleep and no client-side throttling.Mocked test coverage (no network in CI)
New tests in
skills/finance/uk_companies_house_handler/test_skill.py:latest_only, the category filter, andneeds_inputwhencompany_numberis missing.not_found; 410 ->api_error; 429 ->rate_limitedwithretry_after.include_content: false./content.metadata_url.run_pipelinedocument flow; candidates surviving the pipeline context merge; unknowntransaction_idresolved through the filing-history item; unknowntransaction_idwithout a company number.1.4.0and the new action enum values.Live e2e (local only, against the real API, company
00445790Tesco 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_length82,803, 3 pages.fetch_document:/contentwith auth andallow_redirects=False-> 302 tos3.eu-west-2.amazonaws.com.application/pdf.%PDF-1.4, filename00445790_confirmation-statement_2026-07-01.pdf.document_too_largefrom metadata alone (no/contentrequest):max_bytes=1024was also rejected.not_foundfor both the metadata and fetch actions.run_pipeline(resolve_company"Tesco PLC" ->needs_input, resume with 00445790 ->list_downloadable_documentswithlatest_only):ready. A followingfetch_documentwith onlytransaction_idpluscontextdownloaded the same 82,803-byte PDF.document_candidatesfrom a pipeline step were dropped by the outer context merge.transaction_idas a Document API id, which returned 404.document_too_largeandnot_foundmessages 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
skills/examples/uk_companies_house_document_flow_demo.py,examples/README.mdChecklist (all PRs)
Fixes #421)python -m black --check .,flake8, andruff check .pass locallypytest skills/(539 passed, 1 skipped) andpytest tests/(567 passed, 8 skipped) pass locallyCHANGELOG.mdupdated under[Unreleased]examples/README.mdupdated for the new runnable scriptpytest tests/test_registry_docs.py(included inpytest tests/)pytest tests/test_skill_docs.py(included inpytest tests/)New or updated skill
Bundle and metadata
skills/finance/uk_companies_house_handler/(existing bundle)manifest.yaml: version1.4.0, new actions and parameters, constitution rules 8-10requirementsunchanged (requests);env_varsunchanged (the sameCOMPANIES_HOUSE_API_KEYis used for the Document API)Effect, Directive, Assurance
skill.pyinstructions.mdupdated with the Document Mode playbookcard.jsondescription updated; issuer unchangedtest_skill.pycovers the new actions and error pathsSkillLoader.load_skill("finance/uk_companies_house_handler")succeedsDocumentation and catalog
docs/skills/README.mdrow updated (existing skill, so no sitemap change)Constitution and safety (skills only)
metadata_urlmust be on a Companies House Document API host; other hosts are rejected withinvalid_metadata_url.max_bytes, with a 10 MB hard ceiling, enforced both from declared metadata and during streaming.Related Issues
Fixes #421
Parent: #220