Skip to content

fix: the ODPS and ODCS exporters against the released Bitol standards, and seven optional 0.7.6 fields - #753

Merged
fas89 merged 27 commits into
mainfrom
fix/odps-export-import-defects
Oct 27, 2026
Merged

fas89 merged 27 commits into
mainfrom
fix/odps-export-import-defects

Conversation

@fas89

@fas89 fas89 commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes the ODPS and ODCS exporters and importers against the released Bitol standards, and adds seven optional fields to the 0.7.6 preview schema for things a contract could not previously say. Most of this was found by auditing forge's output against the published specs and by feeding its exports to a real contract resolver and a real DQ engine; the rest was found by reviewing the fix.

Read this first if you consume forge's exported artifacts

Exported ODCS and ODPS documents change shape. Each change corrects output that was wrong, and the ### Changed section of CHANGELOG.md lists them, but the old values may be baked into consumers:

  • a per-port ODCS contract is now named after its port, not the product, with the product name in a fluidProductName custom property;
  • ODPS inputPorts[].contractId is now <productId>.<exposeId> rather than the bare upstream product id;
  • a Kafka server's ODCS format is a message format when the contract declares binding.serialization (a contract that has not adopted the field exports exactly as before);
  • a Kafka column's physicalType is an Avro type name — notably bigint is now long, where it was integer;
  • derived quality rules carry an id, and declared dq.rules are exported as ODCS quality;
  • fluid odps import refuses to write an invalid contract instead of reporting success (--lenient restores the old behaviour), and its pass-through bucket moved from metadata.odps_passthrough to the root extensions.odps.

Two commits are marked breaking. A contract that pins an exposes[].contract.id colliding with a sibling's id now fails fluid validate and every export path. That export used to succeed and silently lose one output port's entire schema, so the failure is the fix.

The defects

Silent data corruption. A Kafka contract exported a bigint column as physicalType: integer. fluid_to_physical falls back to the ODCS logicalType when a provider has no physical-type table, Kafka had none, and ODCS's own schema defaults an unqualified integer to i32 — so a tool deriving an Avro schema read 32 bits while a producer such as Flink BIGINT wrote 64. The registered schema and the data disagreed with nothing to signal it.

Silent data loss. A pinned contract id equal to a sibling's derived <productId>.<exposeId> passed fluid validate, and the export then wrote one per-port ODCS file for two output ports: the losing port's schema was absent from the bundle and both ports resolved to the survivor. fluid odps validate was green over it and generate artifacts sealed it under a MANIFEST merkle root. Introduced by this branch's own new field and caught in review.

A credential in the clear. fluid odps import's validate-before-write gate quotes the schema error, which quotes the offending document value — so a bundle whose root id was a DSN had its password printed to stderr, while a GitHub token in the same string was masked, because the redactor knew that shape and was simply not on this path.

Exports a resolver and a DQ engine rejected. Derived quality rules carried no id, and one engine refuses the whole contract for it. Rules an author declared in dq.rules were never exported at all, only parked in fluidExtras. A column's quality was replayed on export although the closed column definition made fluid validate reject it. And fluid odps import reported success over a contract its own validator failed with ten errors.

Commands that lied or crashed. fluid odps-bitol validate checked five keys and never ran the schema, so it and fluid odps validate gave opposite answers on the same file. --env was accepted and ignored by three generate standard formats. -o out.yaml with a bare filename raised FileNotFoundError: ''. Three validate commands crashed to exit 2 on malformed JSON, non-UTF-8 bytes or a too-deeply-nested document. The vendored ODPS v1.1.0 schema was a dev-branch snapshot taken before the 2026-09-08 release, so it rejected documents valid against the published standard.

New in the 0.7.6 preview schema

All optional and additive, so a contract that sets none of them validates and exports byte-identically. Names are taken from Bitol ODCS v3.1.0 and the released ODPS v1.1.0 so import and export are a pass-through rather than a translation: metadata.team with members[].{username, name, role, email}; the root description as a string or {purpose, usage, limitations}; consumes[].contractId; exposes[].contract.id; binding.location.host; binding.serialization; and quality on a column. All require fluidVersion: "0.7.6", which stays a preview — see #752 for the promotion criteria.

Borrow receipts

  • Bitol ODPS v1.1.0 @ tag v1.1.0 — vendored verbatim. A structural diff against the tag reports 0 differing values; the only difference from upstream is the $comment, which names the tag and says to refresh from a tag rather than a branch.
  • Bitol ODCS v3.1.0 @ tag v3.1.0 — Team/TeamMember, description, DataQuality (metric and operator names, and StableId for the rule id charset) and KafkaServer read from the schema rather than guessed.
  • Avro 1.12 specification and datacontract-cli's datacontract/export/avro_exporter.py::to_avro_type — the Kafka physical-type table, including decimal carrying precision and scale in customProperties. The test replays that consumer's mapping on the exported property and parses the result with the real Apache Avro library, so the claim is checked against a tool rather than by eye.
  • Reused in-repo rather than rewritten: the ODCS importer's normalisation helpers, load_contract_with_overlay, contract_id_for_port, BitolOdpsProvider.validate_product and the existing or "." dirname guard.

Validation

  • Full suite: 28054 passed, and a second broad run after the final round gave 6222 passed. The only failures are the pre-existing machine-specific ones (DuckDB sandbox TMPDIR classification, dotenv/dlt credential allowlist, a live-LLM test); a full run against a clean origin/main extraction shows zero HEAD-only failures.
  • Every CI gate that runs on a diff was reproduced locally and exits 0: both ruff invocations, black --check at the exact pinned 24.10.0, Product Guardrail, lint-imports, mypy_strict, license headers, bandit, semgrep at the pinned 1.163.0, detect-secrets, and the Build Smoke Test including twine check --strict and an isolated wheel install.
  • Cold path: fluid --help imports 1018 modules against the 1100 ceiling — 82 of headroom, unchanged from baseline — with the forbidden-import set still absent.
  • Driven through the real CLI, not only tests: the preview gate in both directions, the collision guard across twelve export doors including the false-positive case that must stay accepted, the linking invariant across all five ODCS entry points for an imported UUID / a pin / a derived id, export→import→validate at 0.7.5 and 0.7.6, overlays reaching every format, a bare -o for every format, the three validate commands over eleven document shapes, six hostile recorded fluidVersions, and a byte-diff of the no-op case against origin/main confirming only the three documented deltas.

Review

Three rounds, each adversarial and each one found defects the previous had missed.

  1. Four independent lenses (security, correctness, architecture, test strength) with every finding attacked by a skeptic before being reported: 18 raw, 5 surviving. The skeptics killed 13 by extracting origin/main and proving the behaviour byte-identical there. Confirmed: two HIGH and two MEDIUM, all one design hole in the new exposes[].contract.id — fixed in 2d702843.
  2. Gate run: detect-secrets failed on the security fix's own synthetic fixtures, and the branch was four commits behind main including security fix security: a contract could send any environment variable to a host it named #751, whose 291-line module was absent — a tag cut from that point would have published without it. Both fixed; main is merged.
  3. Live CLI pass: the collision guard still had nine uncovered doors, a pinned URN could not be imported back, the email field was never exported, and three claims in the changelog and schema text said more than the code did. All fixed in the last three commits; the overclaims were corrected rather than quietly reworded, and each is now pinned by a test so the claim and the code cannot drift apart again.

Security specifically: the version pin is untrusted input that selects a JSON Schema, and it was attacked with whitespace, case, traversal, non-string types and a 100 KB value — the allowlist is exact membership against BUNDLED_VERSIONS, the rejected value is logged as a %r argument so ANSI and newline injection are dead, and a preview version is never the fallback.

Documentation

forge_docs#167 is this PR's companion: the seven new 0.7.6 preview fields in reference/preview-fields.md, and a "Read this first if you consume the exported artifacts" section placed above the per-release checklists in upgrading.md, leading with the Kafka physicalType change and naming it as silent data corruption.

Two further docs PRs are open as related work rather than companions to this change:

  • forge_docs#165 — 152 stale "unreleased" markers, the overlay-merge and ODPS v1.1.0 passages, and a CI check so markers cannot go stale again.
  • forge_docs#166 — the DuckDB 1.x→2.x upgrade notes and the project-dotenv-trust page, both leftover 0.22.0 release tasks.

All three were measured to merge cleanly in any order, pairwise and all-three-in-one-tree.

Writing #167 also caught four places this PR's own prose said something the code does not — a field miscount, --lenient described as new when it is widened, a non-existent fluid generate odcs --validate in a schema description that would have published verbatim into the generated reference, and an argument against ODCS's json default that read as though forge never defaults. All four are corrected in e1b62a53.

One part cannot be written before the release exists, and it is a generator constraint rather than an omission: docs/reference/contract-0.7.6-preview.md is produced by scripts/gen_contract_reference.py, which refuses to run unless the installed data-product-forge equals the version pinned in docs/.vuepress/cli-version.json (today 0.18.1). So that page regenerates in a follow-up once this release's version is final and the pin is bumped — which is also what will finally give it engine: flink and trigger.type: continuous, four releases late.

Follow-ups, all pre-existing and proven against origin/main

Not fixed here, each with a reproduction: fluid validate prints a credential from the document it was handed (same root cause as the gate fix, different surface); a JSON integer over 4300 digits crashes two of the three validate commands; the .json read path has no size cap while .yaml is capped at 5 MB; a rejected fluidVersion is logged at full length, so a 2 MB document emits 4.2 MB of stderr; an output port with no type does not round-trip; the three validate commands still disagree on diagnostics for identical I/O conditions; the datamesh-manager ODCS builder's output has four schema errors unrelated to this change, and the builder should be retired in favour of OdcsProvider.render; and api/catalog_publication.py logs a contract-id collision rather than failing, because that module's documented invariant is never to raise.

Docs are a separate PR against Agenticstiger/forge_docs (#165), plus a cli-version.json bump and a regeneration of the 0.7.6 preview reference once this release's version is final.

fas89 added 24 commits October 26, 2026 19:55
…cuments were rejected

Bitol released ODPS v1.1.0 on 2026-09-08. The schema shipped here was taken
from the dev branch on 2026-07-26, before the release opened `SBOM` for
`authoritativeDefinitions`, `customProperties` and `tags`. Because `SBOM` is
`additionalProperties: false`, `fluid odps validate` rejected a document that
is valid against the published standard:

    Additional properties are not allowed ('tags' was unexpected)

Vendor the file verbatim from tag v1.1.0 — the only difference from upstream
is the added `$comment`, which now names the tag and says to refresh from a
tag, never from a branch. Nothing else in the schema changed: a structural
diff against the release reports 0 differing values.

The help text and the two code comments also stopped calling v1.1.0
pre-release. v1.0.0 stays the default on purpose, which the comment now
states as a compatibility decision rather than a wait for the release.

Forge's own exports were never affected — it writes no SBOM. The gap showed
when validating a document produced by another tool.
`fluid generate standard <contract> --format odcs|odps|odps-bitol --env prod`
loaded the base contract and printed nothing about it, so an overlay that
changes a Kafka topic or a Glue database produced the dev value under a prod
flag. Only the `odps-v4.1` path and a dead ImportError fallback applied the
overlay; `fluid odps export --env` and `fluid generate artifacts --env`
always did.

Route both through `load_contract_with_overlay`, the loader the sibling
paths already use.

Two defects in the same emitter, found while reading it:

* `-o out.yaml` (a bare file name, no directory) failed with
  `[Errno 2] No such file or directory: ''`, because `os.path.dirname("")`
  is `""` and `os.makedirs("")` raises. Two of the four call sites already
  guarded with `or "."`; the other two now do.
* The success line said `Bitol ODPS v1.0.0` whatever was written, so
  `ODPS_API_VERSION=v1.1.0` wrote a v1.1.0 document and announced v1.0.0.
  It now prints the apiVersion of the document on disk.
…schema

It asserted that apiVersion, kind, id, name and status were present and that kind was DataProduct, then printed a pass. A document with `status: 123` and an unknown top-level key exited 0, while `fluid odps validate` rejected the same file with "Additional properties are not allowed" and exited 1. Two commands, one standard, opposite answers.

It now calls the same code `fluid odps validate` calls — BitolOdpsProvider.validate_product, which picks the schema from the document's own apiVersion. No second validator and no new jsonschema import.

The read path was asymmetric too: only the YAML branch caught parse errors, so a malformed .json escaped as an uncaught JSONDecodeError and the CLI exited 2 as a crash. Opening and parsing now share one guard, and a missing path, a directory, non-UTF-8 bytes and a document nested past the recursion limit each exit 1 naming the file and the problem.

`odps-bitol export` and `odcs export` also accepted no --env at all, passing None to the loader while `odps export` honoured it. Both now take --env with the same help text.
…and --env

The earlier fix to this module guarded three of the four os.makedirs sites that derive a path from os.path.dirname(out); the fourth, in _export_odps_bitol's ImportError fallback, was still bare, so `-o out.yaml` with a bare file name raised FileNotFoundError: ''. The previous commit message claimed all of them were covered. They are now.

A test parses the module with ast and fails if any makedirs is ever handed an unguarded dirname again, so a fifth site cannot reintroduce it.
…rectory

The ODPS importer's pass-through bucket moved from metadata.odps_passthrough to extensions.odps, because metadata is additionalProperties:false in the FLUID schema and the old address could never validate. _load_directory still read the old address, so the lookup always yielded {} and every ODCS-only directory was classified as SHAPE_DIRECTORY.

A test now pins the marker's address at the writer as well as the detected shape, so the next relocation fails at the source instead of silently downgrading seed detection.
… could not say

All additive and all optional, so a contract that sets none of them validates and exports byte-identically. Field names are copied from Bitol ODCS v3.1.0 and the released ODPS v1.1.0 so import and export are a pass-through rather than a translation.

metadata.team {name, members[]} with $defs.teamMember {username required, name, role, email} — metadata.owner could express at most one person and carried no role, so catalogs requiring named roles skipped the product. Root description now accepts a string (unchanged, it becomes purpose) or {purpose, usage, limitations}. consumes[].contractId lets an author name the contract id of an upstream not built with FLUID, which the exporter already preferred but the closed schema rejected. exposes[].contract.id pins a contract id so it survives renaming the product or the expose. binding.location.host carries Kafka bootstrap servers, which the ODCS exporter already read while the schema only accepted `account`. binding.serialization {value, key} says how messages are serialized. A column may carry `quality`, which the exporter already replayed although the closed column definition rejected it.

Duplicate contract ids are refused in code, since JSON Schema cannot express uniqueness across a nested key, at the one chokepoint `fluid validate --offline` reaches: "contract id 'x' is used by exposes 'a' and 'b' — give each expose its own contract.id".
Both exporters derived team from metadata.owner, which yields at most one member: username set to the owner's email, no name, no role. The mapper already read owner.name, owner.role and owner.contacts[], but the schema rejected those keys, so a valid contract never reached that code.

With metadata.team.members set, both exporters emit {name, members} with every member and role passed through; without it, the owner-derived path is unchanged, which the tests pin as a whole team object. Imports place the roster at metadata.team instead of stranding it in the extensions bucket, and Bitol fields FLUID has no slot for stay in the pass-through and are merged back on export.

metadata.team wins over metadata.owner field by field, because it is the explicit, richer declaration; the reasoning sits at the decision point in the ODCS mapper.
… object description vanished

consumes[] became ODPS inputPorts[] with contractId set to the upstream PRODUCT id. But forge names that upstream port's own ODCS "<productId>.<exposeId>", so the lineage edge pointed at a contract no catalog has. It now defaults to that expose-scoped id — the one forge itself mints — while an explicit consumes[].contractId still wins. versionConstraint was read and then dropped; Bitol's InputPort has no such field, so it rides in a customProperties entry, and the emitted document is validated against both vendored ODPS schemas.

The ODPS exporter wrapped only a string description, so given the object the ODCS exporter has accepted since #585 it wrote no description at all. It now passes the object through.

An explicit exposes[].contract.id is used verbatim as the ODPS output port contractId and as the per-port ODCS id. Both resolve it through one helper, so the fragments-mode invariant that the two are the same string holds by construction rather than by two call sites agreeing. A port imported with a foreign or UUID contractId re-exports verbatim instead of being re-derived.
ODCS defines a Kafka server's `format` as the format of the messages — json, avro, protobuf or xml — and forge lifted it from binding.format, so every exported Kafka server claimed `kafka_topic` and a consumer could not tell whether values were Avro or JSON. It now comes from binding.serialization.value, with the registry-carried spellings mapped to the wire format they actually are: avro-confluent to avro, json-confluent to json.

binding.location.host is now a declared field, so the bootstrap servers no longer have to be smuggled through `account`; that fallback is kept, with the reason in a comment, and the ${KAFKA_BOOTSTRAP_SERVERS} placeholder still stands in when neither is set. A contract that declares no serialization exports exactly as before, so no existing export changes.
A per-port export named each ODCS after the product, so a product with several exposes emitted several contracts sharing one name, and tools that key files or registry subjects on the contract name collided. ODCS puts identity on `id`, which was already per-expose, so the collision was invisible to id-keyed consumers and silent for everyone else.

Each per-port contract is now named after its port — the expose title, falling back to its exposeId — and the product name is kept in a `fluidProductName` custom property, following the entry shape fluidExtras already uses. Not ODCS `dataProduct`: Bitol deprecated that in v3.1.0 to keep products in ODPS and contracts in ODCS.

This changes the exported name for a single-expose product too, whenever the port's title differs from the product name. That is deliberate: a name that depended on how many siblings a port has would change again the day a second port was added, and schema[0].name already carried the exposeId, so the document now agrees with itself. The ODCS id is unchanged in every case, and whole-contract exports are untouched.
…lidator rejects

`fluid odps export` then `fluid odps import` reported success and wrote a document that `fluid validate --offline` failed with ten errors: no fluidVersion, kind, id or name at the root; unexpected root keys contract, expects and owner; an expose with id and odps_passthrough but no kind; a metadata block holding description, domain, name, status and version and no owner. The help claimed it "Always emits one validated FLUID contract".

The importer now normalises, reusing the ODCS leg's own helpers rather than restating them, and validates before writing: an import that cannot produce a valid contract names the first problem, writes nothing and exits 1, while --lenient writes it anyway behind a warning. The pass-through bucket moves to root extensions.odps, the one object the FLUID schema declares open — metadata is additionalProperties:false, so metadata.odps_passthrough was itself three of those ten errors. Nothing is dropped: anything with no schema home rides in the bucket and is replayed verbatim on export, so the product document still round-trips byte for byte.

The imported contract declares the fluidVersion the source document recorded, falling back to the latest bundled STABLE version when it records none. Without that, every import pinned to 0.7.5 and the new 0.7.6 fields could be exported but never imported back. The recorded version selects a JSON Schema and comes from a document that may be foreign, so it is honoured only when it is a string in BUNDLED_VERSIONS; anything else falls back to stable and logs a warning naming the rejected value, and preview is never the fallback. Only the sibling ODCS records the version, so a bundle import can honour it and a bare product import cannot — that is now a test.

Also in cli/odps.py, the `--spec odps-4.1` validate branch caught only OSError and JSONDecodeError, so non-UTF-8 bytes and a document nested past the recursion limit exited 2 as unhandled exceptions. Both now exit 1 with a diagnosis, in the same message shapes the odps-bitol command settled on.
…o bigint shrank to 32 bits

fluid_to_physical falls back to the ODCS logicalType when a binding's provider has no physical-type table, and Kafka had none. FLUID `bigint` maps to the logicalType `integer`, and ODCS's own schema defaults an unqualified `integer` to `i32` — so the exported column declared 32 bits. A tool deriving an Avro schema from that contract reads Avro `int` while a producer such as Flink BIGINT writes a 64-bit long, and the registered schema and the data disagree with nothing to signal it.

There is now a Kafka/Avro table: bigint and long export as `long`, int stays `int`, and a decimal exports as `bytes` with its precision and scale in customProperties, which is where an ODCS-to-Avro consumer reads them. It covers all 79 FLUID types rather than the handful that surfaced the bug, derived from the Avro 1.12 specification and from datacontract-cli's own avro_exporter.py, whose to_avro_type reproduces the defect. Providers that already had a table are unchanged, pinned by a test. The test replays that consumer's mapping on the exported property and parses the result with the real Apache Avro library, so the claim is checked against a tool rather than by eye.

mappers/schema.py also gains the include_quality parameter threading that the next commit's quality switch needs; the two changes share this file.
…nt invalid

Three defects, all found by feeding forge's exports to a real DQ engine.

Derived rules carried no id. The exporter synthesises not-null rules from `required` and uniqueness rules from primary-key tags; ODCS rules carry an id, and an engine that labels results by rule id rejected the whole contract, so one derived rule made the export unusable there. They now carry a deterministic `<column>-<metric>` id, sanitised to the charset ODCS's StableId declares and unique within its schema object, so two renders are byte-identical. Rules forge did not write are still replayed verbatim.

Rules the author wrote never arrived. exposes[].contract.dq.rules and exposes[].contract.quality were not mapped at all — the extras pass parked them in fluidExtras, so a consumer running ODCS quality rules saw only what forge derived and none of the author's own. They now attach to the property or object their selector names, using ODCS's own metric and operator names. A rule type with no executable ODCS form is logged by name with the reason and still carried in fluidExtras; silence was the bug. This pass needs the schema mapper to have run first, so that ordering is now load-bearing — the package docstring said the opposite, and a test pins it, since the order was enforced only by a list literal.

The contract-level block was written to the ODCS root, which is additionalProperties:false and has no `quality`, so any contract reaching that branch exported a document failing its own validation with two errors. It is reachable by two public routes, both exercised. It now rides in a `fluidContractQuality` custom property, following the fluidExtras naming, and the import reads it back, so the export stays a fixed point even with ODCS_FLUID_EXTRAS=false.

Finally, ODCS_INCLUDE_QUALITY=false and include_quality_checks did not suppress derived per-column rules, only the contract-level block — so turning quality off still emitted a rule on every column. Both now suppress every quality surface forge derives. The object-level pass-through from an imported document is deliberately not gated: it is verbatim round-trip plumbing, and gating it would make an imported contract lossy on re-export.
This provider builds an ODCS contract directly instead of going through the mapper pipeline, so it set `dataProduct`, which ODCS deprecated in v3.1.0 to keep products in ODPS and contracts in ODCS. It now carries the product name in the same fluidProductName custom property the mapper writes, importing the constant rather than retyping it.

Two things found while confirming it, both left alone and worth a card. The deprecation is not marked in the vendored v3.1.0 schema — other deprecated fields there carry "deprecated": true, and this one is only marked in the official 3.2.0 schema — so the old output was still schema-valid and validity does not distinguish before from after. And the builder's output has four pre-existing schema errors unrelated to this change, including a server with no required `server` id and the binding location's `table` key leaking onto the server. The deeper fix is to delete the builder and call OdcsProvider().render(expose_id=...), which the publish flow already does; that needs a decision about what publish_data_contract means for a multi-expose product, so it is a follow-up rather than a release-eve refactor.
Twenty fixed entries, the six new 0.7.6 preview fields, and a Changed section that leads with the warning that exported artifacts change shape in this release — the per-port ODCS name, the input-port contractId, the Kafka server format and column physicalType, quality rule ids, and the relocated import pass-through bucket are all values a consumer may have baked in.
…stderr

`fluid odps import`'s validate-before-write gate reports the first schema error, and a jsonschema error quotes the value that failed. That value comes from a document the command was handed, so a contract whose rejected field holds a DSN had its password printed in full: importing a bundle whose root id is `snowflake://svc_etl:S3cr3t-Pa55w0rd@acme-prod.snowflakecomputing.com/?token=ghp_...` wrote the password to stderr while masking the GitHub token in the same string — the repo's redactor knows that shape and simply was not on this path, which went through the console's weaker key=value sanitiser instead.

The gate's message now goes through observability.secret_redactor.redact_secret_text, imported function-locally so the `fluid --help` cold path is unaffected (tests/perf/test_startup_budget.py still passes). Verified end to end from outside the worktree, with fluid_build.__file__ asserted on both trees: the password appears once in the import output without the redaction and not at all with it. A malformed id with nothing credential-shaped in it is still quoted, so the diagnosis is not swallowed.

The underlying weakness is older than this branch — `fluid validate` prints the same bytes on main, which is why an adversarial review classed it pre-existing — but this gate is a new surface that leaked it, and the fix belongs with the surface. The `fluid validate` path is left alone deliberately and is worth its own change; four of the nine new tests fail without this fix, and the two token-shaped cases pass either way because that family was already masked.
…'s schema

An adversarial review of this branch found four defects that were all one hole in the `exposes[].contract.id` this branch added: the id had several independent resolution sites and a uniqueness guard that only compared pinned ids against other pinned ids.

The data loss. A pin that happened to equal a sibling's DERIVED `<productId>.<exposeId>` passed `fluid validate` with a tick, and the export then wrote ONE per-port ODCS file for TWO output ports: the losing port's schema was absent from the bundle entirely and both ports resolved to the survivor's contract. `fluid odps validate` was green over the result and `fluid generate artifacts` sealed it under a MANIFEST merkle root. On the previous release the same input is schema-rejected, so this branch introduced it.

The guard now seeds every UN-PINNED expose's derived id before walking the pinned ones, and reports a pin that collides with a derived id in its own message — telling the user to change the pin or rename the expose, since the un-pinned sibling has no contract.id to change. Pinned exposes are deliberately not seeded: a pinned expose never publishes its derived string, so a sibling pinning to it collides with nothing, and a test exports that contract to prove both schemas really survive.

Because `fluid odps export` and `fluid generate artifacts` never run the validate step, `BitolOdpsProvider.render` now refuses rather than overwrites, naming both ports and the shared id. There is no escape hatch: silently losing a schema is worse than failing.

One resolution point. `util/contract.py::expose_contract_id` takes the whole contract and reads the pin, the inline pass-through, and the root `extensions.odps` where an import actually leaves the id — which is what fixes the second defect: four per-expose ODCS entry points never rehydrated that bucket, so an imported contract exported ODPS `3f2a9c1e-...` and standalone ODCS `shop.catalog.items`. All five entry points now answer the same string, fixed by one line in `_scope_to_expose`; forge_copilot_runtime and api/catalog_publication needed no edit, each verified rather than assumed. The derived convention is spelled exactly once, so validate cannot derive a different string from the exporters.

Third defect: `fluid publish` to Data Mesh Manager re-derived the id with a bare f-string and PUT the ODCS body under an id the product did not reference. Both payload shapes now route through the shared resolver, including the legacy DPS `dataContractId` branch — fixing only the ODPS branch would have introduced the mirror-image mismatch.

Marked breaking: a contract that pins an id colliding with a sibling's derived id used to export, and now fails validate and export. That export was lossy, so the failure is the fix.
…alised unnoticed

A review showed each of these could be replaced with a stub while all 435 tests in the affected files stayed green, so none of them was actually pinned. Two were user-visible when neutralised: an edited `lifecycle.state` silently re-published the stale ODPS status, and the whole port-type-to-platform table collapsed to `other`.

38 tests, each discriminated by neutralising the production behaviour in a scratch copy — nine neutralisations including both arms of the two conditionals. Two of the review's descriptions were wrong and the tests pin what the code actually does: the sibling-version rule lives in `_declared_fluid_version` with TWO collection sites feeding it, both now covered in both sibling orders plus a case proving it picks the highest ACCEPTED rather than the highest recorded; and the port-type table is only consulted for a port whose contract did not resolve, so the tests drive an unresolved port under `--lenient`, which is the real third-party-bundle case.
Also corrects a claim this branch made and the code did not support: the fragments-mode invariant is held by the export refusing a contract that would break it, not by two call sites resolving through one helper.
…scanner

detect-secrets flagged seven synthetic strings the previous commit added — the example DSN in two docstrings, the test's password constant, and four fixture credentials — so CI's Secret scan step would have failed the pull request. The repo's remedy for exactly this is an inline `# pragma: allowlist secret`, used 343 times elsewhere and absent from both new files.

The five code-level fixtures now carry the pragma. The two docstring hits are reworded to `snowflake://user:<secret>@host` instead, because a trailing pragma comment inside a docstring renders as prose. The baseline's line numbers are refreshed, which the hook asks for. Verified against both this branch's baseline and origin/main's: exit 0 where it was exit 1 with seven findings.
…n it covered

The earlier fix put the refusal in `BitolOdpsProvider.render`, so it covered `fluid validate` and the two ODPS export paths and nothing else. A live CLI pass found the same silent data loss still reachable: `fluid generate standard --format odcs` wrote two ODCS documents BOTH carrying `id: gate.coll.port_b`, `generate artifacts --emit odcs` did the same and minted a MANIFEST over them, and the Data Mesh Manager preview and publish paths produced two PUT urls under one id. Two ports under one contract id publish two different schemas under a single identity, so a consumer resolving it gets one of the two arbitrarily and the other port's schema is lost.

A collision is a property of the CONTRACT, not of one exporter, so the rule now lives once in `util/contract.py` beside the single id resolution point and every door calls it. Twelve doors are enumerated and covered by one parametrised test table, so a thirteenth added later is a test away from being caught rather than a defect away. `cli/validate.py` delegates to the same function instead of keeping its own copy, which is why its message now carries the consequence clause the other doors already used.

Two doors behave deliberately differently. `api/catalog_publication.py` logs the collision at ERROR and renders no ODCS rather than raising, because that module documents an invariant that the canonical layer never raises on bad input; a caller that needs failure should run `fluid validate`, which rejects the same contract with the same message. And a pair of exposes sharing one `exposeId` is left to each exporter's duplicate-name guard, which says something more useful than a contract-id collision would — a regression caught by the existing suite, not by review.

A refused export now also leaves no empty output directory: `generate artifacts` created its subdirectory before the check, where `fluid odps export --out-dir` left nothing. The pre-emptive mkdir is gone from both emitters and from `fluid odcs export --per-port`; the providers' own writers create the directory as they write, so this covers every refusal rather than only this one.

Shares providers/odps_standard/provider.py with the description promotion in the next commit.
… not be imported back

`exposes[].contract.id` exists so a catalog with its own id format can supply one, so it carries no pattern — and `urn:acme:contract:hourly-orders:v1` validated, exported into an ODCS `id` correctly, and then failed `fluid odcs import`, which reconstructs the FLUID root id from that field. Constraining the field would have been the smaller change and would have gutted the feature, since a catalog-supplied id is usually exactly this shape.

The importer now uses the ODCS id verbatim as the FLUID root id whenever it matches that schema version's own `identifier` pattern — every derived id and most hand-written ones, so nothing changes for them — and otherwise slugs it, keeping the original verbatim so a re-export is byte-identical. The pattern is read from the bundled schema rather than restated, because 0.7.1 and 0.7.2 accept lowercase only.

Two claims this branch made and the code did not support, both now true rather than reworded. A column's `quality` survives an ODPS round trip, which the schema already asserted was what made an imported ODCS column lossless. And a structured root description reaches the export again: the ODPS product mapper hands over `purpose` alone and parks the object, because the root description is a string before 0.7.6 and that mapper has no declared version to consult, so `_to_fluid_document` — the one place that knows — promotes it back when the schema accepts an object, asking the schema rather than comparing against a version string. A bundle recording no version still roots `purpose`, since rooting the object there would write a contract the import's own gate refuses.
The 0.7.6 `teamMember` declares `email` and both exporters dropped it, so an author who filled it in got silence — and the ODCS importer read it back, so it could enter a contract but never leave one.

An earlier agent had decided not to export it, reasoning that Bitol's TeamMember is closed. That diagnosis is right: `additionalProperties: false` with no `email` in ODCS v3.1.0 and in both vendored ODPS versions, so emitting the field fails all three. But "closed" was over-read — all three allow `customProperties` — so the address now ships as the member's `fluidEmail` custom property, the spec's own escape hatch and the same convention as `fluidProductName`, `fluidExtras` and `fluidContractQuality". It reaches both exporters, where before ODPS carried it nowhere and ODCS only inside the round-trip blob that `ODCS_FLUID_EXTRAS=false` switches off.

Also the third changelog overclaim this branch has produced. The Changed section said rules declared on a contract now appear in the exported ODCS; that holds for the four `dq.rules` types ODCS can express, while the other four have no executable form and are logged and left in `fluidExtras` — which the Fixed entry already said, so the summary contradicted its own detail. Corrected there, and pinned by a test rather than by prose.
@github-actions github-actions Bot added provider Changes or requests related to providers security Security-related changes or reports cli Changes to the CLI surface or implementation tests Test coverage or test infrastructure changes needs-docs Pull request needs a linked docs update or justification labels Oct 27, 2026
@github-actions

Copy link
Copy Markdown

📄 Documentation Reminder

This PR appears to be missing a documentation reference. Our docs live in a separate repo.

Please update the PR description with one of:

  • Link a docs PR — check the "Docs PR linked" box and paste the URL
  • Mark as no docs needed — check "No docs needed" with a justification
  • Acknowledge docs TODO — check "Docs TODO" and create the docs PR before merge

See the Contributing Guide for details.

fas89 added 3 commits October 27, 2026 03:18
…and 3.14 went red

CI passed on 3.10, 3.11 and 3.12 and failed on 3.13 and 3.14. CPython 3.13 improved the message for a trailing comma: "Expecting property name enclosed in double quotes" became "Illegal trailing comma before end of object". Both tests asserted the old text, so they were pinning the interpreter's wording rather than the behaviour the guard adds.

They now assert what the guard is actually for: exit 1 rather than the exit 2 traceback it replaced, the file named, the parser's own position relayed ("line 1 column", which every version emits), and no "Unexpected error" or "unhandled exception" in the output. Checked against the real 3.13 message from the failing job, not against a guess, and still green on 3.12.

The YAML sibling at line 212 asserts PyYAML's "while parsing a flow sequence", which is library rather than interpreter text and passed on all five legs; left alone, but it is the same brittleness class and will bite on a PyYAML bump.
Found by the companion docs PR checking my summaries against the code rather than taking them as source, which is what I had asked it to do.

The `### Added` heading counted six optional fields and listed seven — the column `quality` the quality commit added was never counted in.

`--lenient` is not new. It is in 0.18.1 and on main, where it downgrades output-port resolution failures to warnings; this branch WIDENS it to mean "write it anyway". The entry read as though the flag arrived with the import gate.

The column `quality` schema description cited `fluid generate odcs --validate`, which does not exist: `generate` has no `odcs` subcommand and `generate standard` has no `--validate`. The real command is `fluid odcs validate`. This one would have SHIPPED — `gen_contract_reference.py` renders schema descriptions verbatim into the published 0.7.6 reference, so the wrong command would have been documented as fact once the pin moves. While there, every one of the eight `fluid …` strings in that schema was run against the real CLI; the other seven resolve.

The Kafka bullet argued against ODCS's `json` default as though forge never defaults. `details.setdefault("format", "json")` is on main too, for a Kafka binding that declares no format at all — pre-existing, unchanged, and not what the new field replaces. Said so, so a reader diffing exports is not surprised. The no-op claim is unaffected, since the default is not new.
…nned the wrong thing

Second round of the same mistake. Both tests feed 100,000 opening brackets and asserted the words "nested too deeply", which come from the RecursionError arm. Verified locally on both interpreters rather than inferred from the CI text: on 3.12 `json.loads` raises "maximum recursion depth exceeded while decoding a JSON array"; on 3.14 the same document PARSES, because that decoder is not recursive, and the failure is a plain JSONDecodeError on running out of input ("Expecting value: line 1 column 100001"). So the test's premise — a recursion limit — does not exist on 3.14.

They now assert the outcome the guard is responsible for: exit 1, the file named, and no "Unexpected error" or "unhandled exception" banner. Renamed to say so, since "reports_json_nested_past_the_recursion_limit" described an interpreter detail rather than a behaviour. The RecursionError arm stays in the code; the interpreters that take it still need it.

Also swept both files for every remaining third-party message assertion rather than waiting for a third round. PyYAML's "while parsing a flow sequence" got the same treatment — a library's wording is not this code's contract. Kept the jsonschema "Additional properties are not allowed" assertion, which is what proves the schema actually ran, and our own messages. The changelog and a module docstring said "nested past the recursion limit" as though it were universal; both now name the per-interpreter split.

Verified on a real 3.14.6 environment, with fluid_build.__file__ asserted: 51 passed. Still 51 passed on 3.12.
@fas89
fas89 merged commit d31a19b into main Oct 27, 2026
37 checks passed
@fas89
fas89 deleted the fix/odps-export-import-defects branch October 27, 2026 03:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli Changes to the CLI surface or implementation provider Changes or requests related to providers security Security-related changes or reports tests Test coverage or test infrastructure changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant