From d972002c2708b163ac3b4a4e4fddec040d811f48 Mon Sep 17 00:00:00 2001 From: benbenbang Date: Wed, 30 Sep 2026 20:08:08 +0800 Subject: [PATCH 1/3] feat(codecs): add empty_or_null deserializer for nullable typed codes This adds a new field codec that decodes a required wire key holding a typed code whose absent value the provider sends as `""` or JSON null, per ADR 0033 (#368). Both spellings decode to `None`, while any other value is parsed with the type's `FromStr`, so whitespace-only or control-bearing input remains a decode error that never echoes the value. The key stays required, matching `required_option`, and `None` re-encodes as null through serde's default serialization. **New codec:** * Added the `empty_or_null` module in `crates/libfmp/src/codecs.rs`, applied via `deserialize_with = "crate::codecs::empty_or_null::deserialize"` (the exact path gen_go matches). * Added a test covering empty/null-to-`None` mapping, round-trip serialization, missing-field errors, and that decode errors never contain the offending value. **Documentation:** * Added ADR 0033 describing nullable response members. * Extended ADR 0001 to cover observed wire evidence where a typed code or date seen as `""` or null decodes to `None`. * Updated ADR 0030 to document the fifth Go field codec introduced by ADR 0033. --- crates/libfmp/src/codecs.rs | 95 +++++++++++++++++++++- docs/adr/0001-v0.1-public-architecture.md | 3 +- docs/adr/0030-go-sdk-architecture.md | 7 +- docs/adr/0033-nullable-response-members.md | 71 ++++++++++++++++ 4 files changed, 173 insertions(+), 3 deletions(-) create mode 100644 docs/adr/0033-nullable-response-members.md diff --git a/crates/libfmp/src/codecs.rs b/crates/libfmp/src/codecs.rs index 606904c7..a7e1398f 100644 --- a/crates/libfmp/src/codecs.rs +++ b/crates/libfmp/src/codecs.rs @@ -708,6 +708,35 @@ pub mod empty_date { } } +/// Deserializer for a required wire key holding a typed code whose absent +/// value the provider sends as `""` or JSON null (ADR 0033, #368). +/// +/// Both spellings decode to `None`; any other string is parsed with the +/// type's `FromStr`, so a whitespace-only or control-bearing value is still a +/// decode error, and that error never includes the value. The key stays +/// required, as with `required_option`: do not add `default` at call sites. +/// Apply with `deserialize_with = "crate::codecs::empty_or_null::deserialize"` +/// (the exact path gen_go matches); serialization is serde's default, so +/// `None` re-encodes as null. +pub(crate) mod empty_or_null { + use super::*; + + use crate::types::StringValueError; + + #[cfg_attr(not(test), allow(dead_code))] + pub(crate) fn deserialize<'de, D, T>(deserializer: D) -> Result, D::Error> + where + D: Deserializer<'de>, + T: FromStr, + { + match Option::::deserialize(deserializer)? { + None => Ok(None), + Some(value) if value.is_empty() => Ok(None), + Some(value) => value.parse().map(Some).map_err(de::Error::custom), + } + } +} + /// Serializers for response fields typed with an integral-`f64` alias. /// /// The provider documents fields such as [`Volume`](crate::types::Volume) @@ -802,7 +831,71 @@ pub(crate) mod count { mod tests { use serde::{Deserialize, Serialize}; - use crate::types::{Count, Volume}; + use crate::types::{Count, CountryCode, Ticker, Volume}; + + #[test] + fn empty_or_null_maps_empty_and_null_to_none_and_parses_the_rest() { + #[derive(Debug, PartialEq, Serialize, Deserialize)] + struct Row { + #[serde(deserialize_with = "super::empty_or_null::deserialize")] + removed_ticker: Option, + #[serde(deserialize_with = "super::empty_or_null::deserialize")] + country_code: Option, + } + let decode = |text: &str| serde_json::from_str::(text); + let from_value = + |text: &str| serde_json::from_value::(serde_json::from_str(text).unwrap()); + + for text in [ + r#"{"removed_ticker":"","country_code":null}"#, + r#"{"removed_ticker":null,"country_code":""}"#, + ] { + let row = decode(text).unwrap(); + assert_eq!(row.removed_ticker, None); + assert_eq!(row.country_code, None); + assert_eq!(from_value(text).unwrap(), row); + } + let row = decode(r#"{"removed_ticker":"BRK.B","country_code":"US"}"#).unwrap(); + assert_eq!(row.removed_ticker, Some(Ticker::new("BRK.B").unwrap())); + assert_eq!(row.country_code, Some(CountryCode::new("US").unwrap())); + assert_eq!( + serde_json::to_string(&row).unwrap(), + r#"{"removed_ticker":"BRK.B","country_code":"US"}"# + ); + assert_eq!( + serde_json::to_string(&Row { + removed_ticker: None, + country_code: None, + }) + .unwrap(), + r#"{"removed_ticker":null,"country_code":null}"# + ); + + for (text, secret) in [ + ( + r#"{"removed_ticker":"SECRET,X","country_code":"US"}"#, + "SECRET", + ), + (r#"{"removed_ticker":" ","country_code":"US"}"#, " "), + ( + r#"{"removed_ticker":"A","country_code":"SECRET\u0007"}"#, + "SECRET", + ), + ] { + let error = decode(text).unwrap_err().to_string(); + assert!(from_value(text).is_err(), "from_value {text}"); + assert!( + !error.contains(secret), + "the error must not echo the value: {error}" + ); + } + assert!(decode(r#"{"removed_ticker":"A","country_code":3}"#).is_err()); + let missing = decode(r#"{"removed_ticker":"A"}"#).unwrap_err().to_string(); + assert!( + missing.contains("missing field `country_code`"), + "{missing}" + ); + } #[test] fn count_accepts_integers_and_integral_floats_on_both_decode_paths() { diff --git a/docs/adr/0001-v0.1-public-architecture.md b/docs/adr/0001-v0.1-public-architecture.md index 456172ce..30460d80 100644 --- a/docs/adr/0001-v0.1-public-architecture.md +++ b/docs/adr/0001-v0.1-public-architecture.md @@ -163,7 +163,8 @@ Exchange, currency, and country codes are open string newtypes, not closed enums, because the provider's supported values can grow. Documented provider sentinels such as an empty string or `"N/A"` are converted to `None` only on fields whose `outer.md` examples establish that behavior; sentinels are not -valid identifiers. +valid identifiers. ADR 0033 extends this to observed wire evidence: a typed +code or date seen as `""` or null decodes to `None`. The public API also provides: diff --git a/docs/adr/0030-go-sdk-architecture.md b/docs/adr/0030-go-sdk-architecture.md index 978c92ae..daa87c70 100644 --- a/docs/adr/0030-go-sdk-architecture.md +++ b/docs/adr/0030-go-sdk-architecture.md @@ -176,7 +176,7 @@ struct with pointer fields for every required member and an `UnmarshalJSON` that reports a `Decode` error naming the first missing field. Unknown members are ignored, as in Rust (no `deny_unknown_fields` exists in the crate). -Four field codecs need more than a pointer, because json/v2 sets a pointer +Four field codecs (five since ADR 0033) need more than a pointer, because json/v2 sets a pointer to nil for both a missing key and a JSON null. Their shadow member is the raw `jsontext.Value` (empty means missing) and the generated decoder finishes the job: `deserialize_with = "required_option"` (key required, null allowed, @@ -192,6 +192,11 @@ the struct otherwise. `FiscalYear` has no Go codec on purpose: no public model uses it, and the generator fails if one appears rather than widening it to `NumberOrString`. +The fifth, from ADR 0033, is +`deserialize_with = "crate::codecs::empty_or_null::deserialize"` on an +`Option` of a string-backed typed code: key required, `""` and null are +`nil`, public field `*string`. + ### Generator placement `crates/fmp-py-gen` gains a `gen_go` binary. The `syn` parser and model IR diff --git a/docs/adr/0033-nullable-response-members.md b/docs/adr/0033-nullable-response-members.md new file mode 100644 index 00000000..1cec8424 --- /dev/null +++ b/docs/adr/0033-nullable-response-members.md @@ -0,0 +1,71 @@ +# ADR 0033: Nullable response members and empty-string sentinels + +- Status: Accepted +- Date: 2026-09-30 +- Decision owners: libfmp maintainers +- Scope: response models in Rust, Python, and Go; implemented by #368 + (1.2.0) and its follow-up fix issues + +## Context + +Response members were typed from the `outer.md` examples, which rarely show +a null. The 2026-09-30 live audit on +[#368](https://github.com/bitbrew-dev/libfmp/issues/368) (every registry path, 4012 +requests) found 194 rows that fail to decode today: a member the examples +show as a value arrives as `null`, or a typed code such as a country code, +ISIN, or CUSIP arrives as `""`, which the non-empty string newtypes of +ADR 0001 reject. One such member fails the whole response. + +ADR 0001 allowed sentinels to become `None` only where `outer.md` +establishes it, and ADR 0030 lists the field codecs Go mirrors. Observed wire +evidence is now a second source, so both need a rule. + +## Decision + +| Wire value observed on a member | Rust | Python | Go | +| --- | --- | --- | --- | +| `null` | `Option`, key still required through the file's `required_option` | `Optional[T]` | `*T` | +| `""` on a typed code (`CountryCode`, `Isin`, `Ticker`, ...) | `Option` through `codecs::empty_or_null` | `None` | `nil` | +| `""` on a date | `Option` through `codecs::empty_or_null_date` | `None` | `nil` | +| `""` on a plain `String` | unchanged: stays `""` | `""` | `""` | + +- Evidence decides: a member changes only when the audit (or a later + report) saw it null or empty. Parity siblings are not widened. +- `codecs::empty_or_null` accepts `""` and null as `None` and parses any + other string with the type's validation, so a whitespace-only or + control-bearing value is still an error; its message never includes the + value. `None` re-encodes as `null`. +- No invented values: a null amount never becomes `0` or `NaN`, and a row + is never dropped to hide a bad member. +- A member whose non-null value has the wrong type (a year where a date is + documented, the text `"NULL"`) is a separate type fix, not an `Option`. +- Every change is breaking and ships in a minor release with a + `BREAKING CHANGE:` footer naming the members. + +### Go + +`gen_go` maps `deserialize_with = "crate::codecs::empty_or_null::deserialize"` +on an `Option` of a string-backed type to `Codec::EmptyOrNullString`: the +shadow member keeps the raw `jsontext.Value`, the key is required, and +`""` or null decode to `nil`. It fails on any other shape rather than +guessing. + +## Alternatives considered + +- **Reject `""` on typed codes.** Rejected: the audit saw it on live rows + (ETF holdings, fund disclosures, insider Form 3 filings), so every such + response would fail. +- **Keep `""` as a value of the typed code.** Rejected: it breaks the + non-empty guarantee of ADR 0001 and makes callers test two spellings of + absent. +- **Default null amounts to `0.0` or `NaN`.** Rejected: that invents a + financial figure. +- **Make every member optional.** Rejected: it hides the documented + contract and pushes checks onto every caller for members never seen null. + +## Consequences + +- ADR 0001's sentinel rule and ADR 0030's codec list point here. +- More `Option` members across the three SDKs; callers handle `None`. +- New evidence of a null or `""` is a small, local model change under this + rule, with a decode test proving it. From a399e23f99c81cbbfd988e90d80c802a0853bdbb Mon Sep 17 00:00:00 2001 From: benbenbang Date: Wed, 30 Sep 2026 20:10:15 +0800 Subject: [PATCH 2/3] feat(gen-go): add empty_or_null string codec for Go models This pull request adds support for the `empty_or_null` typed-code codec in the Go model generator, so Rust fields using `crate::codecs::empty_or_null::deserialize` on an `Option` of a string-backed type map to a nil-able Go pointer. **Codec support:** * Added the `EmptyOrNullString` variant and its `EMPTY_OR_NULL_CODEC` path, mapping an `Option` of a string-backed scalar to a `*string` public type backed by a `jsontext.Value` shadow, and rejecting any other shape with an explanatory error. * Treated the key as required and decoded both `""` and null to nil through the raw shadow, per ADR 0033. **Testing:** * Added a test that decodes empty and null values to nil through the raw shadow, checks the rendered public and shadow tags, the required-key handling, and the generated unmarshal body, and asserts that non-string and bare shapes fail with an `empty_or_null` error. --- crates/fmp-py-gen/src/bin/gen_go/models.rs | 12 ++++++ crates/fmp-py-gen/src/bin/gen_go/tests.rs | 49 ++++++++++++++++++++++ crates/fmp-py-gen/src/bin/gen_go/types.rs | 18 ++++++++ 3 files changed, 79 insertions(+) diff --git a/crates/fmp-py-gen/src/bin/gen_go/models.rs b/crates/fmp-py-gen/src/bin/gen_go/models.rs index a75d5379..07c3e71b 100644 --- a/crates/fmp-py-gen/src/bin/gen_go/models.rs +++ b/crates/fmp-py-gen/src/bin/gen_go/models.rs @@ -221,6 +221,7 @@ fn render_model(model: &ModelPlan, out: &mut String) { Codec::RequiredOption | Codec::RequiredNumber | Codec::Count + | Codec::EmptyOrNullString | Codec::EmptyDate | Codec::EmptyOrNullDate => local_name(rust_name), }; @@ -312,6 +313,7 @@ fn render_required_switch(model: &ModelPlan, out: &mut String) { Codec::RequiredOption | Codec::RequiredNumber | Codec::Count + | Codec::EmptyOrNullString | Codec::EmptyDate | Codec::EmptyOrNullDate | Codec::DynamicJson => format!("len(shadow.{}) == 0", field.name), @@ -354,6 +356,16 @@ fn render_codec_block(model: &str, field: &GoField, local: &str, out: &mut Strin field.public_ty, field.name, field.name ); } + Codec::EmptyOrNullString => { + let inner = field.public_ty.trim_start_matches('*'); + let _ = writeln!( + out, + "\tvar {local} {}\n\tif shadow.{}.Kind() != 'n' {{\n\t\tvar value {inner}\n\ + \t\tif err := json.Unmarshal(shadow.{}, &value); err != nil {{\n\ + \t\t\treturn err\n\t\t}}\n\t\tif value != \"\" {{\n\t\t\t{local} = &value\n\t\t}}\n\t}}", + field.public_ty, field.name, field.name + ); + } Codec::EmptyDate | Codec::EmptyOrNullDate => { let allow_null = field.codec == Codec::EmptyOrNullDate; let _ = writeln!( diff --git a/crates/fmp-py-gen/src/bin/gen_go/tests.rs b/crates/fmp-py-gen/src/bin/gen_go/tests.rs index 44c4265a..3ff4d852 100644 --- a/crates/fmp-py-gen/src/bin/gen_go/tests.rs +++ b/crates/fmp-py-gen/src/bin/gen_go/tests.rs @@ -648,6 +648,55 @@ fn count_codec_decodes_through_the_raw_shadow_and_keeps_the_public_uint64() { } } +#[test] +fn empty_or_null_codec_decodes_empty_and_null_to_nil_through_the_raw_shadow() { + let aliases = BTreeMap::new(); + let structs = Vec::new(); + let table = TypeTable::new(&structs, &aliases); + let codec = "crate::codecs::empty_or_null::deserialize"; + let def = row(vec![ + field("removed_ticker", "Option", deserialize_with(codec)), + field("isin", "Option", deserialize_with("required_option")), + ]); + let mapped = table.go_field(&def, &def.fields[0]).expect("maps"); + assert_eq!( + ( + mapped.public_ty.as_str(), + mapped.shadow_ty.as_str(), + mapped.codec, + mapped.required_key(), + ), + ("*string", "jsontext.Value", Codec::EmptyOrNullString, true) + ); + let models = plan_models("test", &[&def], &table).expect("plans"); + let rendered = render_models("test", &models); + for expected in [ + "RemovedTicker *string `json:\"removedTicker\"`", + "RemovedTicker jsontext.Value `json:\"removedTicker\"`", + "case len(shadow.RemovedTicker) == 0:", + "var removedTicker *string\n\tif shadow.RemovedTicker.Kind() != 'n' {", + "if err := json.Unmarshal(shadow.RemovedTicker, &value); err != nil {", + "if value != \"\" {\n\t\t\tremovedTicker = &value\n\t\t}", + "RemovedTicker: removedTicker,", + ] { + assert!( + rendered.contains(expected), + "missing {expected:?} in\n{rendered}" + ); + } + + for (name, ty) in [ + ("bare", "Ticker"), + ("price", "Option"), + ("many", "Option>"), + ("date", "Option"), + ] { + let def = row(vec![field(name, ty, deserialize_with(codec))]); + let error = table.go_field(&def, &def.fields[0]).expect_err(name); + assert!(error.contains("empty_or_null"), "{name}: {error}"); + } +} + #[test] fn skip_serializing_if_none_marks_only_the_public_tag_omitzero() { let aliases = BTreeMap::new(); diff --git a/crates/fmp-py-gen/src/bin/gen_go/types.rs b/crates/fmp-py-gen/src/bin/gen_go/types.rs index ba880a9c..6e459d1f 100644 --- a/crates/fmp-py-gen/src/bin/gen_go/types.rs +++ b/crates/fmp-py-gen/src/bin/gen_go/types.rs @@ -36,6 +36,10 @@ pub(crate) enum Codec { /// `Count`: the key is required, null is rejected, and an integral float /// such as `3.0` decodes as the integer. The shadow keeps the raw value. Count, + /// `deserialize_with = "crate::codecs::empty_or_null::deserialize"` on an + /// `Option` of a string-backed type: the key is required, and `""` and + /// null are both absent (ADR 0033). The shadow keeps the raw value. + EmptyOrNullString, /// A `DynamicObject` that holds every member the named fields do not /// claim, as serde `flatten` on a map does: `#[serde(flatten)]`, or the /// single `DynamicObject` field of a struct with a hand-written @@ -47,6 +51,9 @@ pub(crate) enum Codec { /// The `deserialize_with` path of the Rust count codec. const COUNT_CODEC: &str = "crate::codecs::count::deserialize"; +/// The `deserialize_with` path of the Rust empty-or-null typed-code codec. +const EMPTY_OR_NULL_CODEC: &str = "crate::codecs::empty_or_null::deserialize"; + /// One Go struct member derived from a Rust field. #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct GoField { @@ -80,6 +87,7 @@ impl GoField { Codec::RequiredOption | Codec::RequiredNumber | Codec::Count + | Codec::EmptyOrNullString | Codec::EmptyDate | Codec::EmptyOrNullDate | Codec::DynamicJson @@ -189,6 +197,16 @@ impl<'a> TypeTable<'a> { .to_string(), )); } + (Some(EMPTY_OR_NULL_CODEC), BaseKind::Scalar, [Wrap::Option]) + if base.go == "string" => + { + (Codec::EmptyOrNullString, "jsontext.Value".to_string()) + } + (Some(EMPTY_OR_NULL_CODEC), _, _) => { + return Err(fail( + "the empty_or_null codec needs an Option of a string-backed type".to_string(), + )); + } (Some("required_option"), _, _) => { return Err(fail( "required_option needs an Option field" From ace3b6a14f32928aa18647c27320e4ea70134945 Mon Sep 17 00:00:00 2001 From: benbenbang Date: Wed, 30 Sep 2026 20:13:31 +0800 Subject: [PATCH 3/3] docs(wiki): add ADR 0033 on nullable response members Link ADR 0033 in the design index, covering nullable response members and how `""` or null on typed codes and dates decode to `None` / `nil` based on observed wire evidence. --- wiki/design.md | 1 + 1 file changed, 1 insertion(+) diff --git a/wiki/design.md b/wiki/design.md index d1ef73ca..fb184853 100644 --- a/wiki/design.md +++ b/wiki/design.md @@ -44,4 +44,5 @@ hold in every language: - [ADR 0030](../docs/adr/0030-go-sdk-architecture.md): the Go SDK architecture, including why it is generated rather than bound through cgo. - [ADR 0031](../docs/adr/0031-pre-1.0-contract-decisions.md): pre-1.0 contract decisions (error equality without a source chain, `#[non_exhaustive]` responses, `f64` quantities, exact numeric strings, Python model equality, the `fmp` import name). - [ADR 0032](../docs/adr/0032-naming-policy.md): the naming policy for methods, row types, and Go initialisms across all three SDKs. +- [ADR 0033](../docs/adr/0033-nullable-response-members.md): nullable response members, and `""` or null on typed codes and dates decoding to `None` / `nil`, from observed wire evidence. - [Contract ambiguities register](../docs/contract-ambiguities.md): where the captured documentation is silent or contradictory, the SDK keeps an explicit raw type, a closed query, or a strict decode rather than a guessed schema. The register lists those deferred decisions.