Accept negative foundationDepth — it is a NAP level, not a length - #106
Merged
Conversation
`foundationDepth` was validated with `numericLength` (`gte(0)`), so any value below NAP was rejected with a 400. Data Studio renders the field with the `LEVEL` preset (unit `m NAP`, min -999.99), so every realistic entry failed and surfaced as the generic "Opslaan van dit adres is niet gelukt." toast. Reported against inquiry 156423, a `no_pile` sample — exactly the case the field exists for. The frontend is correct: - `report.inquiry_sample.foundation_depth` is on the `report.height` domain, same as `construction_level`, `groundwater_level_temp` and `mason_level`. The real measures (`concrete_charger_length`, `pile_distance_length`) are on `report.length`. - The risk model computes `foundation_depth - groundwater_level - 0.6` (FunderMapsWorker `sql/model/recreate_model_risk_dynamic_all.sql`), which is only meaningful on a shared datum. `groundwater_level_temp` is unambiguously NAP (9,925 of 13,812 rows negative). The legacy C# `InquirySample.FoundationDepth` `[Range(0.0, 999.99)]` is simply wrong; a comment now records that so it does not get "restored". Nothing else reads the field — the Webservice never exposed it, and the remaining C# usages are in the retired WebApi path. No data migration: only 3 rows ever had a value, all junk (an all-zeros row plus two test reports named "Cheese" and "Test"). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Problem
A Data Studio user could not save "Onderkant fundering" and got the generic
Opslaan van dit adres is niet gelukt.toast, which reads like a database fault.It isn't — it's a validator/frontend contract mismatch.
foundationDepthwas validated withnumericLength(gte(0)), while ClientApprenders the field with the
LEVELpreset (unitm NAP, min-999.99). Everybelow-datum value — i.e. essentially every real one — came back a 400.
Reported against inquiry 156423 (
no_pilesample, Hoofdweg 80A Lijnden),which is precisely the non-piled case the field's own hint says it's for. The
record has been stuck since 5 August.
Of the 18 numeric sample fields, this was the only mismatch between the
ClientApp registry and the API validators.
Which side is wrong
The frontend is right; the API validator is the bug:
report.inquiry_sample.foundation_depthsits on thereport.heightdomain— the same one as
construction_level,groundwater_level_temp,mason_level.The genuine measures (
concrete_charger_length,pile_distance_length) sit onreport.length.foundation_depth - groundwater_level - 0.6(FunderMapsWorker
sql/model/recreate_model_risk_dynamic_all.sql:184), which isonly meaningful if both are on the same datum.
groundwater_level_tempisunambiguously NAP: 9,925 of 13,812 filled rows are negative, min
-44.00.The legacy C#
InquirySample.FoundationDepth[Range(0.0, 999.99)]is simplywrong. A comment now records that, so it doesn't get "restored" by someone
diffing against the monolith.
Blast radius
interaction with the August EOL work.
all-zeros row plus two test reports named "Cheese" and "Test" (with
groundlevelof 34 m and 68 m).Follow-up
PR #98's contract test pins enum mappings only — numeric bounds have no
equivalent, which is why a frontend relabel (ClientApp
bceedae, #265,23 July) could diverge from the validator unnoticed. Worth extending to the
numeric presets once #98 lands.
🤖 Generated with Claude Code