[tomasmontielp]

Fixes open-metadata/openmetadata-collate#8310

Describe your changes:

A metric's concept context (GET /metrics/name/{fqn}/context) carried only its expression and bindings, so the agent computing it never saw the rest of what defines the number:

• filters: the SQL conditions the metric is defined under (status = 'paid'). Without them the agent computes the metric unfiltered, and the number is wrong.
• granularity: the time grain it is reported at.
• unit: the unit it is measured in; the custom unit when unitOfMeasurement is OTHER. QA found a DOLLARS metric whose $ in the answer was only the model's guess.

conceptContext gains the three optional fields; ConceptContextBuilder fills them for a metric (each omitted when the metric does not declare it), and ConceptContextMarkdown renders them under the definition. Type, dimensions, measures and the variant hierarchy stay out: they do not change the number.

Companion: open-metadata/ai-platform renders the same fields in the SQL and worker concept text (open-metadata/ai-platform#2061); it builds against this one, so this merges first.

Type of change:

• Improvement

Verification

• New tests: ConceptContextBuilderTest.metricCarriesWhatDefinesItsNumber, metricNamesItsCustomUnitAndOmitsWhatItDoesNotDeclare; ConceptContextMarkdownTest.rendersWhatDefinesAMetricsNumber.
• Not run locally: openmetadata-service needs a full mvn clean install (the shaded Elasticsearch module is only produced at install), which would overwrite the local snapshot used by another branch. CI runs them.
• mvn spotless:apply: clean.

Checklist:

• I have read the CONTRIBUTING document.
• I have commented on my code, particularly in hard-to-understand areas.
• I have added tests around the new logic.

[gitar-bot]

⚠️ Bug: Blank filter not dropped: nullOrEmpty(" ") is false, so new test fails

filters() uses !nullOrEmpty(condition) to drop empty conditions. CommonUtil.nullOrEmpty(String) is string == null || string.isEmpty(), so a whitespace-only where such as " " gets through. metricCarriesWhatDefinesItsNumber passes new MetricFilter().withWhere(" ") and asserts List.of("fee_category = 'ATM Fee'"), so the test will fail in CI. In production, a blank filter gets sent to the agent and rendered as an empty line inside the sql block. Check for blank strings as well (the Java rules call for nullOrEmpty(value) || value.isBlank()). Ideally, strip each condition before keeping it.

Drop null, empty and whitespace-only conditions:

private static List<String> filters(Metric metric) {
  final List<String> where =
      listOrEmpty(metric.getFilters()).stream()
          .map(MetricFilter::getWhere)
          .filter(condition -> !nullOrEmpty(condition) && !condition.isBlank())
          .map(String::strip)
          .toList();
  return where.isEmpty() ? null : where;
}

• Apply fix

_{Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎}

[gitar-bot]

⚠️ Quality: New cross-repo conceptContext contract (filters/granularity/unit) has no ADR

This PR adds three JSON Schema fields to conceptContext (filters, granularity, unit). The description says open-metadata/ai-platform#2061 builds against them and that this PR has to merge first. That makes it a cross-repo contract, and nothing in docs/decisions/ covers the concept context shape. The rules exist only in the PR description and in schema prose. A record would need to say: "A metric's concept context carries the metric's non-blank filter conditions, its granularity enum value and its unit; the unit is customUnitOfMeasurement when unitOfMeasurement is OTHER and set, otherwise the enum value; each field is omitted when the metric does not declare it; type, dimensions, measures and the variant hierarchy are excluded because they do not change the number. OpenMetadata merges before ai-platform." These rules come from the description's lines "Type, dimensions, measures and the variant hierarchy stay out: they do not change the number" and "it builds against this one, so this merges first". Add a record under docs/decisions/ that states them.

_{Was this helpful? React with 👍 / 👎}

[gitar-bot]

Code Review ⚠️ Changes requested 0 closed / 2 findings

🟡 Medium risk · Adding metric filters, granularity, and units to AI context changes how agents interpret metrics; misapplied metadata could produce incorrect reported values.

Adds filters, granularity and unit to the metric concept context so the agent computes metrics with their defining conditions and units. Changes are requested because blank filter conditions such as " " are not dropped (the nullOrEmpty check misses whitespace), which makes the new metricCarriesWhatDefinesItsNumber test fail, and the new cross-repo conceptContext contract has no ADR under docs/decisions/.

⚠️ Bug: Blank filter not dropped: nullOrEmpty(" ") is false, so new test fails

📄 openmetadata-service/src/main/java/org/openmetadata/service/aicontext/ConceptContextBuilder.java:241-248 📄 openmetadata-service/src/test/java/org/openmetadata/service/aicontext/ConceptContextBuilderTest.java:279-288

filters() uses !nullOrEmpty(condition) to drop empty conditions. CommonUtil.nullOrEmpty(String) is string == null || string.isEmpty(), so a whitespace-only where such as " " gets through. metricCarriesWhatDefinesItsNumber passes new MetricFilter().withWhere(" ") and asserts List.of("fee_category = 'ATM Fee'"), so the test will fail in CI. In production, a blank filter gets sent to the agent and rendered as an empty line inside the sql block. Check for blank strings as well (the Java rules call for nullOrEmpty(value) || value.isBlank()). Ideally, strip each condition before keeping it.

Drop null, empty and whitespace-only conditions

private static List<String> filters(Metric metric) {
  final List<String> where =
      listOrEmpty(metric.getFilters()).stream()
          .map(MetricFilter::getWhere)
          .filter(condition -> !nullOrEmpty(condition) && !condition.isBlank())
          .map(String::strip)
          .toList();
  return where.isEmpty() ? null : where;
}

⚠️ Quality: New cross-repo conceptContext contract (filters/granularity/unit) has no ADR

📄 openmetadata-spec/src/main/resources/json/schema/type/aiContext.json:503-515 📄 openmetadata-service/src/main/java/org/openmetadata/service/aicontext/ConceptContextBuilder.java:119-123 📄 openmetadata-service/src/main/java/org/openmetadata/service/aicontext/ConceptContextBuilder.java:250-256

This PR adds three JSON Schema fields to conceptContext (filters, granularity, unit). The description says open-metadata/ai-platform#2061 builds against them and that this PR has to merge first. That makes it a cross-repo contract, and nothing in docs/decisions/ covers the concept context shape. The rules exist only in the PR description and in schema prose. A record would need to say: "A metric's concept context carries the metric's non-blank filter conditions, its granularity enum value and its unit; the unit is customUnitOfMeasurement when unitOfMeasurement is OTHER and set, otherwise the enum value; each field is omitted when the metric does not declare it; type, dimensions, measures and the variant hierarchy are excluded because they do not change the number. OpenMetadata merges before ai-platform." These rules come from the description's lines "Type, dimensions, measures and the variant hierarchy stay out: they do not change the number" and "it builds against this one, so this merges first". Add a record under docs/decisions/ that states them.

🤖 Prompt for agents

Code Review: Adds `filters`, `granularity` and `unit` to the metric concept context so the agent computes metrics with their defining conditions and units. Changes are requested because blank filter conditions such as `" "` are not dropped (the `nullOrEmpty` check misses whitespace), which makes the new `metricCarriesWhatDefinesItsNumber` test fail, and the new cross-repo `conceptContext` contract has no ADR under `docs/decisions/`.

1. ⚠️ Bug: Blank filter not dropped: nullOrEmpty(" ") is false, so new test fails
   Files: openmetadata-service/src/main/java/org/openmetadata/service/aicontext/ConceptContextBuilder.java:241-248, openmetadata-service/src/test/java/org/openmetadata/service/aicontext/ConceptContextBuilderTest.java:279-288

   `filters()` uses `!nullOrEmpty(condition)` to drop empty conditions. `CommonUtil.nullOrEmpty(String)` is `string == null || string.isEmpty()`, so a whitespace-only `where` such as `" "` gets through. `metricCarriesWhatDefinesItsNumber` passes `new MetricFilter().withWhere(" ")` and asserts `List.of("fee_category = 'ATM Fee'")`, so the test will fail in CI. In production, a blank filter gets sent to the agent and rendered as an empty line inside the `sql` block. Check for blank strings as well (the Java rules call for `nullOrEmpty(value) || value.isBlank()`). Ideally, strip each condition before keeping it.

   Fix (Drop null, empty and whitespace-only conditions):
   private static List<String> filters(Metric metric) {
     final List<String> where =
         listOrEmpty(metric.getFilters()).stream()
             .map(MetricFilter::getWhere)
             .filter(condition -> !nullOrEmpty(condition) && !condition.isBlank())
             .map(String::strip)
             .toList();
     return where.isEmpty() ? null : where;
   }

2. ⚠️ Quality: New cross-repo conceptContext contract (filters/granularity/unit) has no ADR
   Files: openmetadata-spec/src/main/resources/json/schema/type/aiContext.json:503-515, openmetadata-service/src/main/java/org/openmetadata/service/aicontext/ConceptContextBuilder.java:119-123, openmetadata-service/src/main/java/org/openmetadata/service/aicontext/ConceptContextBuilder.java:250-256

   This PR adds three JSON Schema fields to `conceptContext` (`filters`, `granularity`, `unit`). The description says open-metadata/ai-platform#2061 builds against them and that this PR has to merge first. That makes it a cross-repo contract, and nothing in `docs/decisions/` covers the concept context shape. The rules exist only in the PR description and in schema prose. A record would need to say: "A metric's concept context carries the metric's non-blank filter conditions, its granularity enum value and its unit; the unit is `customUnitOfMeasurement` when `unitOfMeasurement` is OTHER and set, otherwise the enum value; each field is omitted when the metric does not declare it; type, dimensions, measures and the variant hierarchy are excluded because they do not change the number. OpenMetadata merges before ai-platform." These rules come from the description's lines "Type, dimensions, measures and the variant hierarchy stay out: they do not change the number" and "it builds against this one, so this merges first". Add a record under `docs/decisions/` that states them.

Verification

Rules No rules evaluated

Functional validation Not enabled

Set up

Auto-approval Not enabled

Set up

_{For more verification details, comment gitar display:verbose.}

Options

Display: compact → Showing compact verification results.

Comment with these commands to change the behavior for this request:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source}