Fix JSON formatter name collision issue (#129)

## Summary

- Fix JSON formatter to use `unique_id` instead of `name` as dictionary
keys
- Prevents evaluables of different types with the same name from
overwriting each other
- Adds comprehensive test case for name collision scenario
- Updates existing tests to match new output format

## Problem

The JSON formatter was using `evaluable.name` as the dictionary key,
which caused issues when evaluables of different types (e.g., a model
and an exposure) had the same name. The second evaluable processed would
overwrite the first in the JSON output.

## Solution

Changed the JSON formatter to use `evaluable.unique_id` (e.g.,
`model.package.model_name`, `exposure.package.exposure_name`) instead of
just the name. This approach:

- Ensures unique keys for all evaluables regardless of name collisions
- Makes the JSON formatter consistent with the Manifest formatter
behavior
- Preserves all evaluables in the output

## Changes

- Modified `src/dbt_score/formatters/json_formatter.py` to use
`unique_id` as keys
- Updated docstring examples to reflect the new key format
- Updated existing tests in `tests/formatters/test_json_formatter.py`
- Added new test case `test_json_formatter_name_collision_prevention` to
verify the fix

## Test plan

- [x] Existing JSON formatter tests pass with updated expected output
- [x] New test case specifically validates name collision prevention
- [x] All linting and type checking passes
- [x] Pre-commit hooks pass

🤖 Generated with [Claude Code](https://claude.ai/code)

---------

Co-authored-by: Claude <[email protected]>
Co-authored-by: Matthieu Caneill <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -9,6 +9,8 @@ and this project adheres to
 ## [Unreleased]
 
 - Display the parse error message when `dbt parse` fails.
+- **Breaking**: JSON-formatted output is using `unique_id` as key instead of
+  `name`, to avoid duplicates (e.g if exposure and model have the same name).
 
 ## [0.13.1] - 2025-07-29
 

src/dbt_score/formatters/json_formatter.py

@@ -5,7 +5,7 @@
 ```json
 {
     "evaluables": {
-        "model_foo": {
+        "model.package.model_foo": {
             "score": 5.0,
             "badge": "🥈",
             "pass": true,
@@ -23,7 +23,7 @@
             },
             "type": "model"
         },
-        "model_bar": {
+        "model.package.model_bar": {
             "score": 0.0,
             "badge": "🥉",
             "pass": false,
@@ -35,7 +35,7 @@
             },
             "type": "model"
         },
-        "source_baz": {
+        "source.package.source_name.source_baz": {
             "score": 10.0,
             "badge": "🥇",
             "pass": false,
@@ -80,7 +80,7 @@ def evaluable_evaluated(
         self, evaluable: Evaluable, results: EvaluableResultsType, score: Score
     ) -> None:
         """Callback when an evaluable item has been evaluated."""
-        self.evaluable_results[evaluable.name] = {
+        self.evaluable_results[evaluable.unique_id] = {
             "score": score.value,
             "badge": score.badge,
             "pass": score.value >= self._config.fail_any_item_under,
@@ -90,19 +90,25 @@ def evaluable_evaluated(
         for rule, result in results.items():
             severity = rule.severity.name.lower()
             if result is None:
-                self.evaluable_results[evaluable.name]["results"][rule.source()] = {
+                self.evaluable_results[evaluable.unique_id]["results"][
+                    rule.source()
+                ] = {
                     "result": "OK",
                     "severity": severity,
                     "message": None,
                 }
             elif isinstance(result, RuleViolation):
-                self.evaluable_results[evaluable.name]["results"][rule.source()] = {
+                self.evaluable_results[evaluable.unique_id]["results"][
+                    rule.source()
+                ] = {
                     "result": "WARN",
                     "severity": severity,
                     "message": result.message,
                 }
             else:
-                self.evaluable_results[evaluable.name]["results"][rule.source()] = {
+                self.evaluable_results[evaluable.unique_id]["results"][
+                    rule.source()
+                ] = {
                     "result": "ERR",
                     "severity": severity,
                     "message": str(result),

tests/conftest.py

@@ -83,6 +83,12 @@ def model2(raw_manifest) -> Model:
     return Model.from_node(raw_manifest["nodes"]["model.package.model2"], [])
 
 
+@fixture
+def model_collision_test(raw_manifest) -> Model:
+    """Model with collision_test name."""
+    return Model.from_node(raw_manifest["nodes"]["model.package.collision_test"], [])
+
+
 # Sources
 
 
@@ -123,13 +129,21 @@ def snapshot2(raw_manifest) -> Snapshot:
 @fixture
 def exposure1(raw_manifest) -> Exposure:
     """Exposure 1."""
-    return Exposure.from_node(raw_manifest["nodes"]["exposure.package.exposure1"])
+    return Exposure.from_node(raw_manifest["exposures"]["exposure.package.exposure1"])
 
 
 @fixture
 def exposure2(raw_manifest) -> Exposure:
     """Exposure 2."""
-    return Exposure.from_node(raw_manifest["nodes"]["exposure.package.exposure2"])
+    return Exposure.from_node(raw_manifest["exposures"]["exposure.package.exposure2"])
+
+
+@fixture
+def exposure_collision(raw_manifest) -> Exposure:
+    """Exposure with collision_test name."""
+    return Exposure.from_node(
+        raw_manifest["exposures"]["exposure.package.exposure_collision"]
+    )
 
 
 # Multiple ways to create rules

tests/formatters/test_json_formatter.py

@@ -1,5 +1,6 @@
 """Unit tests for the JSON formatter."""
 
+import json
 from typing import Type
 
 from dbt_score.formatters.json_formatter import JSONFormatter
@@ -32,7 +33,7 @@ def test_json_formatter(
         stdout
         == """{
   "evaluables": {
-    "model1": {
+    "model.package.model1": {
       "score": 10.0,
       "badge": "🥇",
       "pass": true,
@@ -55,7 +56,7 @@ def test_json_formatter(
       },
       "type": "model"
     },
-    "table1": {
+    "source.package.my_source.table1": {
       "score": 10.0,
       "badge": "🥇",
       "pass": true,
@@ -87,3 +88,50 @@ def test_json_formatter(
 }
 """
     )
+
+
+def test_json_formatter_name_collision_prevention(
+    capsys,
+    default_config,
+    manifest_loader,
+    model_collision_test,
+    exposure_collision,
+    rule_severity_medium,
+):
+    """Ensure evaluables with same name but different types don't overwrite."""
+    # Verify they have the same name but different unique_ids
+    assert model_collision_test.name == exposure_collision.name == "collision_test"
+    assert model_collision_test.unique_id != exposure_collision.unique_id
+
+    formatter = JSONFormatter(manifest_loader=manifest_loader, config=default_config)
+    results: dict[Type[Rule], RuleViolation | Exception | None] = {
+        rule_severity_medium: RuleViolation("Test violation")
+    }
+
+    # Evaluate both evaluables with same name
+    formatter.evaluable_evaluated(model_collision_test, results, Score(5.0, "🥈"))
+    formatter.evaluable_evaluated(exposure_collision, results, Score(7.0, "🥇"))
+    formatter.project_evaluated(Score(6.0, "🥈"))
+
+    stdout = capsys.readouterr().out
+    output_data = json.loads(stdout)
+
+    # Both evaluables should be present (no collision)
+    evaluables = output_data["evaluables"]
+    assert len(evaluables) == 2, "Both evaluables with same name should be preserved"
+
+    # Keys should be unique_id, not name
+    assert model_collision_test.unique_id in evaluables
+    assert exposure_collision.unique_id in evaluables
+
+    # Verify both evaluables maintain their distinct data
+    model_data = evaluables[model_collision_test.unique_id]
+    exposure_data = evaluables[exposure_collision.unique_id]
+
+    assert model_data["type"] == "model"
+    assert model_data["score"] == 5.0
+    assert model_data["badge"] == "🥈"
+
+    assert exposure_data["type"] == "exposure"
+    assert exposure_data["score"] == 7.0
+    assert exposure_data["badge"] == "🥇"

tests/resources/manifest.json

@@ -141,6 +141,40 @@
       "access": "public",
       "group": "them_over_there"
     },
+    "model.package.collision_test": {
+      "resource_type": "model",
+      "unique_id": "model.package.collision_test",
+      "name": "collision_test",
+      "relation_name": "database.schema.collision_test",
+      "description": "A model to test name collision with exposure.",
+      "original_file_path": "/path/to/collision_test.sql",
+      "config": {},
+      "meta": {},
+      "columns": {
+        "a": {
+          "name": "column_a",
+          "description": "Column A.",
+          "data_type": "string",
+          "meta": {},
+          "constraints": [],
+          "tags": []
+        }
+      },
+      "constraints": [],
+      "package_name": "package",
+      "database": "db",
+      "schema": "schema",
+      "raw_code": "SELECT x FROM y",
+      "alias": "collision_test_alias",
+      "patch_path": "/path/to/collision_test.yml",
+      "tags": [],
+      "depends_on": {
+        "nodes": ["model.package.model2"]
+      },
+      "language": "sql",
+      "access": "public",
+      "group": "default"
+    },
     "model.package2.model1": {
       "resource_type": "model",
       "unique_id": "model.package2.model1",
@@ -520,6 +554,44 @@
       "sources": [],
       "metrics": [],
       "created_at": 1744832856.199685
+    },
+    "exposure.package.exposure_collision": {
+      "name": "collision_test",
+      "resource_type": "exposure",
+      "package_name": "package",
+      "path": "models/exposures/exposures.yml",
+      "original_file_path": "models/exposures/exposures.yml",
+      "unique_id": "exposure.package.exposure_collision",
+      "fqn": ["package", "exposures", "exposure_collision"],
+      "type": "application",
+      "owner": {
+        "email": null,
+        "name": "owner"
+      },
+      "description": "This exposure has the same name as collision_test model.",
+      "label": null,
+      "maturity": null,
+      "meta": {},
+      "tags": ["collision", "test"],
+      "config": {
+        "enabled": true
+      },
+      "unrendered_config": {},
+      "url": null,
+      "depends_on": {
+        "macros": [],
+        "nodes": ["model.package.collision_test"]
+      },
+      "refs": [
+        {
+          "name": "collision_test",
+          "package": null,
+          "version": null
+        }
+      ],
+      "sources": [],
+      "metrics": [],
+      "created_at": 1744832856.199685
     }
   }
 }

tests/test_models.py

@@ -51,6 +51,7 @@ def test_manifest_load(mock_read_text, raw_manifest):
         ]
         assert loader.models["model.package.model2"].children == [
             loader.models["model.package.model1"],
+            loader.models["model.package.collision_test"],
             loader.exposures["exposure.package.exposure2"],
         ]
         assert loader.models["model.package.model2"].parents == [
@@ -67,9 +68,18 @@ def test_manifest_load(mock_read_text, raw_manifest):
             loader.models["model.package.model2"]
         ]
 
+        assert loader.models["model.package.collision_test"].parents == [
+            loader.models["model.package.model2"]
+        ]
+        assert loader.models["model.package.collision_test"].children == [
+            loader.exposures["exposure.package.exposure_collision"],
+        ]
         assert loader.exposures["exposure.package.exposure1"].parents == [
             loader.models["model.package.model1"]
         ]
+        assert loader.exposures["exposure.package.exposure_collision"].parents == [
+            loader.models["model.package.collision_test"]
+        ]
 
 
 @patch("dbt_score.models.Path.read_text")