Skip to content

fix(workbook): make the zoned and sparkline wire forms canonical-only #774

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hhimanshu merged 2 commits into from
Jul 28, 2026
Merged

fix(workbook): make the zoned and sparkline wire forms canonical-only #774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2a566a9
fix(workbook): describe zoned and sparkline in the published schema (…
hhimanshu Jul 28, 2026
5b85273
fix(workbook): make the zoned and sparkline wire forms canonical-only…
hhimanshu Jul 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
171 changes: 170 additions & 1 deletion crates/workbook/schema/workbook.v1.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"$schema": "https://json-schema.org/draft/2020-12/schema",
"$id": "https://truecalc.dev/schema/workbook.v1.schema.json",
"title": "TrueCalc Workbook v1",
"description": "Structural schema for a TrueCalc workbook document (schema spec v1). Rules that JSON Schema cannot express -- canonical key ordering, ECMAScript number formatting, spill reconstruction, duplicate-key rejection, simple-case-fold uniqueness, named-range ref canonicality, and resource limits -- are governed by the prose spec and enforced by Workbook::from_json, not here.",
"description": "Structural schema for a TrueCalc workbook document (schema spec v1). Rules that JSON Schema cannot express -- canonical key ordering, ECMAScript number formatting, spill reconstruction, duplicate-key rejection, simple-case-fold uniqueness, named-range ref canonicality, and resource limits -- are governed by the prose spec and enforced by Workbook::from_json, not here. Validate with a draft 2020-12 validator: a draft-07 validator silently ignores the `prefixItems` keyword used by `sparklineOption`, which drops all inner validation of a sparkline's option pairs and quietly accepts pairs this schema rejects.",
"type": "object",
"additionalProperties": false,
"required": [
Expand Down Expand Up @@ -189,6 +189,12 @@
}
}
},
{
"$ref": "#/$defs/zonedValue"
},
{
"$ref": "#/$defs/sparklineValue"
},
{
"type": "object",
"additionalProperties": false,
Expand Down Expand Up @@ -312,6 +318,169 @@
"type": "number"
}
}
},
{
"$ref": "#/$defs/zonedValue"
},
{
"$ref": "#/$defs/sparklineValue"
}
]
},
"zonedValue": {
"description": "A zone-aware instant, carried as a self-describing RFC-9557 string: an RFC-3339 timestamp optionally suffixed with the zone in brackets. The serializer always emits one canonical spelling -- local wall clock, a `T` separator, whole seconds, a numeric +HH:MM / -HH:MM offset, and the bracketed zone for a named IANA zone but not for a bare fixed offset. The pattern is deliberately wider, because the reader accepts every RFC-3339 spelling and normalizes it: a lower-case `t`, a space separator, `Z`/`z` for UTC, fractional seconds, and a leap second (`:60`). It is sized to the reader on purpose -- a pattern narrower than the reader would report a loadable document as malformed. Three things it still cannot express, so a string may match here and be rejected on load: calendar validity (`2026-04-31` matches), the representable instant range (an i64 nanosecond count, so roughly 1677-09-21 to 2262-04-11 -- `9999-01-01` matches), and whether a bracketed name is a zone the pinned tzdb actually knows.",
"type": "object",
"additionalProperties": false,
"required": [
"type",
"value"
],
"properties": {
"type": {
"const": "zoned"
},
"value": {
"type": "string",
"pattern": "^[0-9]{4}-(0[1-9]|1[0-2])-(0[1-9]|[12][0-9]|3[01])[Tt ]([01][0-9]|2[0-3]):[0-5][0-9]:([0-5][0-9]|60)(\\.[0-9]+)?([Zz]|[+-]([01][0-9]|2[0-3]):[0-5][0-9])(\\[[A-Za-z0-9_+:/-]+\\])?$"
}
}
},
"sparklineValue": {
"description": "A sparkline: the parsed, validated render spec produced by SPARKLINE, carried in full because it is part of the value's storage identity.",
"type": "object",
"additionalProperties": false,
"required": [
"type",
"value"
],
"properties": {
"type": {
"const": "sparkline"
},
"value": {
"$ref": "#/$defs/sparklineSpec"
}
}
},
"sparklineSpec": {
"type": "object",
"additionalProperties": false,
"required": [
"charttype",
"data",
"options"
],
"properties": {
"charttype": {
"description": "Lifted out of the option list into its own field; `line` when the option was omitted. Canonical lower-case only: SPARKLINE's own argument matching is case-insensitive, but the wire form is not, exactly as for option keys.",
"enum": [
"line",
"bar",
"column",
"winloss"
]
},
"data": {
"description": "The points to plot, flattened row-major from the data argument. A single-point sparkline is unrepresentable -- the evaluator answers #N/A -- so there are always at least two.",
"type": "array",
"minItems": 2,
"items": {
"$ref": "#/$defs/sparklinePoint"
}
},
"options": {
"description": "The remaining option key/value pairs in the order given. Keys the engine does not recognise are kept rather than rejected, matching Sheets.",
"type": "array",
"items": {
"$ref": "#/$defs/sparklineOption"
}
}
}
},
"sparklineOption": {
"description": "One [key, value] pair. Keys are stored ASCII-lower-cased, and `charttype` is never among them -- it has its own field.",
"type": "array",
"minItems": 2,
"maxItems": 2,
"prefixItems": [
{
"type": "string",
"pattern": "^[^A-Z]*$",
"not": {
"const": "charttype"
}
},
{
"$ref": "#/$defs/sparklinePoint"
}
]
},
"sparklinePoint": {
"description": "A sparkline data point or option value. Narrower than scalarValue: only these four value types can occupy either position.",
"oneOf": [
{
"type": "object",
"additionalProperties": false,
"required": [
"type",
"value"
],
"properties": {
"type": {
"const": "number"
},
"value": {
"type": "number"
}
}
},
{
"type": "object",
"additionalProperties": false,
"required": [
"type",
"value"
],
"properties": {
"type": {
"const": "text"
},
"value": {
"type": "string"
}
}
},
{
"type": "object",
"additionalProperties": false,
"required": [
"type",
"value"
],
"properties": {
"type": {
"const": "boolean"
},
"value": {
"type": "boolean"
}
}
},
{
"type": "object",
"additionalProperties": false,
"required": [
"type",
"value"
],
"properties": {
"type": {
"const": "empty"
},
"value": {
"type": "null"
}
}
}
]
}
Expand Down
68 changes: 65 additions & 3 deletions crates/workbook/src/value.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ use truecalc_core::types::{SparklineChartType, SparklineSpec, SparklineValue, Zo
/// array is collapsed to its scalar element before storage, schema spec
/// §6), and holds only scalar values (never a nested `Array`). It appears
/// only as a spill anchor's value (schema spec §5).
/// - `Sparkline` plots at least two points, and its option keys are lower-case
/// and never `charttype`. The serializer rejects a spec that breaks this and
/// the deserializer refuses one, so a spec that can be written can be read.
/// - `Zoned` is written, and must be read, as an unpadded RFC-9557 string: the
/// wire form is canonical, even where the formula-level parsers it delegates
/// to are lenient about casing and surrounding whitespace.
#[derive(Debug, Clone)]
pub enum Value {
/// Finite IEEE-754 f64.
Expand Down Expand Up @@ -221,6 +227,30 @@ struct SparklineSpecWire<'a>(&'a SparklineSpec);

impl Serialize for SparklineSpecWire<'_> {
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
// Enforce the reader's invariants at the writing end too, exactly as
// the `Array` arm of `Value`'s serializer does for its own shape rules:
// a `SparklineSpec` is a public struct, so it can be hand-built in a
// state the evaluator never produces, and emitting it would yield bytes
// that neither `parse_sparkline` nor the published schema accepts —
// breaking this crate's round-trip guarantee.
if self.0.data.len() < 2 {
return Err(S::Error::custom(
"a sparkline plots at least two points; a shorter spec does not exist \
in serialized form (the evaluator answers #N/A for one point)",
));
}
for (key, _) in &self.0.options {
if *key != key.to_ascii_lowercase() {
return Err(S::Error::custom(format!(
"a sparkline option key must be lower-case, got {key:?}"
)));
}
if key == "charttype" {
return Err(S::Error::custom(
"charttype is carried by the sparkline's own field, not in options",
));
}
}
let data: Vec<Value> = self.0.data.iter().map(sparkline_value_to_value).collect();
let options: Vec<(&str, Value)> = self
.0
Expand Down Expand Up @@ -365,9 +395,7 @@ fn parse_value(raw: &serde_json::Value) -> Result<Value, String> {
"number" => Ok(Value::Number(parse_finite_f64(payload, kind)?)),
"date" => Ok(Value::Date(parse_finite_f64(payload, kind)?)),
"zoned" => match payload.as_str() {
Some(s) => parse_rfc9557(s)
.map(|zi| Value::Zoned(Box::new(zi)))
.ok_or_else(|| format!("a zoned value must be a valid RFC-9557 string, got {s:?}")),
Some(s) => parse_zoned(s),
None => Err("a zoned value must be a JSON string".to_string()),
},
"text" => match payload.as_str() {
Expand Down Expand Up @@ -414,6 +442,28 @@ fn parse_finite_f64(payload: &serde_json::Value, kind: &str) -> Result<f64, Stri
Ok(if n == 0.0 { 0.0 } else { n })
}

/// Parse the payload of a `zoned` value: the canonical RFC-9557 string, in the
/// same shape the serializer emits.
///
/// `parse_rfc9557` trims the string, and trims the bracketed zone inside it,
/// because the *formula* level has to accept a padded argument. The wire is
/// canonical-only, so padding is rejected here rather than silently absorbed —
/// the serializer never emits it, and accepting it would put a document on disk
/// that the published schema calls malformed.
fn parse_zoned(s: &str) -> Result<Value, String> {
let zone = s
.split_once('[')
.and_then(|(_, rest)| rest.strip_suffix(']'));
if s.trim() != s || zone.is_some_and(|z| z.trim() != z) {
return Err(format!(
"a zoned value must not be padded with whitespace, got {s:?}"
));
}
parse_rfc9557(s)
.map(|zi| Value::Zoned(Box::new(zi)))
.ok_or_else(|| format!("a zoned value must be a valid RFC-9557 string, got {s:?}"))
}

/// Parse the payload of a `sparkline` value: the full parsed spec, in the same
/// shape [`SparklineSpecWire`] emits.
fn parse_sparkline(payload: &serde_json::Value) -> Result<Value, String> {
Expand All @@ -434,8 +484,20 @@ fn parse_sparkline(payload: &serde_json::Value) -> Result<Value, String> {
let raw_chart_type = obj["charttype"]
.as_str()
.ok_or_else(|| "a sparkline charttype must be a JSON string".to_string())?;
// `SparklineChartType::parse` is ASCII case-insensitive because the
// *formula* level has to accept `=SPARKLINE({1,2},{"charttype","LINE"})`.
// The wire is canonical-only — as it already is for option keys below — so
// a non-canonical spelling is rejected here rather than silently
// normalized: the serializer never emits one, and accepting one would put
// a document on disk that the published schema calls malformed.
let chart_type = SparklineChartType::parse(raw_chart_type)
.ok_or_else(|| format!("unknown sparkline charttype {raw_chart_type:?}"))?;
if chart_type.as_str() != raw_chart_type {
return Err(format!(
"a sparkline charttype must be spelled in its canonical lower-case \
form, got {raw_chart_type:?}"
));
}

let raw_data = obj["data"]
.as_array()
Expand Down
Loading
Loading