From f8beed04cff834169942302ce661c6e99a0193f2 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Thu, 13 Aug 2026 01:36:56 +0000 Subject: [PATCH] Enforce generated text report identity --- docs/internal/generatedtext.md | 12 ++- docs/templates.md | 5 +- go.mod | 2 +- internal/generatedtext/catalog.go | 17 +++- internal/generatedtext/catalog_test.go | 56 +++++++++-- internal/generatedtext/json.go | 46 +++++++++ internal/generatedtext/json_test.go | 130 +++++++++++++++++++++++++ 7 files changed, 251 insertions(+), 17 deletions(-) create mode 100644 internal/generatedtext/json_test.go diff --git a/docs/internal/generatedtext.md b/docs/internal/generatedtext.md index 678a426..14b378a 100644 --- a/docs/internal/generatedtext.md +++ b/docs/internal/generatedtext.md @@ -9,9 +9,10 @@ maintainer-facing context fields belong to [report templates](../templates.md). ## Catalog and validation The Daily, Today, Tomorrow, and Hourly report definitions each use structured -generated text. `LookupDefinition` rejects unknown schema or template IDs and -unsupported schema/template pairs before the run begins. A handler validates raw JSON, returns a typed -value and canonical normalized JSON, loads its canonical schema through +generated text. `LookupDefinition` requires the exact report, schema, and +template triple and rejects unknown IDs, unsupported pairs, and a pair that +belongs to another report before the run begins. A handler validates raw JSON, +returns a typed value and canonical normalized JSON, loads its canonical schema through `internal/promptassets`, builds a render context, and renders through `internal/reporttemplate`. @@ -19,8 +20,9 @@ Daily, Today, and Tomorrow use a day-style value with required trimmed summary and one or more nonblank discussion paragraphs. Hourly requires trimmed summary and a single trimmed discussion string. Every form also requires the `precipitation_timing` field; an empty string means there is no supported timing -prose to render. Typed decoding rejects missing required fields and unknown JSON -fields; no general-purpose JSON Schema engine is used at runtime. +prose to render. Typed decoding requires the exact lowercase JSON field names, +rejects missing, duplicate, case-variant, and unknown fields, and checks field +shapes; no general-purpose JSON Schema engine is used at runtime. ## Render contexts diff --git a/docs/templates.md b/docs/templates.md index 48b53aa..c11e31b 100644 --- a/docs/templates.md +++ b/docs/templates.md @@ -20,8 +20,9 @@ source: | Hourly | `templates/hourly.md.tmpl` (`hourly`) | `hourly` | `weather.hourly_generated_text`; `internal/promptassets/assets/prompts/hourly/` | The matching schemas and Promptkit definitions are embedded by -`internal/promptassets`. The generated-text catalog pairs each schema ID with -its template ID; keep the matching prompt definition aligned with that pair. +`internal/promptassets`. The generated-text catalog requires each report's +exact schema/template pair; keep the matching prompt definition aligned with +that report-specific triple. Shared partials are under `internal/reporttemplate/templates/partials/`: diff --git a/go.mod b/go.mod index 82562c2..3b6e55d 100644 --- a/go.mod +++ b/go.mod @@ -7,10 +7,10 @@ require gopkg.in/yaml.v3 v3.0.1 require ( gitea.maximumdirect.net/eric/distributor v0.5.0 gitea.maximumdirect.net/eric/promptkit v0.5.0 + github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 golang.org/x/sys v0.45.0 ) require ( - github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 // indirect golang.org/x/text v0.14.0 // indirect ) diff --git a/internal/generatedtext/catalog.go b/internal/generatedtext/catalog.go index 43a7612..38575c8 100644 --- a/internal/generatedtext/catalog.go +++ b/internal/generatedtext/catalog.go @@ -28,6 +28,7 @@ type validator func([]byte) (any, []byte, error) type renderContextBuilder func(report.ID, string, briefing.Metadata, module.Snapshot, facts.CollectedFacts, facts.DerivedFacts, any) (any, error) type catalogEntry struct { + reportID report.ID schemaID string templateID string validate validator @@ -44,24 +45,28 @@ type Handler struct { var catalog = []catalogEntry{ { + reportID: report.Hourly, schemaID: schemaIDHourly, templateID: templateIDHourly, validate: validateHourly, renderContextBuilder: buildHourlyContext, }, { + reportID: report.Daily, schemaID: schemaIDDaily, templateID: templateIDDaily, validate: validateDaily, renderContextBuilder: buildDailyContext, }, { + reportID: report.Today, schemaID: schemaIDToday, templateID: templateIDToday, validate: validateToday, renderContextBuilder: buildTodayContext, }, { + reportID: report.Tomorrow, schemaID: schemaIDTomorrow, templateID: templateIDTomorrow, validate: validateTomorrow, @@ -70,8 +75,7 @@ var catalog = []catalogEntry{ } func LookupDefinition(definition report.Definition) (Handler, error) { - - var schemaKnown, templateKnown bool + var schemaKnown, templateKnown, pairKnown bool for _, entry := range catalog { if entry.schemaID == definition.GeneratedTextSchemaID { schemaKnown = true @@ -80,8 +84,12 @@ func LookupDefinition(definition report.Definition) (Handler, error) { templateKnown = true } if entry.schemaID == definition.GeneratedTextSchemaID && entry.templateID == definition.TemplateID { + pairKnown = true + if entry.reportID != definition.ID { + continue + } return Handler{ - reportID: definition.ID, + reportID: entry.reportID, schemaID: entry.schemaID, templateID: entry.templateID, validate: entry.validate, @@ -95,6 +103,9 @@ func LookupDefinition(definition report.Definition) (Handler, error) { if !templateKnown { return Handler{}, fmt.Errorf("report template %q is not supported for report %q", definition.TemplateID, definition.ID) } + if pairKnown { + return Handler{}, fmt.Errorf("generated text schema %q and report template %q do not belong to report %q", definition.GeneratedTextSchemaID, definition.TemplateID, definition.ID) + } return Handler{}, fmt.Errorf("generated text schema %q and report template %q are not supported together for report %q", definition.GeneratedTextSchemaID, definition.TemplateID, definition.ID) } diff --git a/internal/generatedtext/catalog_test.go b/internal/generatedtext/catalog_test.go index 61d0078..eb285b3 100644 --- a/internal/generatedtext/catalog_test.go +++ b/internal/generatedtext/catalog_test.go @@ -8,17 +8,32 @@ import ( ) func TestCatalogCompleteForGeneratedTextTemplateReports(t *testing.T) { - for _, definition := range report.DefaultRegistry().All() { - t.Run(string(definition.ID), func(t *testing.T) { + expected := []struct { + reportID report.ID + schemaID string + templateID string + }{ + {reportID: report.Daily, schemaID: "daily", templateID: "daily"}, + {reportID: report.Today, schemaID: "today", templateID: "today"}, + {reportID: report.Tomorrow, schemaID: "tomorrow", templateID: "tomorrow"}, + {reportID: report.Hourly, schemaID: "hourly", templateID: "hourly"}, + } + registry := report.DefaultRegistry() + for _, expected := range expected { + t.Run(string(expected.reportID), func(t *testing.T) { + definition := registry.MustLookup(expected.reportID) + if definition.GeneratedTextSchemaID != expected.schemaID || definition.TemplateID != expected.templateID { + t.Fatalf("definition = %#v, want schema/template %q/%q", definition, expected.schemaID, expected.templateID) + } handler, err := LookupDefinition(definition) if err != nil { t.Fatalf("LookupDefinition() error = %v", err) } - if handler.SchemaID() != definition.GeneratedTextSchemaID { - t.Fatalf("SchemaID() = %q, want %q", handler.SchemaID(), definition.GeneratedTextSchemaID) + if handler.SchemaID() != expected.schemaID { + t.Fatalf("SchemaID() = %q, want %q", handler.SchemaID(), expected.schemaID) } - if handler.TemplateID() != definition.TemplateID { - t.Fatalf("TemplateID() = %q, want %q", handler.TemplateID(), definition.TemplateID) + if handler.TemplateID() != expected.templateID { + t.Fatalf("TemplateID() = %q, want %q", handler.TemplateID(), expected.templateID) } if handler.validate == nil { t.Fatal("validator is nil") @@ -40,6 +55,35 @@ func TestCatalogCompleteForGeneratedTextTemplateReports(t *testing.T) { } } +func TestCatalogRejectsKnownPairForAnotherReport(t *testing.T) { + pairs := []struct { + reportID report.ID + schemaID string + templateID string + }{ + {reportID: report.Daily, schemaID: "daily", templateID: "daily"}, + {reportID: report.Today, schemaID: "today", templateID: "today"}, + {reportID: report.Tomorrow, schemaID: "tomorrow", templateID: "tomorrow"}, + {reportID: report.Hourly, schemaID: "hourly", templateID: "hourly"}, + } + registry := report.DefaultRegistry() + for _, definitionPair := range pairs { + for _, mismatchedPair := range pairs { + if definitionPair.reportID == mismatchedPair.reportID { + continue + } + t.Run(string(definitionPair.reportID)+"/"+string(mismatchedPair.reportID), func(t *testing.T) { + definition := registry.MustLookup(definitionPair.reportID) + definition.GeneratedTextSchemaID = mismatchedPair.schemaID + definition.TemplateID = mismatchedPair.templateID + if _, err := LookupDefinition(definition); err == nil || !strings.Contains(err.Error(), "do not belong") { + t.Fatalf("LookupDefinition(%#v) error = %v, want wrong-report error", definition, err) + } + }) + } + } +} + func TestCatalogLookupRejectsUnsupportedSchemaAndTemplate(t *testing.T) { tests := []struct { name string diff --git a/internal/generatedtext/json.go b/internal/generatedtext/json.go index 7d3897d..8e07e8c 100644 --- a/internal/generatedtext/json.go +++ b/internal/generatedtext/json.go @@ -8,8 +8,17 @@ import ( "strings" ) +var generatedTextFieldNames = map[string]struct{}{ + "summary": {}, + "forecast_discussion": {}, + "precipitation_timing": {}, +} + func decodeGeneratedText[T any](data []byte, name string) (T, error) { var value T + if err := validateGeneratedTextObject(data, name); err != nil { + return value, err + } decoder := json.NewDecoder(bytes.NewReader(data)) decoder.DisallowUnknownFields() if err := decoder.Decode(&value); err != nil { @@ -25,6 +34,43 @@ func decodeGeneratedText[T any](data []byte, name string) (T, error) { return value, fmt.Errorf("decode %s generated text: multiple JSON values", name) } +func validateGeneratedTextObject(data []byte, name string) error { + decoder := json.NewDecoder(bytes.NewReader(data)) + token, err := decoder.Token() + if err != nil { + return fmt.Errorf("decode %s generated text: %w", name, err) + } + if delimiter, ok := token.(json.Delim); !ok || delimiter != '{' { + return fmt.Errorf("decode %s generated text: expected JSON object", name) + } + seen := make(map[string]struct{}, len(generatedTextFieldNames)) + for decoder.More() { + token, err := decoder.Token() + if err != nil { + return fmt.Errorf("decode %s generated text: %w", name, err) + } + field, ok := token.(string) + if !ok { + return fmt.Errorf("decode %s generated text: expected object field name", name) + } + if _, ok := generatedTextFieldNames[field]; !ok { + return fmt.Errorf("decode %s generated text: json: unknown field %q", name, field) + } + if _, ok := seen[field]; ok { + return fmt.Errorf("decode %s generated text: duplicate field %q", name, field) + } + seen[field] = struct{}{} + var value json.RawMessage + if err := decoder.Decode(&value); err != nil { + return fmt.Errorf("decode %s generated text: %w", name, err) + } + } + if _, err := decoder.Token(); err != nil { + return fmt.Errorf("decode %s generated text: %w", name, err) + } + return nil +} + func requireGeneratedTextStringField(data []byte, name, field string) error { var fields map[string]json.RawMessage if err := json.Unmarshal(data, &fields); err != nil { diff --git a/internal/generatedtext/json_test.go b/internal/generatedtext/json_test.go new file mode 100644 index 0000000..6f39618 --- /dev/null +++ b/internal/generatedtext/json_test.go @@ -0,0 +1,130 @@ +package generatedtext + +import ( + "bytes" + "fmt" + "strings" + "testing" + + "gitea.maximumdirect.net/eric/weatherreporter/internal/promptassets" + "github.com/santhosh-tekuri/jsonschema/v6" +) + +func TestGeneratedTextValidatorsRequireExactObjectFields(t *testing.T) { + validators := []struct { + name string + valid string + validate func([]byte) error + }{ + {name: "daily", valid: `{"summary":"Summary","forecast_discussion":["Discussion"],"precipitation_timing":""}`, validate: validateDailyJSON}, + {name: "today", valid: `{"summary":"Summary","forecast_discussion":["Discussion"],"precipitation_timing":""}`, validate: validateTodayJSON}, + {name: "tomorrow", valid: `{"summary":"Summary","forecast_discussion":["Discussion"],"precipitation_timing":""}`, validate: validateTomorrowJSON}, + {name: "hourly", valid: `{"summary":"Summary","forecast_discussion":"Discussion","precipitation_timing":""}`, validate: validateHourlyJSON}, + } + for _, validator := range validators { + t.Run(validator.name, func(t *testing.T) { + for _, test := range []struct { + name string + input string + valid bool + }{ + {name: "canonical", input: validator.valid, valid: true}, + {name: "missing", input: strings.Replace(validator.valid, `"summary":"Summary",`, "", 1)}, + {name: "additional", input: strings.Replace(validator.valid, "}", `,"extra":"value"}`, 1)}, + {name: "case variant summary", input: strings.Replace(validator.valid, `"summary"`, `"Summary"`, 1)}, + {name: "case variant discussion", input: strings.Replace(validator.valid, `"forecast_discussion"`, `"Forecast_Discussion"`, 1)}, + {name: "duplicate", input: strings.Replace(validator.valid, `"summary":"Summary",`, `"summary":"Summary","summary":"Other",`, 1)}, + } { + t.Run(test.name, func(t *testing.T) { + err := validator.validate([]byte(test.input)) + if (err == nil) != test.valid { + t.Fatalf("validate(%s) error = %v, want valid = %t", test.input, err, test.valid) + } + }) + } + }) + } +} + +func TestTypedValidatorsMatchEmbeddedSchemaObjectShape(t *testing.T) { + validators := []struct { + name string + valid string + validate func([]byte) error + }{ + {name: "daily", valid: `{"summary":"Summary","forecast_discussion":["Discussion"],"precipitation_timing":""}`, validate: validateDailyJSON}, + {name: "today", valid: `{"summary":"Summary","forecast_discussion":["Discussion"],"precipitation_timing":""}`, validate: validateTodayJSON}, + {name: "tomorrow", valid: `{"summary":"Summary","forecast_discussion":["Discussion"],"precipitation_timing":""}`, validate: validateTomorrowJSON}, + {name: "hourly", valid: `{"summary":"Summary","forecast_discussion":"Discussion","precipitation_timing":""}`, validate: validateHourlyJSON}, + } + for _, validator := range validators { + t.Run(validator.name, func(t *testing.T) { + schema := generatedTextSchema(t, validator.name) + for _, test := range []struct { + name string + input string + }{ + {name: "canonical", input: validator.valid}, + {name: "missing", input: strings.Replace(validator.valid, `"summary":"Summary",`, "", 1)}, + {name: "additional", input: strings.Replace(validator.valid, "}", `,"extra":"value"}`, 1)}, + {name: "case variant", input: strings.Replace(validator.valid, `"summary"`, `"Summary"`, 1)}, + {name: "null", input: strings.Replace(validator.valid, `"precipitation_timing":""`, `"precipitation_timing":null`, 1)}, + {name: "wrong type", input: strings.Replace(validator.valid, `"summary":"Summary"`, `"summary":false`, 1)}, + } { + t.Run(test.name, func(t *testing.T) { + instance, err := jsonschema.UnmarshalJSON(strings.NewReader(test.input)) + if err != nil { + t.Fatalf("parse schema instance: %v", err) + } + schemaValid := schema.Validate(instance) == nil + typedValid := validator.validate([]byte(test.input)) == nil + if schemaValid != typedValid { + t.Fatalf("schema valid = %t, typed valid = %t for %s", schemaValid, typedValid, test.input) + } + }) + } + }) + } +} + +func generatedTextSchema(t *testing.T, id string) *jsonschema.Schema { + t.Helper() + data, err := promptassets.Schema(id) + if err != nil { + t.Fatalf("Schema(%q) error = %v", id, err) + } + document, err := jsonschema.UnmarshalJSON(bytes.NewReader(data)) + if err != nil { + t.Fatalf("parse schema %q: %v", id, err) + } + compiler := jsonschema.NewCompiler() + location := fmt.Sprintf("https://weatherreporter.example/schemas/%s.json", id) + if err := compiler.AddResource(location, document); err != nil { + t.Fatalf("add schema %q: %v", id, err) + } + schema, err := compiler.Compile(location) + if err != nil { + t.Fatalf("compile schema %q: %v", id, err) + } + return schema +} + +func validateDailyJSON(data []byte) error { + _, _, err := ValidateDaily(data) + return err +} + +func validateTodayJSON(data []byte) error { + _, _, err := ValidateToday(data) + return err +} + +func validateTomorrowJSON(data []byte) error { + _, _, err := ValidateTomorrow(data) + return err +} + +func validateHourlyJSON(data []byte) error { + _, _, err := ValidateHourly(data) + return err +}