Repository navigation
feat(org-tools): add the requirements extractor and a reusable catalog check - #76
Merged
damaz91 merged 27 commits intoOct 22, 2026
Conversation
Moves the requirements extractor from Universal-Commerce-Protocol/ucp (tools/requirements_extractor and scripts/extract_requirements.py) into org-tools, as agreed on ucp#853. The tool lives here; the catalog it generates stays in the ucp repository. This commit is a verbatim copy. The only changes are the import paths, which now resolve against scripts/ where the package sits beside the CLI. Formatting, path handling and fixes follow in separate commits so each can be reviewed on its own.
The code was written to the ucp repository's two-space style. This reformats it with the ruff defaults used across org-tools. No behavior changes: `git diff -w` on this commit shows only re-wrapped lines.
In ucp the extractor found the specification by walking up from its own file. It now lives in a different repository from the one it reads, so the specification checkout is passed in explicitly: uv run extract_requirements.py --spec-root path/to/ucp --check --spec-root is required, and a directory without mkdocs.yml is an error (exit 2). The published release is read from that file, so a wrong root would otherwise produce a catalog stamped "draft" with nothing checked. The recorded commit_sha now comes from the specification checkout rather than from this repository. Dependencies are declared inline (PEP 723) and pinned to the versions the ucp site builds with. `markdown` decides heading anchors, so a different version can publish URLs that do not resolve. Verified: run against ucp at 1c939c1e, --check passes on the catalog committed there, and a fresh write is identical apart from generated_at.
…sentence
The parser strips a trailing list-marker numeral that sentence splitting
can leave behind ("... unless already cached. 2."). The pattern anchored
on a word boundary, and the dot inside a number is one, so the last
component of any number ending a sentence was cut off:
All REST endpoints **MUST** be served over HTTPS with minimum TLS
version 1.3. -> ... minimum TLS version 1.
The marker is now only stripped when it follows the sentence's own
closing punctuation.
Across the whole specification this restores 21 truncated clauses
(TLS 1.3 in four REST bindings, "step 1" / "step 2" in identity
linking, RFC and section numbers, and dollar amounts in examples). The
new pattern strips nothing in the current corpus.
In the checkout catalog one record changes:
REQ-CHECKOUT-REST-TRANSPORT-SECURITY-01 gets its full clause and a new
content_digest (0c8c65f887 -> df21d831cd). Its id is unchanged.
Catalogs declared "$schema": "https://ucp.dev/schemas/requirements-catalog-v1.json", which returns 404 because nothing publishes it. The schema now ships with the tool, and both the catalog's $schema and the schema's $id use the URL GitHub serves it from on this repository's main branch: https://raw.githubusercontent.com/Universal-Commerce-Protocol/.github/main/org-tools/requirements-extractor/schema/requirements-catalog-v1.json The URL resolves once this change merges. The schema's description now names the tool's new location.
…e ids Several module docstrings still described the first design, where the content digest was the only identifier: - the package and identity docstrings said IDs were content-addressed and that duplicates received an appended ordinal; - the catalog docstring said no timestamp is recorded and warned against adding generated_at, which has since been added and is ignored by --check; - models.Annotation referred to an --include-annotations CLI flag that does not exist. Comments only; no behavior changes.
Catalog output changed in this series: the TLS clause is no longer truncated and $schema points at a new URL. The catalog format itself is unchanged, so schema_version stays 1.0.0.
72 unittest cases, following the org-tools convention of running offline from the repository root. They read a small fixture specification under tests/fixtures/spec rather than a pinned ucp checkout, so this repository's tests never depend on the state of another repository. - test_parser: sentence splitting (including numbers that end a sentence), code exclusion, reflow, colon stems and table context. - test_classifier: levels, the emphasis rule, code spans, boilerplate, compound splitting and annotations. - test_metadata: the actor ladder, referenced fields and conditions. - test_identity: normalization, a digest recomputed from published fields, ordinals, document tokens and duplicate reporting. - test_config: spec version, published URLs, id tokens, capability resolution, and that $schema equals the shipped schema's $id and path. - test_cli: end to end on the fixture, with every extracted requirement pinned, the catalog checked against the schema's keys, and --check covering missing, stale and timestamp-only differences. The tests fail against the code before the two fixes in this series (six failures), and pass after them. tests.yml gains a step that passes the markdown dependencies with --with, pinned to the same versions as the script, instead of adding them to the shared pyproject.
When --check found a stale catalog it printed the script's absolute
path, which in CI is a runner directory an author cannot use. It now
prints the command relative to the working directory, and names where
the extractor lives:
Regenerate by running the extractor without --check:
uv run .github-central/org-tools/requirements-extractor/scripts/extract_requirements.py --spec-root .
The extractor is org-tools/requirements-extractor/scripts/extract_requirements.py in Universal-Commerce-Protocol/.github.
segiodongo
force-pushed
the
feat/requirements-extractor
branch
from
September 29, 2026 18:20
9704b7e to
bbfc3e8
Compare
Repositories that commit a generated requirements catalog (ucp, for the checkout capability) call this to verify the catalog matches their specification. Modeled on reusable-copyright-check.yml: it checks out the caller, checks out this repository into .github-central, and runs uv run .github-central/org-tools/requirements-extractor/scripts/extract_requirements.py --spec-root <spec_root> --check Inputs: - central_repo: tools repository name (default ".github"). - central_ref: ref to run the extractor from (default "main"). Callers should pin it to the same commit as their `uses:` line, so the workflow and the extractor only change when the caller opts in. - spec_root: directory holding the specification's mkdocs.yml (default "."). Inputs reach the shell through env rather than inline expressions, and the job only needs contents: read. Path filtering is left to the caller's trigger, since a reusable workflow cannot filter paths itself. Verified by reproducing the job layout locally against ucp at 1c939c1e: the check fails on the catalog committed there (expected; it predates the fixes in this series), passes after regenerating, and prints the regenerate command.
Documents what the extractor produces, the two identifiers, how spec authors regenerate the catalog, how a repository wires up the reusable check (pinned to a commit), the classification rules, and how to run the tests. Adds the tool to the org-tools index and to the offline test command there.
segiodongo
force-pushed
the
feat/requirements-extractor
branch
from
September 29, 2026 18:31
bbfc3e8 to
6b877c8
Compare
damaz91
reviewed
Sep 30, 2026
damaz91
left a comment
damaz91
left a comment
Contributor
There was a problem hiding this comment.
Thanks @segiodongo for moving this over to org-tools and structuring the commit history and tests so cleanly! Left a few inline comments on edge cases and minor cleanups.
A colon-terminated paragraph is remembered so the list directly beneath it can record it as compound_parent. The stem was only cleared by a list or by later prose, so a code block, HTML block, heading or table in between let it leak into an unrelated list, and into that list's content digests. In the specification, the "MCP Transport" stem in overview/index.md was attached to the six requirements under "Platform Requirements", past a JSON sample and two headings; the payment guide's checklist stem crossed a heading the same way. The checkout catalog is unaffected. Reported in review.
…in prose REQUIRED, RECOMMENDED and OPTIONAL in running prose were promoted to MUST, SHOULD and MAY whether or not they were bolded, while MUST, SHOULD and MAY themselves only count when bolded. Apply the same rule: a bolded annotation keyword is promoted as before, and an unbolded one is reported under candidates_without_emphasis. When emphasis is mixed, the bolded keyword sets the level. The checkout catalog is unaffected: its five promotions are all bolded. Across the specification, four clauses move from the catalog to the report, among them "This is the RECOMMENDED path for integrators" in signatures.md. Reported in review.
…g metadata keyword_position() only searched MUST, SHOULD, MAY and their negations. A clause promoted from REQUIRED, RECOMMENDED or OPTIONAL has none of them, so it fell back to the end of the sentence. Actor extraction then searched the whole sentence and could take the object as the subject, and a trailing condition was read as a leading one and dropped. Search the annotation keywords when no obligation keyword is present. Obligation keywords still take precedence, so a sentence holding both is anchored on its MUST. The checkout catalog is unaffected. Across the specification one requirement changes: signatures.md:850 no longer takes "business", which follows **OPTIONAL**, as its actor. Reported in review.
…mkdocs.yml The organization's copyright check covers .yml files and flagged the test fixture, the only file in the tool without a header. Reported in review.
The site enables attr_list, so a heading such as `### Total {: #totals }`
renders as "Total" with the anchor #totals. The extractor kept the block
in the heading text: it showed up in section breadcrumbs, the published
URL pointed at #total-totals, and it leaked into readable ids.
Strip the block from the breadcrumb and use its id, if any, as the URL
anchor. Both use attr_list's own pattern and parser, so the result
matches the rendered site. The explicit id also becomes the section part
of the readable id: authors add one to tell same-named headings apart,
as with the order capability's two "Guidelines" sections
(#operations-guidelines and #events-guidelines), which would otherwise
collapse into one ordinal sequence.
The checkout catalog is unaffected: no checkout requirement sits
directly under an anchored heading, though report breadcrumbs under
"Total" are corrected. Across the specification 19 requirements in the
location and order documents get corrected ids and URLs.
Reported in review.
Links in the specification often carry attributes, as in
`[RFC 8259](https://...){ target="_blank" }`. Normalization removed the
link target but left the block, so `{ target="_blank" }` ended up in the
normalized text and in the content digest. The site renders the block
as HTML attributes; it is no more part of the wording than the target.
Consume a block directly after the link, using one pattern shared by
normalization and condition cleanup. A brace anywhere else, such as
`{action}` in inline code, is kept.
In the checkout catalog, REQ-CHECKOUT-REST-CONTENT-TYPES-01 and
REQ-CHECKOUT-REST-SPECIFIC-HEADER-01 get clean normalized text and new
digests; their ids do not change. Across the specification 60 digests
change, most of them in identity-linking.
Reported in review.
- readable_id() builds on _group_key() instead of repeating it, so the id and the key its ordinal is counted within cannot drift apart. - Remove config.ID_PREFIX, unused since READABLE_ID_PREFIX replaced it. - Remove Clause.is_obligation, which nothing uses. - _commit_sha() ignores stdout when `git rev-parse` exits non-zero. No change to the output. Reported in review.
Contributor
|
Thanks for resolving all the comments, LGTM now! |
damaz91
approved these changes
Oct 1, 2026
Settle the published shape before any catalog is committed downstream, so v1 does not need a breaking revision once conformance tests depend on it. - Records no longer carry a release. spec_version lives once in each catalog file's header, and source.url is replaced by a version-free source.page and source.anchor. Cutting a release now changes one header line per file instead of every record. - content_digest becomes text_digest, computed from the normalized compound_parent and clause only. Attributing a page to a different capability, or renaming a capability, no longer re-mints its digests. - Records gain scope and capabilities, so protocol-wide requirements and pages covering several capabilities can be represented without a later schema change. Every page in scope today is a capability page. - The catalog is written as one file per specification page under generated/requirements/catalog/, mirroring docs/specification/. Each file stays small enough to review, and --check flags catalog files that no page produces any more; regenerating removes them. - Every catalog file is validated against the schema with jsonschema before it is written or compared. The schema now requires every field the tool emits, and a document with no capability stops the run rather than producing an unattributed record, replacing --fail-on-unresolved. - The schema is renamed to requirements-catalog-1.0.0.json, so each format version is its own file that is never edited in place; a unit test pins its content.
Contributor
Author
|
Thanks again for the review and the approval! Before the first catalog lands in ucp, I've pushed one more commit that settles the catalog format, so v1 won't need a breaking revision once conformance tests start binding to it. It's a single commit,
Against current ucp 116 tests pass, and pre-commit passes on the changed files. I'm happy to split this into a separate follow-up PR if you'd rather merge this one as approved. |
nicholasjameshall
approved these changes
Oct 13, 2026
nicholasjameshall
approved these changes
Oct 13, 2026
nicholasjameshall
left a comment
nicholasjameshall
left a comment
Contributor
There was a problem hiding this comment.
this pr is pretty huge - I don't suppose we could break it into smaller prs to make it easier to review?
summarize() spelled out five of the seven obligation levels, so a SHALL or SHALL NOT requirement was counted in total and by_level but in no per-level counter, and the counters no longer added up to total. - Level is now the single definition of the levels. The per-level counters are generated from it, which adds shall_count and shall_not_count and covers any level added later. - The schema's summary lists and requires all seven counters. v1 has not been published yet, so its pinned digest is updated with it. - New tests keep the copies that cannot import Level in step with it (the keyword list in config and the schema's level and summary lists), and check that a requirement at every level is counted and the counters add up to total.
Open
8 of 17 tasks
gsmith85
reviewed
Oct 15, 2026
`central_ref` read naturally as a ref of the calling repository, when it is the commit of this repository the extractor is checked out from. The new name says what it pins, and the description says which repository it belongs to. No caller uses the workflow yet, so the rename breaks nothing.
test_catalog.py validates against the published schema, so the command failed at test discovery without jsonschema. It now matches tests.yml and the extractor's README.
…erride The `<!-- ucp:actor=... -->` hook could never fire on real Markdown. The parser drops inline HTML before a clause's text is built, and a line that starts with an HTML comment is an HTML block, which the parser skips whole. Its test passed only because it handed extract_actor text that the parser cannot produce. No specification page uses the comment, so removing it changes no output. A page-level metadata block is the better home for overrides if the need comes back.
Four fixes to the actor ladder, each measured against the whole
specification on ucp main:
- A possessive ("the buyer's system preference") is used only when no
party is named plainly before the keyword, so the Embedded Checkout
rather than the buyer is the subject of REQ-CHECKOUT-EP-COLOR-SCHEME-01.
A blanket skip was rejected: it unbinds rules such as "The handler's
trust policy MUST ...", where the possessive is the party.
- "server(s)" resolves to the business and "MCP client(s)" to the
platform. A bare "client" stays out of the lexicon: it matched an "HTTP
client hint", a "Client ID Metadata Document" and the "public clients" a
business serves. A name nested in a longer one is no longer read on its
own, so "authorization server" stays the authorization server.
- A table cell takes its party from a row or column header that is a
party's name, as in an obligation matrix. A descriptive header that only
mentions a party is not used.
- Headings are searched deepest first, so "Business Responsibilities"
wins over the "Fulfillment Extension" heading above it.
Checkout changes in two records: EP-COLOR-SCHEME-01 (BUYER to
EMBEDDED_CHECKOUT) and MCP-REQUEST-METADATA-01 (unresolved to PLATFORM).
Across the whole specification 19 records change, all to the correct
party, and none get worse.
…zing and splitting Normalization stripped bold markers but kept single-asterisk italics, so emphasizing a word changed a requirement's digest. It now strips both, outside code spans only: inside backticks an asterisk is content, as in `dev.ucp.*` and `^[a-z][a-z0-9_]*$`, and stripping it there would have changed four other rules' digests for no reason. Compound sentences were split on a ";", "and" or "or" found anywhere between two keywords, including inside a code span. The split point is now looked for in the masked text, which keeps offsets, so a cut can no longer land inside backticks. Checkout is unchanged. Across the whole specification, four digests move, each on a rule that contains italics.
…inistic Drop `generated_at` and `commit_sha` from the header of every catalog file and the report. The timestamp rewrote every file on every run, even pages whose rules had not changed. The commit could not be right: a file cannot name the commit that contains it, so it was always one commit behind, and orphaned after a squash merge. Git already records both for every file. With every field a function of the specification, `--check` now compares files byte for byte, and `NON_DETERMINISTIC_FIELDS`, `_comparable()` and `_commit_sha()` are gone. The schema drops both properties. It has not been published yet, so it is revised in place and its pinned digest is updated. Regenerating checkout from ucp main changes nothing but the two removed header lines in each file.
The output changed: the header no longer carries `generated_at` or `commit_sha`, actor attribution changed on two checkout records, and digests of rules containing italics moved.
gsmith85
approved these changes
Oct 21, 2026
gsmith85
left a comment
gsmith85
left a comment
There was a problem hiding this comment.
Approving! Thanks for addressing the feedback across all the threads, Segi.
One thing to keep in mind for the roadmap is how we handle anaphoric sentences, where a normative clause grammatically depends on an unbolded preceding sentence for context (for example, "When a buyer presents an expired token, the platform logs the attempt. The server MUST return 401 Unauthorized in this case."). Today, sentence splitting drops the first sentence, leaving the extracted requirement without the condition it governs.
When we want to solve for this, a couple of lightweight options on the specification Markdown side could be:
- Markdown attribute list annotations via MkDocs
attr_list(such as{: ucp-context="..." }), attaching explicit context or an actor to the clause. - An inline HTML directive/comment (such as
<!-- ucp:context="..." -->or<!-- ucp:include-preceding-sentence -->) preserved during parsing. - Automatically retaining bullet lead-ins and stem context, as you noted in the stacked follow-up plan.
This is completely deferrable and not a blocker for landing this PR, but something worth keeping in mind as we start mapping coverage. Looking forward to seeing the test-marker annotations in the reference implementation!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This moves the requirements extractor from Universal-Commerce-Protocol/ucp#853 into
org-tools, as agreed on that PR: the tool lives here, and the catalog it generates stays in the ucp repository next to the specification it describes.The extractor reads the normative prose of a UCP specification and writes a machine-readable catalog of its RFC 2119 requirements, one file per specification page. Each requirement gets a readable id for conformance tests to bind to (
REQ-CHECKOUT-REST-TRANSPORT-SECURITY-01), a text digest that survives the document being reorganized, its level, the party it binds, and the page and anchor where it is published. A companion report accounts for every keyword-bearing clause that did not become a requirement. Every catalog file is validated against the published schema before it is written. Scope is the checkout capability for now.What's in this PR
org-tools/requirements-extractor/scripts/extract_requirements.pyand therequirements_extractorpackage)org-tools/requirements-extractor/schema/requirements-catalog-1.0.0.jsonorg-tools/requirements-extractor/tests/org-tools/requirements-extractor/README.md.github/workflows/reusable-requirements-check.yml.github/workflows/tests.ymlorg-tools/README.mdCommits
The commits are meant to be reviewed one at a time:
git diff -wshows only re-wrapped lines.--spec-root: the tool now reads a specification checkout passed in explicitly, instead of finding it relative to its own file. The flag is required, and a directory withoutmkdocs.ymlis an error. Dependencies are declared inline (PEP 723) and pinned to the versions the ucp site builds with.$schemareturned 404. It now points at the raw GitHub URL of the shipped schema, which resolves once this merges. The schema's$idmatches.extractor_version0.2.0.schema_versionstays 1.0.0.--checknow prints a regenerate command an author can use, instead of an absolute runner path.reusable-copyright-check.yml.source.page+source.anchorreplacesource.url;text_digestreplacescontent_digestand no longer includes the capability; records gainscopeandcapabilities; the catalog is one file per page; every file is validated against the schema; the schema is renamed torequirements-catalog-1.0.0.jsonand pinned by a test.extractor_version0.3.0.Levelis the single definition of the levels: the summary's per-level counters are generated from it (addingshall_countandshall_not_count), the schema's summary lists and requires all seven, and tests keep config's keyword list and the schema in step with it.central_refis renamedextractor_ref; the org-tools test command installs jsonschema; the unreachable<!-- ucp:actor -->override is removed; actor attribution is tightened (a possessive counts only when no party is named plainly,serverandMCP clientname parties, a table cell can take its party from a header that is a party's name, headings are searched deepest first, and a name inside a longer one is not read on its own); emphasis is stripped outside code spans only, and compound sentences are never split inside code;generated_atandcommit_shaare dropped, so every file is byte-for-byte deterministic and--checkcompares bytes.extractor_version0.4.0. In the checkout catalog, two records change actor and no ids change.How ucp will use it
Once this merges, a follow-up PR in ucp will:
generated/requirements/catalog/and the report, generated with this version (the tool itself stays here, replacing the copy that was in feat(tools): add requirements extractor proof of concept for the checkout capability ucp#853);The check is intended to start as advisory.
Testing
uv run --with jsonschema==4.26.0 --with markdown-it-py==3.0.0 --with markdown==3.10.2 python -m unittest discover -s org-tools/requirements-extractor/tests: 130 tests pass.pre-commit runpasses on the changed files.generated_at) before the fixes. After the fixes, only the TLS record changed.main, checkout regenerates to 221 requirements across 5 page files (the largest is 130 KB), every file validates,--checkpasses on a second run, and no ids change. The three fewer records than feat(tools): add requirements extractor proof of concept for the checkout capability ucp#853 are the shared REST header rules, which docs: centralize shared REST header requirements via macro ucp#819 moved into an mkdocs macro; macro-rendered text is a follow-up..github-central).--checkpasses on a current catalog, fails when the catalog is missing or out of date, and passes again after regenerating.Notes for reviewers
uv runwith--withintests.ymlinstead of being added to the sharedpyproject.toml, so other tools' environments don't change.$schemaURL is long. Aucp.devURL would read better, but nothing publishes one today. Switching later only needsCATALOG_SCHEMA_URLand the schema's$idto change together, and a test enforces that they match.unpinned-usescheck is mandatory for changed workflow files. That's whytests.ymlalso shows its existingcheckoutandsetup-uvlines changing, along with a read-onlypermissions:block. The versions are the same ones the rest of the repository uses.