msi_to_osi silently drops offset_window / offset_to_grain on DERIVED metric inputs, producing constant-zero expressions

Open Beginner friendly
#302 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
82/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
python
Domain
tooling

Research direction

Start in converters/dbt/src/ossie_dbt/msi_to_osi.py, especially _resolve_derived and the existing CUMULATIVE_SEMANTICS_LOSS handling. Reproduce the year-over-year case from converters/dbt/ and add coverage in tests/test_msi_to_osi.py for an input with offset_window or offset_to_grain. Done means the lost offset is reported as a ConverterIssue and the test verifies the issue.

Written by the indexing model from the issue text.

Description

MSIToOSIConverter._resolve_derived ignores offset_window and offset_to_grain on DERIVED metric inputs. The offset is dropped silently — no ConverterIssue is emitted — so a period-over-period metric converts into an expression that is algebraically constant, and looks like a successful conversion.

Root cause

converters/dbt/src/ossie_dbt/msi_to_osi.py:337-351, _resolve_derived:

expr = metric.type_params.expr or ""
for input_metric in metric.type_params.metrics or []:
    ref = input_metric.alias if input_metric.alias else input_metric.name
    dep_metric = self._lookup_metric(metric_index, input_metric.name, f"DERIVED metric '{metric.name}'")
    input_filter = _merge_filter_sqls(filter_sql, _collect_filter_sql(input_metric.filter))
    resolved = self._resolve_metric_expression(dep_metric, metric_index, cache, input_filter)
    if dep_metric.type in (MetricType.DERIVED, MetricType.RATIO):
        resolved = f"({resolved})"
    expr = re.sub(rf"\b{re.escape(ref)}\b", resolved, expr)

input_metric.alias, .name and .filter are all read; input_metric.offset_window and input_metric.offset_to_grain are not read anywhere in the file. Two inputs referencing the same metric — one current, one offset — therefore resolve to byte-identical SQL, and the difference between them collapses to zero.

The year-over-year shape this breaks is the canonical use of offset_window, and the one in the MetricFlow docs.

Repro (HEAD 88e0011148283302c9a04cd0287e00e0b9d87354), using the repo's own test helpers:

import sys
sys.path.insert(0, "src"); sys.path.insert(0, "."); sys.path.insert(0, "../../python/src")

from ossie_dbt.msi_to_osi import MSIToOSIConverter
from tests.helpers import _manifest, _simple_metric
from metricflow_semantic_interfaces.implementations.elements.measure import PydanticMeasure
from metricflow_semantic_interfaces.implementations.metric import (
    PydanticMetric, PydanticMetricInput, PydanticMetricTimeWindow, PydanticMetricTypeParams,
)
from metricflow_semantic_interfaces.test_utils import default_meta, semantic_model_with_guaranteed_meta
from metricflow_semantic_interfaces.type_enums import AggregationType, MetricType

sm = semantic_model_with_guaranteed_meta(
    name="bookings",
    measures=[PydanticMeasure(name="booking_count", agg=AggregationType.SUM, expr="1")],
)
base = _simple_metric("booking_count", "booking_count")

yoy = PydanticMetric(
    name="bookings_yoy_growth",
    description=None,
    type=MetricType.DERIVED,
    type_params=PydanticMetricTypeParams(
        expr="(this_year - last_year) / NULLIF(last_year, 0) * 100",
        metrics=[
            PydanticMetricInput(name="booking_count", alias="this_year"),
            PydanticMetricInput(name="booking_count", alias="last_year",
                                offset_window=PydanticMetricTimeWindow(count=1, granularity="year")),
        ],
    ),
    filter=None, metadata=default_meta(), config=None,
)

res = MSIToOSIConverter().convert(_manifest(semantic_models=[sm], metrics=[base, yoy]))
out = [m for m in res.output.semantic_model[0].metrics if m.name == "bookings_yoy_growth"][0]
print("EXPRESSION:", out.expression.dialects[0].expression)
print("ISSUES:", res.issues)

Run from converters/dbt/.

Observed:

EXPRESSION: (SUM(1) - SUM(1)) / NULLIF(SUM(1), 0) * 100
ISSUES: []

The expression is algebraically 0 for every input row, and nothing signals that anything was lost.

Expected: either the offset represented in the output, or — since Ossie expressions have no window semantics to represent it with — a ConverterIssue reporting the loss, as CUMULATIVE metrics already do.

Why this is worse than the cumulative case

CUMULATIVE metrics already emit ConverterIssueType.CUMULATIVE_SEMANTICS_LOSS (msi_to_osi.py:95-98), so a consumer can see the semantics didn't survive and decide what to do. offset_window has no equivalent, so the failure mode is different in kind: the cumulative metric comes out approximate, the offset metric comes out wrong, and both look identical to anything downstream.

Concretely, converting through converters/snowflake and creating the semantic view in Snowflake produces a metric that runs successfully and returns 0 for every period — indistinguishable from genuinely flat year-on-year growth. It is the only failure I hit in an end-to-end dbt → Ossie → Snowflake conversion that produced a plausible wrong number rather than an error or an obvious gap.

Suggested fix

At minimum, emit a ConverterIssue when an input metric carries offset_window or offset_to_grain, matching the existing CUMULATIVE_SEMANTICS_LOSS pattern — e.g. OFFSET_SEMANTICS_LOSS. That turns a silent wrong answer into a visible gap, and lets consumers gate on it.

Beyond that it's a spec question rather than a converter one: representing a time-offset input needs something in the Ossie spec to carry it, which I don't think exists today (related to #290's shape, though not the same issue). Happy to raise that separately if it's useful.

tests/test_msi_to_osi.py has DERIVED coverage but no case where an input carries offset_window, which is why this isn't caught.

Happy to send a PR for the ConverterIssue part.

Dominant language
Python
Stars
2.2k
Forks
280
Avg merge
2d 7h
Merged PRs (30d)
32

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from apache/ossie

All issues in apache/ossie

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.