Add BLN3 farm indicators with heatmap table and map view#615
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a BLN3 farm indicators feature: indicator taxonomy and scoring helpers, server-side BLN3 score orchestration, a TanStack heatmap table with painpoints and aggregation cards, a MapLibre atlas view, props-driven controls, route modules, sidebar/header integration, styling, and calculator type/input updates. ChangesBLN3 Farm Indicators Feature
Sequence Diagram(s)sequenceDiagram
participant Browser
participant IndicatorsLoader
participant Bln3Server
participant Bln3Calculator
participant UI as "Map/Table UI"
Browser->>IndicatorsLoader: request /farm/:b_id_farm/:calendar/indicators (loader)
IndicatorsLoader->>Bln3Server: getIndicatorsForFarm(b_id_farm, timeframe)
Bln3Server->>Bln3Calculator: getBln3Score(inputs per field)
Bln3Calculator-->>Bln3Server: Bln3Score (per-field)
Bln3Server-->>IndicatorsLoader: FieldBln3Score[] (with errors mapped)
IndicatorsLoader-->>Browser: fields + fieldScores (loader data)
Browser->>UI: render HeatmapTable / IndicatorsMap with provided data
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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-app/app/components/blocks/header/indicators.tsx`:
- Around line 17-20: The link-generation is using the calendar from
useCalendarStore (const calendar = useCalendarStore(...)) which can drift from
the active URL; change the logic that builds links (where calendar is referenced
and where isKaart/currentName are used) to read the calendar value from the
route (e.g., useLocation/useParams or parse location.pathname) instead of the
store so generated links reflect the URL's calendar param; update all
occurrences in this file that reference the calendar variable (lines around the
useCalendarStore usage and other link builders between lines 26-45) to derive
calendar from the route before composing the link.
In `@fdm-app/app/components/blocks/indicators/table.tsx`:
- Around line 254-270: The header currently handles pin/unpin via an onClick on
the <th>, which is not keyboard-accessible; instead remove the onClick from the
th and wrap the interactive part in a real <button> (e.g., inside the header
render for header and header.column.id) with type="button" and the same toggle
logic calling onIndicatorClick(selectedIndicatorId === header.column.id ? null :
header.column.id). Move the conditional classes (cursor-pointer,
hover:bg-muted/40, selected bg/ring) and thBase styling as appropriate onto the
button (or apply both th and button classes) and add accessible attributes like
aria-pressed={selectedIndicatorId === header.column.id} and a descriptive
aria-label that includes the column id/title so screen readers can announce
pin/unpin state; ensure the <th> remains non-interactive (no onClick) so
keyboard and screen-reader users can operate the control.
In `@fdm-app/app/components/ui/breadcrumb.tsx`:
- Line 2: Replace the incorrect namespace access SlotPrimitive.Slot with the
Slot component itself by using SlotPrimitive when rendering conditional child
components; specifically update the expression that sets Comp (currently const
Comp = asChild ? SlotPrimitive.Slot : "a") to use const Comp = asChild ?
SlotPrimitive : "a", and make the same fix for other occurrences that use
SlotPrimitive.Slot (e.g., where asChild determines Comp in breadcrumb, sidebar,
button, form, and item components) so the component references the exported Slot
component rather than an undefined property.
In `@fdm-app/app/integrations/bln3.server.ts`:
- Around line 59-68: getIndicatorsForFarm currently always calls getFields
causing duplicated external I/O; modify getIndicatorsForFarm to accept an
optional preloadedFields parameter (e.g., preloadedFields?: Field[]) and only
call getFields(principal_id, b_id_farm, timeframe) when preloadedFields is
undefined, update its return/type handling accordingly, and change the
indicators loaders that already fetch fields to pass their preloaded fields into
getIndicatorsForFarm instead of letting it refetch; ensure you update all
callers and types where getIndicatorsForFarm is invoked and preserve timeframe
handling when deciding to use preloadedFields versus calling getFields.
- Around line 112-120: The current aggregation pushes values from
indicator.score or indicator.index into allValues without validating them, which
can yield null/NaN and cause the average to become NaN; update the block around
indicatorIds/indicator.indicator_id and the assignment const value = mode ===
"score" ? indicator.score : indicator.index to only push numeric, finite values
(e.g., use Number.isFinite(value) or typeof value === "number" && !isNaN(value))
into allValues, and keep the existing guard if (allValues.length === 0) return
null so invalid values are ignored before computing avg with
allValues.reduce(...).
🪄 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: 2d96e489-3cbb-49c6-b2c1-021c0287c1d9
📒 Files selected for processing (19)
.changeset/green-fields-shine.mdfdm-app/app/components/blocks/atlas/atlas-styles.tsxfdm-app/app/components/blocks/header/indicators.tsxfdm-app/app/components/blocks/indicators/aggregation-card.tsxfdm-app/app/components/blocks/indicators/atlas.tsxfdm-app/app/components/blocks/indicators/bln3-help-dialog.tsxfdm-app/app/components/blocks/indicators/category-filter.tsxfdm-app/app/components/blocks/indicators/measures-toggle.tsxfdm-app/app/components/blocks/indicators/score-badge.tsxfdm-app/app/components/blocks/indicators/table-cell.tsxfdm-app/app/components/blocks/indicators/table.tsxfdm-app/app/components/blocks/sidebar/apps.tsxfdm-app/app/components/ui/breadcrumb.tsxfdm-app/app/integrations/bln3.server.tsfdm-app/app/lib/indicators.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.indicators._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.indicators.atlas.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.indicators.tsxfdm-app/app/tailwind.css
coderabbitai
Bot
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
fdm-app/app/routes/farm.$b_id_farm.$calendar.indicators._index.tsx (1)
208-208: 💤 Low valueVisual feedback for instant transitions may confuse users.
Line 208 applies
opacity-50 pointer-events-nonewhenisPending, but if the state updates are near-instant (as suggested by the current implementation), users won't perceive the feedback, or it will flicker distractingly.Consider removing the transition-based dimming unless profiling confirms that renders are slow enough to warrant this UX pattern.
🤖 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-app/app/routes/farm`.$b_id_farm.$calendar.indicators._index.tsx at line 208, The UI applies instantaneous dimming via the section's className (the JSX line using cn("space-y-3 transition-opacity duration-150", isPending && "opacity-50 pointer-events-none")) which can flicker; either remove the conditional dimming by eliminating the isPending && "opacity-50 pointer-events-none" clause from that section's className, or implement a short debounce before showing visual feedback (create a local showPending state that sets true only after a ~150ms timeout when isPending becomes true and clears immediately when false) and use showPending in the className instead of isPending.
🤖 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-app/app/integrations/bln3.server.ts`:
- Around line 50-54: Remove the unnecessary TypeScript assertion when calling
getBln3Score: spread the inputs (typed Bln3ScoreCollectedInputs) and add
nmiApiKey directly so the combined object is inferred as Bln3ScoreInputs by the
compiler; specifically, in the call to getBln3Score in bln3.server.ts replace
the object "{ ...inputs, nmiApiKey } as Bln3ScoreInputs" with just "{ ...inputs,
nmiApiKey }" (no assertion) to let TypeScript infer the correct type from
Bln3ScoreCollectedInputs and Bln3ScoreInputs.
---
Nitpick comments:
In `@fdm-app/app/routes/farm`.$b_id_farm.$calendar.indicators._index.tsx:
- Line 208: The UI applies instantaneous dimming via the section's className
(the JSX line using cn("space-y-3 transition-opacity duration-150", isPending &&
"opacity-50 pointer-events-none")) which can flicker; either remove the
conditional dimming by eliminating the isPending && "opacity-50
pointer-events-none" clause from that section's className, or implement a short
debounce before showing visual feedback (create a local showPending state that
sets true only after a ~150ms timeout when isPending becomes true and clears
immediately when false) and use showPending in the className instead of
isPending.
🪄 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: 7da9e9a2-bf39-4f8e-a312-fb31093022f9
📒 Files selected for processing (11)
fdm-app/app/components/blocks/header/indicators.tsxfdm-app/app/components/blocks/indicators/aggregation-card.tsxfdm-app/app/components/blocks/indicators/atlas.tsxfdm-app/app/components/blocks/indicators/category-filter.tsxfdm-app/app/components/blocks/indicators/measures-toggle.tsxfdm-app/app/components/blocks/indicators/table-cell.tsxfdm-app/app/components/blocks/indicators/table.tsxfdm-app/app/integrations/bln3.server.tsfdm-app/app/lib/bln3.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.indicators._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.indicators.atlas.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- fdm-app/app/components/blocks/header/indicators.tsx
- fdm-app/app/components/blocks/indicators/table-cell.tsx
- fdm-app/app/components/blocks/indicators/aggregation-card.tsx
- fdm-app/app/components/blocks/indicators/atlas.tsx
- fdm-app/app/components/blocks/indicators/table.tsx
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/input.ts`:
- Around line 92-93: The enum fields b_soiltype_agr and b_gwl_class currently
cast latestAnalysis?.b_* directly and can propagate null; restore nullish
normalization by applying a nullish coalescing before the cast (e.g.,
(latestAnalysis?.b_soiltype_agr ?? undefined) as
Bln3ScoreCollectedInputs["b_soiltype_agr"] and similarly for b_gwl_class) so
null values are normalized/omitted and request validation is preserved.
🪄 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: e62a96ff-3b9d-4880-9547-57d75c47740c
📒 Files selected for processing (4)
fdm-app/app/integrations/bln3.server.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.indicators._index.tsxfdm-calculator/src/bln3/input.tsfdm-calculator/src/bln3/types.d.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- fdm-app/app/integrations/bln3.server.ts
- fdm-app/app/routes/farm.$b_id_farm.$calendar.indicators._index.tsx
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Sven Verweij <37927107+SvenVw@users.noreply.github.com>
BoraIneviNMI
approved these changes
May 15, 2026
Collaborator
BoraIneviNMI
left a comment
BoraIneviNMI
left a comment
There was a problem hiding this comment.
I haven't seen any big issues. Maybe the indicator select dropdown can be slightly improved in appearance but that is all.
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
Closes #575