Separate obs4REF reference data from published obs4MIPs - #82
Conversation
✅ Deploy Preview for climate-ref ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds structured reference dataset source metadata, updates ESMValTool, ILAMB, and PMP diagnostic records, exposes execution resource usage in frontend types and views, and updates frontend pre-commit tooling. ChangesReference dataset metadata
Frontend tooling
Merge Risk: 🟡 Moderate · up to The PR changes diagnostic provenance labels and exposes aggregate execution-resource data. Merge readiness is currently moderate because the documentation/UI and validation fixture still disagree with the new source-label contract, and visibility of resource telemetry needs explicit owner confirmation. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 6 files. (4 skipped: 4 unsupported.) ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Team
Run ID: 8f8567be-fa1a-43b5-9e24-6bc05e7229a4
📒 Files selected for processing (13)
.pre-commit-config.yamlbackend/src/ref_backend/core/diagnostic_metadata.pybackend/static/diagnostics/esmvaltool.yamlbackend/static/diagnostics/ilamb.yamlbackend/static/diagnostics/pmp.yamlbackend/tests/test_core/test_diagnostic_metadata.pychangelog/82.improvement.mdchangelog/82.trivial.mdfrontend/biome.jsonfrontend/src/client/schemas.gen.tsfrontend/src/client/types.gen.tsfrontend/src/components/diagnostics/diagnosticCard.tsxfrontend/src/routes/_app/diagnostics.$providerSlug.$diagnosticSlug/route.tsx
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
| files: ^frontend/.*\.(ts|tsx|js|jsx|json|jsonc|md|css)$ | ||
| pass_filenames: false |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expect no unsupported files, or an explicit unknown-file policy.
fd --hidden --type f frontend | rg -n '\.(md|ya?ml|scss)$' || trueRepository: Climate-REF/ref-app
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- .pre-commit-config.yaml ---'
sed -n '60,95p' .pre-commit-config.yaml
printf '%s\n' '--- frontend/biome.json ---'
cat frontend/biome.json
printf '%s\n' '--- frontend package scripts and Biome version ---'
if [ -f frontend/package.json ]; then
sed -n '1,180p' frontend/package.json
fi
printf '%s\n' '--- frontend files with extensions relevant to Biome discovery ---'
fd --hidden --type f frontend | sort | rg '\.(md|ya?ml|scss|ts|tsx|js|jsx|json|jsonc|css)$' || trueRepository: Climate-REF/ref-app
Length of output: 5502
🌐 Web query:
Biome 2.2.4 CLI files ignoreUnknown unsupported Markdown files check documentation
💡 Result:
In Biome, the files.ignoreUnknown configuration option is used to control how the CLI handles files with extensions that Biome does not recognize [1][2]. By setting ignoreUnknown to true in your biome.json configuration file, you tell Biome to suppress diagnostic errors when it encounters unsupported file types [1]. If you are encountering errors when running Biome on unsupported files (such as Markdown files, if they are not explicitly supported or handled by your configuration), you can resolve this in the following ways: 1. Configuration File: Add the following to your biome.json file [1]: { "files": { "ignoreUnknown": true } } 2. CLI Flag: You can also use the --files-ignore-unknown flag directly in your terminal command to achieve the same effect for a single execution [3][4]: biome check --files-ignore-unknown=true. If you continue to see errors such as "No files were processed in the specified paths," this is often expected behavior if Biome is configured to only look at specific files or if the input paths provided are entirely ignored [5][4]. In such cases, you may also want to use the --no-errors-on-unmatched flag, which prevents the CLI from exiting with an error code if no files are matched by your provided patterns [5]. For more details on managing file inclusion and exclusion, you can refer to the official Biome configuration documentation [1][6].
Citations:
- 1: https://biomejs.dev/reference/configuration/
- 2: https://github.com/biomejs/biome/blob/db36fb42/packages/@biomejs/backend-jsonrpc/src/workspace.ts
- 3: GitHub issue 6566 in biomejs/biome (link omitted to avoid creating a cross-reference)
- 4: https://github.com/biomejs/biome/blob/d62b3317/crates/biome_cli/tests/snapshots/main_commands_check/unsupported_file_verbose.snap
- 5: GitHub issue 2140 in biomejs/biome (link omitted to avoid creating a cross-reference)
- 6: https://biomejs.dev/guides/configure-biome/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- all frontend files ---'
fd --hidden --type f frontend | sort | head -200
printf '%s\n' '--- tracked frontend files ---'
git ls-files frontend | sort | head -200
printf '%s\n' '--- lockfile Biome resolution ---'
rg -n -m 8 '"`@biomejs/biome`"|node_modules/@biomejs/biome|biome@' frontend/package-lock.json frontend/yarn.lock frontend/pnpm-lock.yaml frontend/bun.lockb 2>/dev/null || trueRepository: Climate-REF/ref-app
Length of output: 9834
Prevent the Biome hook from scanning unsupported files.
biome check --write scans the whole frontend directory. frontend/biome.json includes unknown files and sets files.ignoreUnknown to false. Files such as frontend/README.md can therefore make the hook fail when a matching file triggers it. Remove unsupported extensions from the hook trigger or set files.ignoreUnknown to true.
Source: MCP tools
| # `slug` is `<supplier>.<source_id>`, and `source` says the same thing in a structured field. | ||
| # The prefix names who supplies the data, not the source type the requirement asks for, so a | ||
| # dataset required as `obs4mips` but served pre-release reads `obs4ref.HadISST-1-1`. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep supplier identity and provenance separate.
A slug encodes the supplier, but source can be "recipe". For example, esmvaltool.GPCP-V2.3 has source: "recipe". The current wording states that these values mean the same thing, so the tooltip gives incorrect provenance for recipe-backed datasets.
backend/static/diagnostics/esmvaltool.yaml#L5-L7: State thatslugidentifies the supplier and thatsourcerecords separate provenance.frontend/src/routes/_app/diagnostics.$providerSlug.$diagnosticSlug/route.tsx#L232-L234: Do not describe the slug as the data source. Displayref.sourcewhen present, or describe the slug as the supplier identifier.
📍 Affects 2 files
backend/static/diagnostics/esmvaltool.yaml#L5-L7(this comment)frontend/src/routes/_app/diagnostics.$providerSlug.$diagnosticSlug/route.tsx#L232-L234
| yaml_content = { | ||
| "with/source": { | ||
| "reference_datasets": [ | ||
| {"slug": "obs4mips.HadISST-1-1", "type": "primary", "source": "obs4ref"}, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a valid slug in the source parsing fixture.
Line 299 sets source: "obs4ref" but uses the obs4mips slug prefix. The new contract requires the supplier prefix and source to agree. Change the slug to obs4ref.HadISST-1-1, or add a separate test that asserts mismatches are rejected.
Reworks the diagnostic reference dataset metadata so it matches the datasets the providers actually select, and so a reader can tell published data from our pre-release data. - Names each dataset by who supplies it, so a dataset required as `obs4mips` but served ahead of publication now reads `obs4ref.HadISST-1-1`. - Adds a `source` field carrying the same thing in a structured form, with a test that keeps it in step with the name. - Corrects the source ids against the providers' data requirements, so ILAMB and PMP datasets are labelled from the providers rather than from a mix of guesses. - Adds the five ozone diagnostics, and drops `pmp/enso_perf` and the AMO mode, which the providers no longer register. - Fixes the ILAMB `emp-gleamgpcp2.3` key, which never matched the diagnostic slug. The reference dataset cards now float the primary/secondary badge to the top right, because the dataset itself matters more than its role.
The Biome hooks passed staged filenames to `bash -lc "..."`, so they landed as positional arguments to bash rather than as arguments to Biome. Biome never saw them and checked the whole `frontend/` tree instead. That pulled in `frontend/public/favicon.svg`, which pre-commit itself excludes, so `biome-lint` failed on every commit regardless of what changed. - Collapses the two hooks into one `biome-check`, because `check --write` already covers format and lint. - Sets `pass_filenames: false`, which is what the hook was doing anyway. - Runs the pinned `frontend/node_modules/.bin` binaries instead of `npx --yes`, so Biome and tsc match the versions in `package.json` rather than whatever the registry serves. - Ignores `public/` in `biome.json` to match the pre-commit exclude, and bumps the schema to the installed 2.5.11.
Uses the field's full name, which lines it up with the `source_type` already used elsewhere in the API so the two can be merged later.
229b8ea to
d549245
Compare
1 participant
Reworks the diagnostic reference dataset metadata so it matches the datasets the REF providers actually select, and separates published obs4MIPs data from our own obs4REF data. A dataset required as
obs4mipsbut supplied from obs4REF now readsobs4ref.HadISST-1-1, and a newsource_typefield carries the same distinction in structured form for API consumers.Worth a close look:
esmvaltool.yaml,ilamb.yamlandpmp.yamlwere previously a mix of guesses. They are now labelled from the data requirements, so most of the diff is relabelling rather than new content.pmp/enso_perfand the AMO mode, which the providers no longer register. Anything linking to those slugs will 404.emp-gleamgpcp2.3key, which never matched the diagnostic slug.source_typeduplicates the slug prefix, and only a test keeps the two in sync. Deduping it, and merging it with thesource_typealready used elsewhere in the API, is follow-up work.The second commit is unrelated tooling. The Biome pre-commit hooks passed staged filenames to
bash -lc "...", so they became positional arguments to bash and never reached Biome. Biome silently checked the wholefrontend/tree instead, includingfrontend/public/, which pre-commit itself excludes. That made the lint hook fail on every commit regardless of what changed. The hooks now run the binaries pinned infrontend/package.jsonrather than whatevernpx --yesresolves from the registry.Summary by CodeRabbit
New Features
Improvements
Removed