Add BLN3 score calculation with caching#608
Merged
Merged
Conversation
…raw NMI API call to `POST /maatwerk/bln3/score/field`), `getBln3Score` (cached wrapper via `withCalculationCache`), and `collectInputForBln3Score` (assembles field inputs from fdm-core: lat/lon from field centroid, soil analysis parameters, cultivations mapped from BRP catalogue codes, and adopted BLN measures). Types exported: `Bln3Score`, `Bln3ScoreInputs`, `Bln3ScoreCollectedInputs`, `Bln3IndicatorResult`, `Bln3AggregationResult`.
Contributor
📝 WalkthroughWalkthroughAdds BLN3 score module: TypeScript types, collectInputForBln3Score (concurrent FDM reads and transformations), requestBln3Score (POST to NMI with 30s timeout and validation), getBln3Score (withCalculationCache wrapper), tests, re-exports, and a Changesets entry. ChangesBLN3 Score Calculation Feature
Sequence DiagramsequenceDiagram
participant App as fdm-app
participant Calc as fdm-calculator
participant FdmCore as FDM Core
participant Cache as Calculation Cache
participant NmiApi as NMI API
App->>Calc: getBln3Score(field_id, nmiApiKey)
Calc->>Cache: lookup hash(requestBln3Score + inputs with nmiApiKey redacted)
alt Cache Hit
Cache-->>App: Bln3Score
else Cache Miss
Calc->>FdmCore: getField, getSoilAnalyses, getCultivations, getMeasures
FdmCore-->>Calc: field + soil + cultivations + measures
Calc->>Calc: collectInputForBln3Score transforms data
Calc->>NmiApi: POST /maatwerk/bln3/score/field (Bearer auth)
NmiApi-->>Calc: {data: {indicator: [...]}}
Calc->>Cache: store result
Cache-->>App: Bln3Score
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
6 tasks
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
coderabbitai
Bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@fdm-calculator/src/bln3/index.test.ts`:
- Around line 129-134: Add unit tests for getBln3Score that exercise actual
cache semantics rather than just type: mock or stub the underlying
requestBln3Score and the cache layer used by withCalculationCache to assert (1)
a cache miss calls requestBln3Score and then stores the result (miss→store), (2)
a subsequent call with the same key returns the cached value and does not call
requestBln3Score (hit without refetch), and (3) simulate cache read/write
failures to verify error-path handling and that appropriate logging occurs;
reference getBln3Score, requestBln3Score and withCalculationCache when locating
code to mock and assert behavior, and mirror patterns used in
fdm-core/src/calculator.test.ts for cache assertions and logging checks.
In `@fdm-calculator/src/bln3/index.ts`:
- Around line 28-38: The POST fetch in bln3 (the call assigning to `response`)
must use an AbortController to enforce a timeout: create an AbortController,
pass its `signal` into the fetch options, start a timer (e.g., with setTimeout)
to call controller.abort() after a configurable ms, and clear the timer once the
response is received; update the surrounding logic that awaits `response` to
handle an aborted request (treat AbortError or aborted fetch as a timeout error)
so the worker doesn't hang. Use the same timeout pattern used in
`dyna.ts`/`nsupply.ts` and reference the `response` variable and the fetch call
in `fdm-calculator/src/bln3/index.ts` when applying the change.
- Around line 47-52: The API response mapping currently assumes semantic
success; before returning from the function that parses the NMI response (where
result: Bln3ScoreResponse is created and you map result.data.indicator →
indicators), validate result.success is true and handle the failure path: if
result.success is false, throw an error (or return a rejected promise) including
any error/message fields from the response for observability instead of
proceeding to map result.data; ensure you reference and check result.success and
use result.data or result.error in the error payload so the calling code can
distinguish HTTP success from API-level failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0235bbb9-1f20-4ed7-a4ed-51f918378b16
📒 Files selected for processing (7)
.changeset/loud-mirrors-kneel.mdfdm-calculator/src/bln3/collect.test.tsfdm-calculator/src/bln3/collect.tsfdm-calculator/src/bln3/index.test.tsfdm-calculator/src/bln3/index.tsfdm-calculator/src/bln3/types.d.tsfdm-calculator/src/index.ts
Collaborator
BoraIneviNMI
left a comment
BoraIneviNMI
left a comment
There was a problem hiding this comment.
I found a couple things but it is up to you to change them. I can just approve if you aren't going to change anything.
| @@ -0,0 +1,106 @@ | |||
| import { | |||
Collaborator
There was a problem hiding this comment.
Make sure you want to call this file "collect.ts" and not "input.ts" or something.
| a_lon: 5.2, | ||
| } | ||
|
|
||
| describe("requestBln3Score", () => { |
Collaborator
There was a problem hiding this comment.
You didn't test the case where fetch throws an error.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
fdm-calculator/src/bln3/index.test.ts (1)
147-152:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd real cache-behavior tests for
getBln3Score(miss/hit + error logging).Lines 147-152 only validate export shape; they do not cover the required cache semantics from the PR objective (miss→store, hit without refetch, and cache error logging paths).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fdm-calculator/src/bln3/index.test.ts` around lines 147 - 152, Add tests in index.test.ts that exercise real cache semantics for getBln3Score: write one test that simulates a cache miss by mocking the underlying request function (requestBln3Score) to return a value and asserting the value is stored (call twice and assert requestBln3Score called only once after store), another test for a cache hit where you pre-seed the cache (or mock the cache layer used by withCalculationCache) and assert getBln3Score returns cached value without invoking requestBln3Score, and a test that forces the cache layer to throw/return an error and asserts the error path logs via the logger (spy on the logger used by withCalculationCache). Use the existing symbols getBln3Score, requestBln3Score and withCalculationCache to locate and mock the pieces and ensure you verify call counts and logger calls for miss/hit/error scenarios.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@fdm-calculator/src/bln3/index.test.ts`:
- Around line 46-145: Add a test in index.test.ts that exercises
requestBln3Score's semantic API-failure branch by mocking fetch to resolve with
ok: true and json() returning { success: false, error_message: "semantic failure
message" } (use baseInputs/Bln3ScoreInputs) and assert that
requestBln3Score(...) rejects with that exact error_message and that fetch was
called once; use vi.mocked(fetch).mockResolvedValueOnce(...) similar to other
tests and mirror the pattern used in the non-ok and AbortError tests.
In `@fdm-calculator/src/bln3/index.ts`:
- Around line 52-61: The code assumes result.data exists after checking
result.success; before returning indicators and aggregations, validate that
result.data is present and has the expected fields (e.g., result.data.indicator
and result.data.aggregations) and throw a clear Error if not (instead of letting
a TypeError occur). In the function handling the Bln3ScoreResponse, guard
against malformed payloads by checking result.data and the shapes of
result.data.indicator/aggregations (or use result.data?.indicator with an
explicit null/undefined check) and return only when valid, otherwise throw a
descriptive error mentioning the missing payload fields.
---
Duplicate comments:
In `@fdm-calculator/src/bln3/index.test.ts`:
- Around line 147-152: Add tests in index.test.ts that exercise real cache
semantics for getBln3Score: write one test that simulates a cache miss by
mocking the underlying request function (requestBln3Score) to return a value and
asserting the value is stored (call twice and assert requestBln3Score called
only once after store), another test for a cache hit where you pre-seed the
cache (or mock the cache layer used by withCalculationCache) and assert
getBln3Score returns cached value without invoking requestBln3Score, and a test
that forces the cache layer to throw/return an error and asserts the error path
logs via the logger (spy on the logger used by withCalculationCache). Use the
existing symbols getBln3Score, requestBln3Score and withCalculationCache to
locate and mock the pieces and ensure you verify call counts and logger calls
for miss/hit/error scenarios.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0e365d78-09d2-4bfb-933f-9a51ddafa39b
📒 Files selected for processing (4)
fdm-calculator/src/bln3/index.test.tsfdm-calculator/src/bln3/index.tsfdm-calculator/src/bln3/input.test.tsfdm-calculator/src/bln3/input.ts
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@fdm-calculator/src/bln3/index.test.ts`:
- Around line 100-112: Add a new unit test alongside the existing one to cover
the case where the API response includes aggregations: mock fetch to return
mockBln3ScoreResponse with a data.aggregations field populated, call
requestBln3Score(baseInputs), and assert the returned Bln3Score equals {
indicators: mockBln3ScoreResponse.data.indicator, aggregations:
mockBln3ScoreResponse.data.aggregations } to verify the mapping done in
requestBln3Score (see where aggregations are read in index.ts).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e5aaa156-6b19-40ec-b466-dbeb7eb82bb4
📒 Files selected for processing (2)
fdm-calculator/src/bln3/index.test.tsfdm-calculator/src/bln3/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- fdm-calculator/src/bln3/index.ts
BoraIneviNMI
approved these changes
May 13, 2026
Collaborator
BoraIneviNMI
left a comment
BoraIneviNMI
left a comment
There was a problem hiding this comment.
Looks good now. Thank you for making the changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Tests
Closes #574