feat: add markings support for datasets, connections, and videos

[drake-nominal]

Adds support for markings — the access-control primitive that restricts a data source to users in the marking's authorized groups. Markings were previously reachable only as raw RID strings on create_dataset; this makes them a first-class resource.

What's new

Marking — a frozen dataclass with rid, id (a slug unique within the organization), description, symbol, color, created_at, updated_at, is_archived, plus update(), archive(), unarchive(), refresh(), and authorized_group_rids().

Symbol — display metadata, built through classmethod constructors: Symbol.icon(...), Symbol.emoji(...), Symbol.image(...). A marking's color is a plain lowercase hex string ("#cc0000"), validated at the call boundary the same way its id is.

Markings on resources — Dataset, Connection, StreamingConnection, and Video gain four methods from a shared mixin:

dataset.list_markings()
dataset.apply_markings([itar])            # accepts Marking instances or RIDs
dataset.remove_markings(["ri.marking.x"])
dataset.set_markings([itar, secret])      # atomic replace, computed as a diff

Client methods — create_marking, get_marking, get_marking_by_id, search_markings.

Markings at creation — markings= on create_dataset, create_streaming_connection, create_video, and the experimental dataset helpers. Accepts Marking instances or RID strings; the existing string form is unchanged.

itar = client.create_marking("export-controlled", description="ITAR", color="#cc0000")
dataset = client.create_dataset("flight-data", markings=[itar])

Notes for reviewers

• Markings apply only to data sources (datasets, connections, videos). The mixin's membership is the guard — other resource types simply don't have these methods.
• Creation with markings is not atomic: the resource is created, then markings are applied. Every create path's docstring says so, since a failure in that second step leaves an unmarked resource.
• search_markings intentionally takes no workspace or archive_status argument — markings are organization-scoped, and archived markings are excluded from search regardless. Results are oldest-first; no sort option is exposed.
• Creating, updating, and archiving markings requires organization admin permission; reading requires organization membership. Docstrings state this.
• Marking carries no name — only the id slug.
• get_marking uses the single-get endpoint rather than a one-element batch, so a marking that is missing and one the caller cannot read raise distinct errors (NominalNotFoundError vs NominalPermissionDeniedError) instead of both reporting as not-found.
• Deferred by design: a batch get-by-RIDs accessor, listing the resources a marking applies to, and applied_at timestamps.

Testing

694 unit tests pass; just check and strict mypy are clean. End-to-end coverage is read-only (search_markings, list_markings) because the write paths require organization-admin rights the e2e token isn't guaranteed to have — those paths have unit coverage only.

Suggest squash-merging: several commits here are intra-branch cleanups of private helpers that never shipped, and they would otherwise appear in the changelog.

🤖 Generated with Claude Code

[claude]

Claude finished @drake-nominal's task in 5m 34s —— View job

---

Review: markings support

Solid, well-shaped addition. The design decisions that matter are the right ones: MarkableMixin membership as the "only data sources can be marked" guard beats a runtime type check; Marking | str unions routed through the canonical rid_from_instance_or_string match how the rest of the client accepts resources; the nested _NotProvided sentinel mirrors Channel.update; module-private _create_marking/_get_*/_search_* mirror event.py; and docs/src/reference/core.md renders ::: nominal.core, so the new __all__ entries need no doc wiring. Client-side validation of the id slug and hex code — failing at the call site with a message that states the rule the user can't see — is the standout call here, and elements.py is correctly split out as a shared display-metadata module rather than buried in marking.py. No breaking changes: create_dataset(markings=) is widened from Sequence[str], everything else is a new keyword-only param with a default.

Five inline comments. The one that matters:

• The "creation and marking are not atomic" note is false, in four docstrings. Every create path passes marking_rids into the create request itself; nothing applies markings afterwards. It instructs users to do post-create verification for a hazard that doesn't exist.

The rest: an unverified "case-insensitive" claim on search_markings (#900 was about exactly this class of error), a marking_rids param added to _construct_new_ingest_options which has no callers, five open-coded copies of the Marking | str → RID mapping in two spellings, and vacuous assert all(...) assertions in the e2e tests.

Minor, not worth their own threads:

• MarkableMixin's docstring opens "Concrete: classes gain these methods…" — reads like an editing artifact.
• _get_marking_proto's len(markings) != 1 branch is unreachable: the request carries exactly one RID, and the not markings case above already handles the only other outcome.
• Several new tests (test_color_round_trips_through_proto, test_id_substring_becomes_a_substring_clause, test_create_sends_symbol_and_color, …) skip the one-line docstring the surrounding tests carry.

Unit test coverage is genuinely good — test_update_distinguishes_unchanged_from_cleared_symbol and the two wiring-pin tests for Video/StreamingConnection test the things that would actually break silently.

Static review only — I did not run tests, ruff, or mypy; CI covers those.
· worktree-markings-support

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

[claude]

"Case-insensitive" — what backs this? The query field is id_exact_substring_search, and marking ids are already constrained to lowercase by _validate_marking_id, so the case story is unobservable from the client. #900 was specifically about mis-stated search semantics; either confirm the backend lowercases before matching or drop the qualifier.

[claude]

_construct_new_ingest_options has no callers — only its definition and tests/core/test_marking.py:368 reference it. This adds a parameter no code path can reach, plus a unit test whose own docstring concedes "this builder has no callers yet." Drop both; wire the parameter when a caller exists, and let that caller's test cover it.

[claude]

This "not atomic" note is wrong, and it's repeated on create_dataset_with_uuid, NominalClient.create_dataset (client.py L868-870), and NominalClient.create_video (L912-914). Every one of those passes marking_rids straight into the create request (L64 here); nothing calls apply_markings afterwards — grep apply_markings nominal/ only hits the mixin definition. Creation is atomic, so instructing users to verify with list_markings() after every create is a real cost for a hazard that doesn't exist.

[claude]

all(...) over a list that is expected to be empty asserts nothing — this passes identically if list_markings returns [], and so does the analogous assert all(...) in test_search_markings_returns_unarchived_markings. The only real signal is "the RPC didn't raise". Either say that (assert ingested_dataset.list_markings() == [] for a fresh dataset, which is a genuine claim) or drop the assertion and keep the call as an explicit smoke check.

[claude]

Marking | str → RID coercion is now open-coded at five call sites (here, _derived_datasets.py L64, client.py L883/L928/L1207) in two spellings that disagree on the empty case ([] if markings is None else [...] vs None if markings is None else [...]). Export one _marking_rids(markings: Sequence[Marking | str] | None) -> list[str] from nominal/core/marking.py and call it at each site: the create paths then read identically and the None-vs-[] question is answered once, in the module that owns the concept.