[lewisjared]

Resolves the metric bundle through the reader's artifacts.bundle, using the path recorded on the execution instead of a hardcoded diagnostic.json. An execution with no recorded bundle (a failed one) gets a 404.

GET /executions/ now raises when a group fails to build, instead of logging and leaving it out of the page. Dropping it made total_count disagree with data and hid the failure from the client, and GET /executions/{group_id} already raised for the same group.

Also makes ExecutionOutput's filename, short_name, long_name and description nullable, matching climate-ref, since a null was the most plausible way a group could fail to build. This changes the generated client types, and the figure components now handle the missing values.

Every successful execution in the fixture database records diagnostic.json, so bundle responses are unchanged there. Adds tests for a loaded bundle and the 404.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 32 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 995ea87b-fdf0-40ef-a59a-34a128f952c7

Commits

Reviewing files that changed from the base of the PR and between 21fb1f1 and fe2f53c.

Files selected for processing: 6

• backend/src/ref_backend/models/executions.py
• backend/tests/test_api/test_routes/test_executions.py
• changelog/173.fix.1.md
• frontend/src/client/types.gen.ts
• frontend/src/components/diagnostics/figure.tsx
• frontend/src/components/diagnostics/figureGalleryData.ts

Walkthrough

The execution API now uses each execution’s recorded metric bundle path and returns an error when it cannot load an execution group.

Changes: Execution routes

Layer / File(s)	Summary
Resolve recorded metric bundles backend/src/ref_backend/core/file_handling.py, backend/src/ref_backend/api/routes/executions.py, backend/tests/test_api/test_routes/test_executions.py, changelog/173.fix.md	The metric bundle route resolves the recorded bundle path and returns 404 when no bundle file exists. Tests cover successful retrieval and the missing-bundle response.
Propagate execution group errors backend/src/ref_backend/api/routes/executions.py, changelog/173.improvement.md	The route no longer catches individual group-build errors or returns a partial collection.

Priority: Normal

Change: Bug fix

Merge Risk: Low · previous review 21fb1

The route changes are mergeable with bounded follow-up to strengthen regression tests. Tests should distinguish recorded filenames from diagnostic.json and explicitly protect both error paths.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly summarises both primary changes: reading the recorded metric bundle path and stopping the omission of execution groups when group construction fails.
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 3 files. (2 skipped: 2 unsupported.)

Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Nitpick comments: 3

backend/tests/test_api/test_routes/test_executions.py: 2

689-692: Trivial · Maintainability & Code Quality · Quick win

Test the recorded bundle path, not only diagnostic.json.

metric_bundle now resolves the bundle from the path recorded on the execution. The success test uses only fixture data that records diagnostic.json. A route that still reads the fixed diagnostic.json name would return the same file and pass. The change to the recorded path is not covered.

Add a fixture where an execution records a different bundle filename, with that file present in the expected output location. Assert that the endpoint returns that file's contents. Keep the existing 404 test for an execution with no recorded bundle.

---

696-697: Trivial · Maintainability & Code Quality · Quick win

Assert the missing-bundle detail, not only the 404 status.

The test test_metric_bundle_404_without_recorded_bundle checks only status_code == 404. The route returns 404 from two branches. A missing execution, or one that belongs to another group, returns Result not found. A recorded bundle file that is missing returns Metrics bundle not found. The status-only assertion cannot tell these apart. If the lookup regresses to the first branch, the test still passes. Asserting the detail text confirms that the request reaches the bundle branch.

Suggested fix

 def test_metric_bundle_404_without_recorded_bundle(client: TestClient, settings) -> None:
     r = client.get(f"{settings.API_V1_STR}/executions/37/metric_bundle?execution_id=20")
     assert r.status_code == 404
+    assert r.json()["detail"] == "Metrics bundle not found"

backend/src/ref_backend/api/routes/executions.py: 1

140-140: Trivial · Maintainability & Code Quality · Quick win

Add a regression test for a failed group build in GET /executions/.

The list route no longer catches a build failure. A failing group now fails the whole request. No test covers this path, so restoring the former catch-and-omit logic would still pass the current suite. The shared client fixture uses TestClient(app) with default exception handling, so the exception re-raises in the test instead of returning a response. The test must disable that behaviour and assert the 500 response.

🧪 Suggested test

+def test_execution_list_fails_when_a_group_cannot_be_built(app, settings, monkeypatch) -> None:
+    def fail_build(*args, **kwargs):
+        raise RuntimeError("build failed")
+
+    monkeypatch.setattr(executions_routes.ExecutionGroup, "build", fail_build)
+
+    with TestClient(app, raise_server_exceptions=False) as client:
+        r = client.get(f"{settings.API_V1_STR}/executions")
+
+    assert r.status_code == 500

---

Review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: ca98b268-e13a-45b7-b8f7-9a94fde81403

Commits

Reviewing files that changed from the base of the PR and between 7130f0f and 21fb1f1.

Files selected for processing: 5

• backend/src/ref_backend/api/routes/executions.py
• backend/src/ref_backend/core/file_handling.py
• backend/tests/test_api/test_routes/test_executions.py
• changelog/173.fix.md
• changelog/173.improvement.md

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.