Carry the terms in the identifier, so they travel with it - #79
Merged
Conversation
The 51Did payload gains one byte after the match key, the Terms, which says which terms document the identifier was created under so that the terms travel with the identifier instead of alongside it. The byte is an index into a table in the specification and not a version number, so a later document can live at any address, and an index is never reused or repointed once published because an identifier issued under it has to stay readable years later. Three members are added to FodId, being terms as a named value, the raw terms_index, and terms_url which is the address or None. A new Terms enumeration in terms.py carries the vocabulary and the addresses, the same shape as the existing Usage and IdType. Terms.NOT_STATED and Terms.UNKNOWN are different answers. NOT_STATED is index 0 and says the identifier does not carry the answer, so it has to come from the surrounding protocol, whilst UNKNOWN says the identifier does state terms this package cannot name because the index was added after the package was released. Reading UNKNOWN as NOT_STATED would read an identifier created under terms as one created under none, so the two are separate members and terms_index is exposed to let a caller say which index it could not read. Neither has an address, and the package answers with an address and never fetches it. Existing identifiers are unaffected. A payload ending at the match key has no byte to read and answers index 0, which is the same answer as a zero byte, so absence and zero mean the same thing and no presence flag exists. No field before the Terms moves and no existing test changed. A Reserved type has no defined match key length and keeps its documented best-effort reading, taking every byte after the header as its match key, so it leaves no byte to read as the Terms and answers index 0. Depends on 51Degrees/specifications#27 and must not merge before it.
The specification gives the table of terms documents but no names for the enumeration members, and six language packages were about to invent six different sets. The names are now fixed across all of them, being NotStated for index 0, ModelTermsForMarketing2 for index 1 and Unknown for an index the package does not know, cased the way each language cases its own enumeration members. Python therefore uses NOT_STATED, MODEL_TERMS_FOR_MARKETING_2 and UNKNOWN. Only the middle one changes here, from MODEL_TERMS_FOR_MARKETING_VERSION_2, along with the string behind it. The comment on reading the Terms of a Reserved identifier now says plainly that answering with terms not stated is the right answer and not a defect, because no match key length is defined for that type, so every byte after the header is its match key and none is left to read. The next reader should not spend time looking for a bug there.
No reader has ever seen a 51Did without the Terms byte, so describing the field as a change from a previous state gives a reader history they cannot use. Every rule the history was wrapped around is kept and reworded to describe the payload instead. A payload that ends at the match key reads as index 0, which says the terms are not stated in the identifier, so absence and zero mean the same thing and no presence flag is needed. A Reserved identifier still reads as index 0, because no match key length is defined for that type so every byte after the header is its match key.
…he Terms with its address Bits 4 and 5 of the flags byte are the payload version. This package reads version 0 and refuses any other with FodIdParseStatus.UNSUPPORTED_PAYLOAD_VERSION, and the raising readers name the version they found. No field is read under the layout this package knows once the version says otherwise, because a later version exists precisely because a field moved, so reading such a payload here would answer with values that are wrong rather than absent. The version is not exposed, because either the package read the layout or there is no identifier to read fields from. The Terms is one member rather than three. fod_id.terms answers with the address of the document the identifier was created under, and the package turns the index into the address so a caller never handles the byte. The raw index and the separate address member are gone, and terms.py is now _terms.py, private to the package as the layout module is, and is no longer exported. An index of zero and an index this package cannot name both answer with None, which a caller cannot tell apart, and that is deliberate because both say the identifier does not give the terms and the answer has to come from somewhere else. No address is ever built from an index, since that would name a document nobody wrote. The test payload builders are the creating side, so they write both new fields. The canonical flags byte carries version 0, the canonical payload carries the Terms of a personalized marketing identifier and the canonical Random payload carries the zero a non-marketing identifier carries. A payload that ends at the match key is now a fixture of its own, since a reader takes it as a Terms of zero and no issuer would write one. Tests: 171 passed, 2 skipped, 29 subtests passed. The offline example prints the address.
Adding a terms document meant editing the member, an index to member dict and an address dict, three places that could disagree. The address lookup also subscripted the dict directly, so a member missing from it would raise KeyError at the caller rather than answer with no address. Each member now carries its index and its address, and the index to member map is built from the members rather than written out again, so a new document is one new member and nothing else. The address comes off the member, so there is nothing left to raise. A test that claimed a member without an address entry would raise there described the old design and could not fail under the new one. It is replaced by two that check what now holds, being that every member reads back from its own index and that a member naming a document has an https address, and that no two members share an index. pytest: 89 passed, 29 subtests, 0 failures.
justadreamer
marked this pull request as ready for review
September 11, 2026 07:45
The readme told a caller to catch OwidError for a payload version this package cannot read. _unpack_or_raise raises ValueError for every status _read_payload returns, so the one new failure this branch introduces was the one a caller following the readme would not catch. The status joins the ValueError list, and the table of statuses, which described the version refusal in prose without ever listing it. Five counts still said two where the version check made them three, being the members added to the OWID status vocabulary, the payload rules _read_payload applies and the ones _from_read applies in turn. The test pinning the vocabulary was renamed to _plus_three; the prose around it was not. The Terms docstring told a caller to treat UNKNOWN as terms it cannot read and to refuse the identifier. No caller can meet UNKNOWN. It is a private member and terms answers None for it exactly as it does for NOT_STATED, which the package surface specification requires, so the paragraph described a surface removed earlier on this branch. It now says why the two reach a caller as one answer and why UNKNOWN is still a member of its own, being that nothing may compose an address from an index the table does not carry. The claim that members carry a name and not the index went with it, every member having carried its index since the terms table was made one place. _payload_length_valid kept the old wording about the bytes beyond the match key, which was reworded in fod_id.py, the readme and the envelope builder when the Terms byte took that position. The readme now also says that no identifier already issued is refused by the version check, the two bits having been written as zero by every issuer before the field was defined, which is the question a reader of that section asks. Also tidied whilst here: two PEP 8 blank-line slips in test_fodid.py, a docstring line in fod_id.py left at 92 characters by a reflow, and the terms constants in test_fodid.py moved to the top block with with_terms above its first caller.
ABSENT_TERMS_INDEX in _layout.py and Terms.NOT_STATED in _terms.py both recorded that index 0 says the terms are not stated in the identifier, so the fact the terms table was made the single place for was still written down twice and could disagree. _layout.py holds offsets and lengths, and an index value is neither. _read_terms_index now reads Terms.NOT_STATED.index, and the tests and the envelope builder that wrote the constant name the same member. TERMS_LENGTH stays in _layout.py, being a length. The assertion that the layout gives the constant the value zero is dropped, because test_every_member_agrees_with_the_table already pins NOT_STATED to index 0 and there is no second place left to disagree with it.
oleksandrlazarenko-pi
approved these changes
Sep 11, 2026
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.
Draft. The specification this implements has merged as 51Degrees/specifications#27, and this stays in draft until the cloud has been updated to write both fields. pipeline-dotnet#409 goes first, because the cloud is .NET and needs that package published before it can write anything.
What this is
Two fields of the 51Did payload, read by
fiftyone_pipeline_did.The Terms is one byte after the match key, saying which terms document the identifier was created under so that the terms travel with the identifier instead of alongside it. A receiver can then tell which document was in force when the identifier was made, rather than having to be told separately and rather than trusting an address whose contents can be edited later.
The payload version is bits 4 and 5 of the flags byte, saying which layout the payload follows.
The Terms
The byte is an index into a table in the specification and not a version number, so that a later document can live at any address rather than only at one the specification could compose from a number. An index is never reused or repointed once published, because an identifier issued under it has to stay readable years later.
fod_id.termsanswers with the address of the document. The package turns the index into the address, so a caller never handles the byte.fod_id.terms0None1https://m4ow.uk/mtm/2.txtNoneThat one member is the whole of the addition to the public surface. The address for index 1 is exactly
https://m4ow.uk/mtm/2.txt, the versioned text of the Model Terms for Marketing version 2, and no other form of it. The package answers with the address and never fetches it, because what to do with the document is the receiver's decision.The named value the specification gives for each index lives in
_terms.py, private to the package as_layout.pyis, so the names are recorded once and a caller never handles the byte.The payload version
This package reads version 0 and refuses every other version with
FodIdParseStatus.UNSUPPORTED_PAYLOAD_VERSION, and the raising readers name the version they found in the message.No field is read under the layout this package knows once the version says otherwise. A later version exists precisely because a field moved, so reading such a payload here would answer with values that are wrong rather than absent, which is worse than refusing. A version that nothing checks protects nothing.
The version is not exposed. Either this package read the layout, in which case the accessors are the answer, or it did not, in which case there is no identifier to read fields from.
The parts that are easy to get wrong
No address is ever built from an index this package cannot name. That would name a document nobody wrote, and a receiver would record having accepted terms that do not exist. An index of zero and an index added after this release therefore give a caller the same answer, which is deliberate, since both say the identifier does not give the terms and the answer has to come from somewhere else.
The byte is read at the offset the match key ends at. The identifier type fixes the match key length and so the type says where the field starts, which is why
_layout.pycarriesTERMS_LENGTHand no offset constant. A reader with one fixed offset reads a match key byte on aRANDOMidentifier.A payload with no byte after the match key reads as index 0. That is the same answer a zero byte gives, so absence and zero mean the same thing and no presence flag exists to tell them apart. No field before the Terms moves, because the byte sits after the match key and every offset before it is fixed by the header and the type.
An identifier of the
RESERVEDtype reads as index 0. No match key length is defined for that type, so every byte after the header is its match key and none is left to read. That is the right answer and not a defect, and no special case is written for it. There is a comment saying so at_read_terms_indexand a test pinning it.No address does not mean the identifier is unrestricted. It says only that the identifier does not carry the answer, so the answer has to come from the surrounding protocol, being the Terms Document Locator in an OpenRTB request or whatever else it provides. Where an identifier may go is a separate question
fod_id.usageanswers.The Terms sits before the creator context section. An issuer writing a context section writes the Terms byte before it, and this package holds no upper bound on a payload, so a longer one is accepted and the same fields are read.
The version is read on its own bits. A reader masking the wrong bits would refuse a version 0 identifier of some usages, or let a later version through, so there is a test over every combination of the usage and type bits.
Files changed
src/fiftyone_pipeline_did/_terms.pyfrom_indexandurl. Renamed fromterms.pyand no longer exported, since the package turns the index into an address.src/fiftyone_pipeline_did/fod_id.pyUNSUPPORTED_PAYLOAD_VERSION, and answerstermswith the address read at the offset the match key ends atsrc/fiftyone_pipeline_did/_layout.pyTERMS_LENGTH,ABSENT_TERMS_INDEXandSUPPORTED_PAYLOAD_VERSION, private as the other layout constants aresrc/fiftyone_pipeline_did/__init__.pytests/test_fodid.pyreadme.mdexamples/fodid_example.pyThe test fixtures write what an issuer writes
canonical_payload()andcanonical_random_payload()are the creating side, so they now carry the payload version 0 in the flags byte and the Terms byte their usage calls for, being index 1 for the personalized marketing canonical payload and index 0 for the non-marketing Random one.payload_ending_at_match_key()andrandom_payload_ending_at_match_key()are the fixtures for a payload that carries no Terms byte, which a reader takes as index 0 and which no issuer would write.Tests
FodIdTermsTests, building the payload byte by byte the way the existing tests do:test_payload_ending_at_the_match_key_has_no_address(both match key lengths)test_an_absent_byte_and_a_zero_byte_read_the_sametest_index_one_answers_with_the_model_terms_address(both match key lengths)test_the_address_is_the_versioned_documenttest_an_unknown_index_has_no_addresstest_an_unknown_index_answers_as_no_terms_stated_doestest_every_index_the_package_does_not_know_has_no_addresstest_terms_is_read_before_a_creator_context_section(both match key lengths)test_a_context_section_alone_does_not_state_termstest_reserved_type_takes_every_byte_as_its_match_keytest_every_reader_reads_the_same_termstest_base64_roundtrip_preserves_the_termstest_terms_does_not_change_how_the_other_fields_readtest_the_payload_rules_are_unchanged_by_the_termsFodIdVersionTests:test_version_zero_reads_every_fieldtest_the_layout_names_the_version_this_package_readstest_an_unassigned_version_is_refusedtest_the_raising_readers_name_the_versiontest_the_version_is_read_apart_from_the_usage_and_type_bitsTermsTests, over the vocabulary alone, includingtest_the_named_value_is_not_part_of_the_package_surface.The terms table
The address, the index and the name are one table, so adding a terms document is one row and not a search for every place a number was written down. That is the point of the byte being an index rather than a version number, and the first version of this change did not honour it, spreading the same fact over three, being the member, an index to member dict and an address dict. The address lookup also subscripted its dict directly, so a member missing from it would raise
KeyErrorat the caller rather than answer with no address places that could disagree.Each member now carries its index and its address, and the index to member map is built from the members rather than written out again, so there is nothing left to raise.
A test that claimed a member without an address entry would raise there described the old design and could not fail under the new one. It is replaced by two that check what now holds, being that every member reads back from its own index and that a member naming a document has an https address, and that no two members share an index.
What was run and what it reported
Python 3.14.5 on Windows, with the
owid-pythonsubmodule on the path and the OWID source copied in byci/copy-owid-source.ps1, which is whatsetup.ps1does.The 2 skipped are the live cloud tests in
test_did_client_live.py, which need a resource key.The offline example runs and prints the address:
toxitself could not be installed on the machine used, because thevirtualenvwheel would not download. The command above is the onetox.iniruns, executed directly against the same environment.The tests were checked against a wrong reader
A green suite proves nothing on its own, so two deliberate faults were put into the reader to see which tests caught them.
Reading the byte at the end of the payload instead of at the offset the match key ends at:
Reading an index the package does not know as index 0 with an address:
Both faults were then removed and the suite returned to green.
Outstanding