Skip to content

Commit c96cbb1

Browse files
flyersworderclaude
andauthored
refactor: convention-default helpers live in base.py, not yaml_source (#77)
OssieSource had been importing _parse_convention_default and _apply_convention_default -- two private functions -- across from yaml_source.py. Every other helper shared between the sources lives in base.py and is imported by both: validate_decompositions, validate_drill_by, parse_review_date, jsonify_extras, build_relationship_index. Two things made the old placement the odd one out. Both helpers operate purely on MetricDefinition / Decomposition and touch nothing YAML-specific, and both had to import VALID_CONVENTIONS and _CROSS_TERM_OPERATORS back out of base.py to do their work. After the move yaml_source.py needs neither constant for anything else -- it had no other consumer of them -- which is the clearest evidence the vocabulary they enforce already lived where the functions belonged. parse_review_date's docstring states the principle the move follows: it is shared rather than per-source because every format reaching the same field reaches the same arithmetic, "so a second copy of this would be a second chance to get the subclass ordering wrong." Both sources resolve a house default onto the same Decomposition.convention field, and fold_into-is-not-a-valid-default is exactly the rule that drifts if it is ever copied. No behavior change. Both source paths keep their existing tests, and verified directly that a YAML default still resolves (split_evenly), an Ossie vendor-block default still resolves (explicit), a fold_into source-level default is still rejected, and the pinned round-trip digest is unmoved. 1078 passing. No version bump: both functions are private, so nothing on the public surface changed. Rides along with the 0.43.1 already on main. Closes #74 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent a3decb0 commit c96cbb1

4 files changed

Lines changed: 59 additions & 55 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ All notable changes to this project will be documented in this file.
1010

1111
### Internal
1212

13+
- **The convention-default helpers moved to `semantic/base.py`.** `OssieSource` had been importing `_parse_convention_default` / `_apply_convention_default` — two *private* functions — across from `yaml_source.py`, while every other helper shared between sources (`validate_decompositions`, `validate_drill_by`, `parse_review_date`, `jsonify_extras`, `build_relationship_index`) lives in `base.py`. The giveaway was that both had to import `VALID_CONVENTIONS` and `_CROSS_TERM_OPERATORS` back out of `base.py` to do their work, and that those two constants had no other consumer in `yaml_source.py` — the vocabulary they enforce already lived where the functions belonged. `parse_review_date`'s own docstring states the principle: a second copy is a second chance to get the rule wrong, and both sources resolve a default onto the same `Decomposition.convention` field. No behavior change; both source paths keep their existing tests. ([#74](https://github.com/flyersworder/agentic-data-contracts/issues/74))
1314
- **The pinned round-trip digest now covers metric serialization.** `_PINNED_ROUNDTRIP_DIGEST` is the repo's broadest guard against canonical-bytes drift, but it was computed over a semantic source with no `metrics:` key at all, so `_dump_metric` was never called and it could not see any change to metric serialization. That is worse than a missing test, because the assertion passed either way: every optional field added to `_dump_metric` since the pin was written could have been made unconditional — moving the digest of every real contract and invalidating every published ARD attestation — with the test still green. The fixture now carries a metric exercising every optional field the dump writes plus two leaves for the omit-when-empty branches, verified by mutation (removing the `if m.decompositions:` guard now fails the pin), and a companion test asserts the pinned contract's canonical bytes still carry those fields so the coverage cannot be silently narrowed again. No shipped code changed. ([#73](https://github.com/flyersworder/agentic-data-contracts/issues/73))
1415

1516
## [0.43.0] - 2026-08-18

src/agentic_data_contracts/semantic/base.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,60 @@ def visit(node: str, stack: list[str]) -> None:
331331
visit(node, [])
332332

333333

334+
# Shared by every source that can carry a house convention, for the reason
335+
# ``parse_review_date`` records just above: both ``YamlSource`` and
336+
# ``OssieSource`` resolve a default onto the same ``Decomposition.convention``
337+
# field, and a second copy would be a second chance to get the
338+
# fold_into-is-not-a-default rule wrong.
339+
def _parse_convention_default(raw: Any) -> str | None:
340+
"""Read and validate the source-level ``decomposition_convention`` block.
341+
342+
``fold_into`` is rejected: it names an operand, and no operand name is
343+
meaningful across metrics. Declaring it source-wide is always a mistake.
344+
"""
345+
if raw is None:
346+
return None
347+
if not isinstance(raw, dict):
348+
raise ValueError(
349+
"decomposition_convention must be a mapping with a 'convention' key,"
350+
f" got {type(raw).__name__}"
351+
)
352+
convention = raw.get("convention")
353+
if convention is None:
354+
raise ValueError("decomposition_convention must set 'convention'")
355+
if convention not in VALID_CONVENTIONS:
356+
raise ValueError(
357+
f"decomposition_convention has unknown attribution convention"
358+
f" {convention!r}; expected one of {sorted(VALID_CONVENTIONS)}"
359+
)
360+
if convention == "fold_into":
361+
raise ValueError(
362+
"convention 'fold_into' cannot be a source-level default: it names"
363+
" an operand, and no operand name is meaningful across metrics."
364+
" Declare it per decomposition."
365+
)
366+
return convention
367+
368+
369+
def _apply_convention_default(
370+
metrics: list[MetricDefinition], default: str | None
371+
) -> None:
372+
"""Stamp *default* onto every cross-term decomposition that declares none.
373+
374+
Resolved at load rather than carried, so the effective value survives
375+
``freeze_semantic_source`` (which re-serializes from parsed objects) and a
376+
frozen contract states its convention outright instead of leaving a
377+
consumer to re-derive it. Linear operators are skipped: a convention on
378+
them fails ``validate_decompositions``.
379+
"""
380+
if default is None:
381+
return
382+
for metric in metrics:
383+
for decomp in metric.decompositions:
384+
if decomp.convention is None and decomp.operator in _CROSS_TERM_OPERATORS:
385+
decomp.convention = default
386+
387+
334388
def validate_drill_by(
335389
metrics: list[MetricDefinition],
336390
table_schemas: dict[str, TableSchema],

src/agentic_data_contracts/semantic/ossie.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,17 +46,15 @@
4646
MetricDefinition,
4747
MetricImpact,
4848
Relationship,
49+
_apply_convention_default,
50+
_parse_convention_default,
4951
build_relationship_index,
5052
fuzzy_search_metrics,
5153
jsonify_extras,
5254
parse_review_date,
5355
validate_decompositions,
5456
validate_drill_by,
5557
)
56-
from agentic_data_contracts.semantic.yaml_source import (
57-
_apply_convention_default,
58-
_parse_convention_default,
59-
)
6058

6159
logger = logging.getLogger(__name__)
6260

src/agentic_data_contracts/semantic/yaml_source.py

Lines changed: 2 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,13 @@
1111

1212
from agentic_data_contracts.adapters.base import Column, TableSchema
1313
from agentic_data_contracts.semantic.base import (
14-
_CROSS_TERM_OPERATORS,
15-
VALID_CONVENTIONS,
1614
Decomposition,
1715
DrillDimension,
1816
MetricDefinition,
1917
MetricImpact,
2018
Relationship,
19+
_apply_convention_default,
20+
_parse_convention_default,
2121
build_relationship_index,
2222
fuzzy_search_metrics,
2323
jsonify_extras,
@@ -78,55 +78,6 @@ def _apply_extras_policy(
7878
)
7979

8080

81-
def _parse_convention_default(raw: Any) -> str | None:
82-
"""Read and validate the source-level ``decomposition_convention`` block.
83-
84-
``fold_into`` is rejected: it names an operand, and no operand name is
85-
meaningful across metrics. Declaring it source-wide is always a mistake.
86-
"""
87-
if raw is None:
88-
return None
89-
if not isinstance(raw, dict):
90-
raise ValueError(
91-
"decomposition_convention must be a mapping with a 'convention' key,"
92-
f" got {type(raw).__name__}"
93-
)
94-
convention = raw.get("convention")
95-
if convention is None:
96-
raise ValueError("decomposition_convention must set 'convention'")
97-
if convention not in VALID_CONVENTIONS:
98-
raise ValueError(
99-
f"decomposition_convention has unknown attribution convention"
100-
f" {convention!r}; expected one of {sorted(VALID_CONVENTIONS)}"
101-
)
102-
if convention == "fold_into":
103-
raise ValueError(
104-
"convention 'fold_into' cannot be a source-level default: it names"
105-
" an operand, and no operand name is meaningful across metrics."
106-
" Declare it per decomposition."
107-
)
108-
return convention
109-
110-
111-
def _apply_convention_default(
112-
metrics: list[MetricDefinition], default: str | None
113-
) -> None:
114-
"""Stamp *default* onto every cross-term decomposition that declares none.
115-
116-
Resolved at load rather than carried, so the effective value survives
117-
``freeze_semantic_source`` (which re-serializes from parsed objects) and a
118-
frozen contract states its convention outright instead of leaving a
119-
consumer to re-derive it. Linear operators are skipped: a convention on
120-
them fails ``validate_decompositions``.
121-
"""
122-
if default is None:
123-
return
124-
for metric in metrics:
125-
for decomp in metric.decompositions:
126-
if decomp.convention is None and decomp.operator in _CROSS_TERM_OPERATORS:
127-
decomp.convention = default
128-
129-
13081
class YamlSource:
13182
"""Loads metric and table definitions from a YAML file."""
13283

0 commit comments

Comments
 (0)