From 3bca2f41f702aaeb38d1f70f8b9966460983ad4b Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 2 Aug 2026 05:12:36 +0000 Subject: [PATCH] Define comparison artifact contracts --- docs/roadmap/implementation.md | 800 +++++++++++++++++++++++++ docs/roadmap/profile-comparison.md | 15 +- internal/comparison/comparison.go | 386 ++++++++++++ internal/comparison/comparison_test.go | 314 ++++++++++ 4 files changed, 1508 insertions(+), 7 deletions(-) create mode 100644 docs/roadmap/implementation.md create mode 100644 internal/comparison/comparison.go create mode 100644 internal/comparison/comparison_test.go diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md new file mode 100644 index 0000000..2634d09 --- /dev/null +++ b/docs/roadmap/implementation.md @@ -0,0 +1,800 @@ +# LLM Profile Comparison Implementation Plan + +Status: Ready for implementation. + +## Purpose And Authority + +This document is the ordered implementation plan for the accepted [LLM +Profile Comparison Roadmap](profile-comparison.md). Implement every stage in +numeric order. The roadmap owns feature purpose, policy, scope, and target +state; this plan owns implementation sequence, concrete interfaces, and stage +exit criteria. + +This is one feature delivered over ten bounded coding prompts. A stage may +refactor code needed by a later stage, but it must leave the repository +compiling, tested, and internally coherent. Do not expose an incomplete +`compare` command before the stages that implement its full command contract. + +## Implementation Rules + +Apply these rules in every stage: + +- Read `docs/development.md`, the task-specific documents it identifies, and + all files under `docs/policy/` before changing code. Treat those documents + and the feature roadmap as binding. +- Preserve the existing behavior of `generate` and `run`, including output + precedence, report/date semantics, notification, debug capture, structured + results, and exit behavior. +- Keep Promptkit types and calls behind `internal/adapters/promptkit` and the + dependency-neutral `internal/promptexec` interface. Do not import Promptkit + from application, CLI, or comparison-artifact packages. +- Keep comparison artifacts operator-owned and explicit. They are not durable + application state and must never be discovered or consumed implicitly by a + later invocation. +- Do not add a Weatherreporter concurrency limit or semaphore. One shared + Promptkit executor is safe for concurrent use, and Promptkit v0.5.0 owns its + engine-local backend capacity. +- Use dependency injection and deterministic fakes for application and CLI + tests. Tests must be offline, credential-free, race-safe, and independent of + completion timing and machine-specific paths. +- Add regression coverage in the same stage as each behavior. Prefer + contract-level assertions over incidental implementation detail, in + accordance with `docs/policy/testing.md`. +- Run `gofmt` on changed Go files and `git diff --check` in every stage. Run + focused tests while developing, then `GOWORK=off go test -count=1 ./...` + before completing the stage. Stages that add concurrency or filesystem + replacement must also run the relevant packages with `-race`. +- Do not commit, tag, push, or prepare a release unless the implementing prompt + separately requests it. + +## Locked Implementation Contracts + +The following choices make the stages decision-complete. Do not substitute a +different command shape, artifact schema, publication algorithm, or concurrency +policy without first updating the accepted roadmap and this plan. + +### Application Request And Result + +Add this exported application request, using the existing application aliases +and interfaces: + +```go +type ComparisonRequest struct { + Config config.Config + Report ReportKind + ProfileIDs []string + WorkingDir string + OutputDir string + Replace bool + LLMDebugDir string + Date time.Time + Clock timeutil.Clock + Collector Collector + Executor promptexec.Executor +} +``` + +`OutputDir` is the optional exact bundle-directory override. `Clock` supplies +the comparison identity plus start and finish timestamps and defaults to +`timeutil.SystemClock` when nil. `Date` retains the existing report-date +meaning. + +Do not include a notifier in this request. Making notification structurally +unavailable is clearer and safer than passing a disabled notifier through the +comparison path. + +Add `app.CompareDetailed(context.Context, ComparisonRequest)` returning a +non-nil `*app.ComparisonResult` whenever report and destination resolution have +progressed far enough to describe the attempted comparison. Use these field +names and meanings: + +```go +type ComparisonResult struct { + ComparisonID string + ReportID report.ID + ReportName string + PromptID string + PromptVersion string + PromptHash string + StartedAt time.Time + FinishedAt time.Time + Timezone string + ValidPeriod timeutil.Period + OutputDirectory string + ManifestPath string + DataPackagePath string + Total int + Succeeded int + Failed int + Results []ComparisonProfileResult +} + +type ComparisonProfileResult struct { + Position int + ProfileID string + BackendID string + ModelName string + Status string + ValidationStatus promptexec.ValidationStatus + ReportPath string + LLMDebugPath string + Error *comparison.SafeError +} +``` + +`OutputDirectory` is absolute once destination resolution succeeds. Manifest +and data-package paths are absolute and nonblank only after publication. +Successful report paths and any returned debug paths are also absolute. +Profile results always follow selection order. + +Profile counts describe profile outcomes only. A publication failure after all +profiles succeed therefore leaves `total == succeeded` and `failed == 0`, while +the command-level status and returned error still report failure. + +Reject fewer than two profile IDs, whitespace-only IDs, and exact +case-sensitive duplicates. Preserve every accepted ID byte-for-byte and in +request order; do not trim, case-fold, deduplicate, or merge it with +`config.promptkit.profile`. Validate this invariant both in the CLI and at the +application boundary. + +### Comparison And File Identity + +Use the resolved report metadata's existing run ID to form: + +```text +comparison_ +``` + +This comparison ID is computed once from the injected clock and shared by the +application result, manifest, and debug identities. + +Derive a profile filename slug as follows: + +1. retain ASCII letters, ASCII digits, `-`, and `_`; +2. replace each run of every other character, including dots, whitespace, + separators, control characters, and non-ASCII characters, with one `-`; +3. trim leading and trailing `-` and `_`; +4. use `profile` if the result is empty; and +5. truncate the result to 64 ASCII bytes, then trim trailing `-` and `_` again, + falling back to `profile` if necessary. + +The report filename is: + +```text +-.md +``` + +The one-based ordinal width is the greater of two and the number of decimal +digits in the profile count. Filename collisions after slug normalization are +safe because the ordinal is authoritative. The exact profile ID remains the +logical identity in results and the manifest. + +Derive a default comparison directory by requiring the resolved report output +name to end in the exact `.md` suffix, removing that suffix, and prefixing the +remaining basename with `comparison-`. Resolve it beneath configured +`output.directory`, or beneath the invocation working directory when no output +directory is configured. An explicit `--out-dir` is the exact bundle directory, +resolves relative to the invocation working directory, and completely +overrides configured output. Return an absolute cleaned path. + +### Versioned Bundle Contract + +Create a dependency-neutral `internal/comparison` package. It owns deterministic +naming, the logical bundle and manifest types, validation, recognition of prior +bundles, hashing, staging, and transactional publication. It must not import +CLI, configuration, Promptkit, collectors, or report-generation packages. + +Use the schema version: + +```text +weatherreporter.comparison.v1 +``` + +The UTF-8 `comparison.json` document is indented by two spaces and ends with one +newline. Its exact top-level JSON fields, in struct declaration order, are: + +```text +schemaVersion +comparisonId +startedAt +finishedAt +reportId +validPeriod +timezone +promptId +promptVersion +promptHash +dataPackage +total +succeeded +failed +results +``` + +`validPeriod` has `start` and `end`. `dataPackage` has `path` and `sha256`. +Each ordered result has: + +```text +position +profileId +backendId (omit when empty) +modelName +status +validationStatus (omit when unavailable) +reportPath (omit for failures) +error (omit for successes) +``` + +An error object has `category` and `message`. Status values are exactly +`succeeded` and `failed`. A successful result must have a relative report path, +must not have an error, and must have passed generated-text validation. A +failed result must not have a report path and must have a safe error. Use the +dependency-neutral string returned by `promptexec.CategoryOf` when available; +use `application` for an unclassified application, validation, rendering, or +debug-callback failure. Bound a manifest or normal-summary error message to +1,024 UTF-8 bytes without splitting an encoded rune. Never serialize wrapped +causes, provider bodies, prompts, schemas, model output, endpoints, or +credentials. + +Encode all manifest timestamps using Go's standard `time.Time` JSON encoding. +Comparison start and finish timestamps are UTC; valid-period start and end +retain their resolved timezone offsets. Require nonzero timestamps, a finish +that is not before the comparison start, and a valid-period end strictly after +its start. + +Use lowercase 64-character hexadecimal SHA-256 digests. `data-package.yml` +contains the exact immutable byte slice supplied to every execution. A logical +bundle contains that data package, ordered successful Markdown report bytes, +and the complete manifest. Manifest counts and paths must agree exactly with +the ordered results and logical bundle. + +### Destination Safety And Publication + +Perform a read-only destination preflight before prompt inspection, weather +collection, directory creation, or model execution. Reject: + +- a filesystem root; +- the exact invocation working directory; +- an existing regular file or non-directory; +- an existing symlink, including a dangling symlink; +- an uninspectable target; and +- a nonempty directory unless `--replace` is present and it is a recognized + current-schema Weatherreporter comparison bundle. + +An absent target and an existing empty real directory are accepted without +`--replace`. `--replace` is also harmless for those cases. Recognition must +decode the manifest with unknown fields rejected, validate every manifest +invariant, require `data-package.yml` and every declared successful report to +be regular non-symlink files, verify the data-package digest, and require the +directory entries to be exactly the manifest, data package, and declared +successful reports. Extra files, subdirectories, links, missing files, invalid +paths, duplicate paths, or a failed digest make the directory unrecognized. +Never follow a manifest path outside the bundle. + +Re-run the destination safety check immediately before publication. Build the +entire logical bundle in memory, create the parent only when publishing, and +write a private uniquely named sibling staging directory with directory mode +`0700` and files mode `0600`. Check cancellation while staging and immediately +before the commit operation. + +For an absent target, atomically rename the completed sibling staging directory +to the target. For an existing empty or recognized target, rename the target to +a unique sibling backup, rename staging to the target, and restore the backup +if the second rename fails. If restoration also fails, retain the backup and +return an actionable joined error that identifies its path. Once this rename +transaction begins, finish commit or rollback without abandoning it because of +a concurrent cancellation. The final pre-commit context check is the +linearization point: cancellation observed before it prevents replacement; +cancellation arriving after it does not retroactively undo a completed commit. +Remove the backup after a successful replacement and clean staging artifacts on +ordinary failures without scanning or modifying unrelated paths. + +### Shared Preparation And Concurrent Execution + +Extract an immutable internal prepared-report value containing the resolved +report, collection-derived facts, module snapshot, briefing metadata, +generated-text handler/render inputs, source warnings, and the one serialized +data-package byte sequence. Preparation resolves, collects, derives, snapshots, +and marshals once. + +Split the generated-text workflow into three internal responsibilities: + +1. deterministic shared preparation; +2. one profile-specific Promptkit execution, validation, and in-memory + Markdown rendering; and +3. publication, with optional ordinary notification. + +`GenerateDetailed` composes those responsibilities once and retains its current +single-report behavior. Comparison never calls `GenerateDetailed` in a loop. + +For comparison, inspect the exact prompt once, then inspect every profile +sequentially in selection order, including effective backend, model, and +credential availability, before weather collection. Initialize an explicitly +requested debug writer before those operations so pre-execution diagnostics +retain current behavior. Any preflight failure causes no collection, model +calls, or bundle publication. + +After shared preparation, start one goroutine per profile against one shared +executor. Pass the same immutable data-package bytes and exact inspected prompt +version to every call. Preallocate an ordered outcome slice and let each +goroutine write only its own index; use synchronization to join all goroutines. +One profile failure does not cancel peers. Parent cancellation propagates to +every call, no additional calls are started after cancellation is observed, and +all started goroutines are joined before return. + +Give each execution a deterministic debug run identity: + +```text +_- +``` + +The existing debug preparation artifact remains authoritative for the exact +profile ID. Debug callback failure is a failure of only that profile and must +not collide with or cancel peers. + +After all non-cancelled outcomes finish, order them by original selection, +capture one UTC finish timestamp, and construct the complete or coherent +partial bundle. Publish partial bundles when one or more profiles fail, then +return the non-nil result with a generic aggregate error so the CLI exits +nonzero. Do not include provider detail in that aggregate error. Cancellation +or deadline expiration publishes no bundle. No comparison path constructs or +calls a Distributor notifier. + +### CLI Contract + +Add: + +```text +weatherreporter compare REPORT [options] +``` + +Support `daily`, `today`, `tomorrow`, and `hourly`, with the same date policy as +`generate`. Accept applicable existing `--config`, `--units`, `--tz`, `--date`, +`--llm-debug-dir`, and `--quiet` flags plus a repeatable custom `--profile`, the +exact-directory `--out-dir`, and boolean `--replace`. Do not accept `--out`, and +do not use the configured default Promptkit profile as a selection. Construct +exactly one executor per command invocation. + +The normal stdout JSON summary has these fields in declaration order: + +```text +command +comparisonId +reportId +reportName +promptId +promptVersion +promptHash +status +startedAt +finishedAt +timezone +validPeriod +outputDirectory +manifestPath (omit when unpublished) +dataPackagePath (omit when unpublished) +total +succeeded +failed +results +error (omit on complete success) +``` + +`command` is `compare`; command status is `succeeded` only when every profile +succeeds and publication succeeds, otherwise `failed`. Each result mirrors the +application profile result with absolute report and debug paths. Preserve +profile order. A coherent partial bundle is still a failed command and exits +nonzero. Failures before a structured result exists use the existing CLI error +path without inventing a partial JSON object. `--quiet` suppresses normal +stdout and routine progress output but never changes work, artifacts, or exit +status. + +## Stage 1: Define Comparison Artifact And Naming Contracts + +### Goal + +Introduce the dependency-neutral logical contract without altering any command +or existing generation path. + +### Work + +1. Create `internal/comparison` with manifest, result, data-package reference, + valid-period, safe-error, logical-bundle, and bundle-report types matching + the locked schema. +2. Add constants for the schema version, `comparison.json`, and + `data-package.yml`, plus explicit succeeded and failed statuses. +3. Implement profile-list validation, deterministic slugging, ordinal width, + report filename construction, comparison ID construction, default directory + name derivation, SHA-256 formatting, safe UTF-8 error truncation, manifest + validation, and deterministic JSON encoding. +4. Ensure validation rejects inconsistent counts, positions, statuses, paths, + digests, timestamps, identities, successful results without reports, + failures with reports, duplicate paths, non-basename artifact paths, and + traversal-capable names. +5. Keep all filesystem mutation and application orchestration out of this + stage. + +### Tests And Exit Criteria + +- Table-test unusual profile IDs, empty slugs, separators, dots, controls, + Unicode, length limits, normalized collisions, and profile counts above 99. +- Round-trip the exact JSON field names, ordering, indentation, newline, + optional fields, safe errors, hashes, and manifest invariants. +- Confirm exact case-sensitive profile identity and duplicate handling. +- Run `GOWORK=off go test -count=1 ./internal/comparison` and the repository-wide + standard test command. The repository remains behaviorally unchanged. + +## Stage 2: Implement Safe Bundle Recognition And Transactional Publication + +### Goal + +Complete the isolated filesystem boundary, including output resolution and +whole-directory replacement, before application concurrency depends on it. + +### Work + +1. Add read-only destination planning and preflight to `internal/comparison`, + parameterized by the absolute working directory, exact target, and replace + authorization. +2. Implement strict current-schema prior-bundle recognition with unknown JSON + fields rejected, manifest validation, exact-entry checks, symlink rejection, + regular-file checks, containment, and digest verification. +3. Implement sibling staging, fixed permissions, cancellation checks, absent + target publication, existing-target backup and replacement, rollback, and + targeted cleanup according to the locked algorithm. +4. Add an application output resolver, alongside existing output helpers, that + applies explicit/configured/current-directory precedence and derives the + default comparison name from the resolved report output name. Do not change + ordinary output resolution. +5. Return typed or inspectable errors sufficient for application and CLI code + to add context without exposing filesystem internals as a public schema. + +### Tests And Exit Criteria + +- Cover relative and absolute explicit destinations, configured destination + precedence, all report-derived names, and an invalid default `.md` suffix. +- Cover absent, empty, regular-file, symlink, dangling-symlink, filesystem-root, + exact-working-directory, unreadable, recognized, and unrecognized targets. +- Cover extra entries, missing reports, subdirectories, manifest traversal, + unknown fields, bad counts, digest mismatch, stale-report removal, successful + replacement, second-rename failure with restoration, and cancellation before + commit. Use narrow injected filesystem hooks only where real filesystem + behavior cannot deterministically reach a consequential failure path. +- Verify a failed publication leaves the prior recognized bundle byte-for-byte + intact and does not touch sibling paths. +- Run focused tests with `-race`, then the repository-wide standard tests. + +## Stage 3: Add Ordered Multi-Profile Preflight + +### Goal + +Make prompt and profile validation reusable for comparison without collecting +weather or executing a model. + +### Work + +1. Extend `internal/app/prompt_inspection.go` with a comparison-oriented request + and result that inspect one resolved prompt exactly once and selected + profiles sequentially in request order. +2. Reuse the existing credential-lookup policy and return dependency-neutral + prompt hash/version plus effective backend and model for every profile. +3. Apply the locked application-level profile validation before any executor + inspection. +4. Fail on the first ordered preflight error with prompt/profile context, while + ensuring no collection or execution occurs. Do not silently fall back to + `config.promptkit.profile`. +5. Leave existing single and batch inspection entry points intact unless a + private shared helper can remove duplication without changing behavior. + +### Tests And Exit Criteria + +- Assert one prompt inspection, ordered profile inspections, effective + backend/model preservation, exact profile IDs, and credential failures. +- Assert invalid lists and any prompt/profile preflight failure perform no + collection or execution using explicit fakes. +- Run `GOWORK=off go test -count=1 ./internal/app` and repository-wide tests. + +## Stage 4: Extract Immutable Shared Report Preparation + +### Goal + +Create the prepare-once boundary required for comparison while preserving all +ordinary generation behavior. + +### Work + +1. Extract an unexported immutable prepared-report value in `internal/app` with + the resolved report, facts, module snapshot, briefing metadata, handler and + render inputs, source warnings, and exact serialized data-package bytes. +2. Move current collection, derivation, snapshot, metadata, prompt-input, YAML + marshal, and generated-text-handler construction into one preparation + function. +3. Refactor `GenerateDetailed` and batch generation to use that function once + per generated report without altering external requests or results. +4. Copy mutable inputs where necessary at the boundary so concurrent consumers + cannot mutate shared maps or byte slices. Treat the prepared value as + read-only after construction. +5. Do not add comparison goroutines or the CLI command in this stage. + +### Tests And Exit Criteria + +- Preserve all existing generation and batch tests. +- Add focused tests showing one collector call, one deterministic serialized + package, stable warnings/metadata, and isolation from caller mutation. +- Compare ordinary generated report bytes, result fields, debug preparation, + notification, and failure behavior before and after the refactor through + existing contract tests. +- Run application tests and repository-wide tests. + +## Stage 5: Separate Profile Execution And In-Memory Rendering From Publication + +### Goal + +Permit several profiles to consume one prepared report without duplicating or +prematurely publishing output. + +### Work + +1. Extract a profile-specific internal function that accepts the immutable + prepared report, exact prompt inspection, effective profile inspection, and + optional deterministic debug reference. +2. Have it invoke `promptexec.Executor.Execute`, preserve the existing + preparation callback contract, validate generated text, build render + context, and render Markdown to memory. Return a dependency-neutral outcome + and bytes; do not write the report or notify. +3. Extract ordinary single-report publication into a separate function that + performs the existing context check, atomic file write, and optional + Distributor notification. +4. Recompose `GenerateDetailed` from prepare, execute/render, and publish. Keep + its debug path, validation status, output path, notification, warnings, and + error wrapping unchanged. +5. Make debug callback bookkeeping local to one execution. Remove any shared + mutable workflow field that would race when the execution function is used + concurrently. + +### Tests And Exit Criteria + +- Prove execution/rendering alone performs no filesystem publication or + notification. +- Preserve ordinary success, validation failure, rendering failure, debug + callback failure, cancellation, atomic output, and notification tests. +- Confirm one ordinary generation still prepares and executes exactly once. +- Run application tests with `-race`, then repository-wide tests. + +## Stage 6: Build The Concurrent Ordered Profile Execution Core + +### Goal + +Implement and test concurrency independently of destination publication and CLI +assembly. + +### Work + +1. Add a private comparison execution coordinator in `internal/app` that takes + one immutable prepared report, one prompt inspection, ordered profile + inspections, comparison identity, optional debug writer, and one shared + executor. +2. Preallocate ordered outcomes, start one goroutine per selected profile while + observing parent cancellation, and join every started goroutine. Each + goroutine may write only its own result slot. +3. Pass the exact same data-package byte content and prompt version to every + call. Do not recollect, rebuild, marshal, retry, or use `GenerateDetailed`. +4. Continue peers after an individual execution, validation, rendering, or + debug callback failure. Convert every failure to a bounded safe structured + error while retaining a wrapped internal error for aggregate diagnosis. +5. Use requested-profile order, not completion order, for outcomes and report + filenames. Generate distinct debug identities from comparison ID, ordinal, + and safe slug. +6. On parent cancellation or deadline, propagate context, join all work, and + mark non-successful outcomes safely. Do not construct a publishable bundle + for a cancelled coordinator. + +### Tests And Exit Criteria + +- Use a barrier-based fake executor to prove overlap without sleeps and to + observe more than one in-flight call. +- Assert the exact prompt version and byte-for-byte identical data package for + eight to twelve profiles, deterministic result order under reversed + completion, and no data races. +- Assert one failure does not cancel peers, cancellation reaches all started + calls and joins them, and debug references are distinct and deterministic. +- Do not duplicate Promptkit's internal capacity tests. Retain adapter tests + showing that Weatherreporter delegates prepared execution through one + concurrency-safe engine. +- Run `GOWORK=off go test -race -count=1 ./internal/app + ./internal/adapters/promptkit`, then repository-wide tests. + +## Stage 7: Assemble Application-Level Comparison And Bundle Publication + +### Goal + +Expose the complete application use case with safe partial publication, +aggregate failure behavior, and no notification. + +### Work + +1. Add the locked exported application request, result, profile-result, and + `CompareDetailed` entry point. +2. Orchestrate in this exact order: validate request; resolve report/date and + comparison identity; resolve and preflight destination; initialize optional + debug writing; inspect prompt and every profile; collect and prepare once; + execute profiles concurrently; capture finish time; build manifest and + logical bundle; re-preflight and publish. +3. Build `data-package.yml` from the exact prepared bytes and include only + successful rendered reports. Derive manifest and application results from + the same ordered outcome source so they cannot drift. +4. On one or more profile failures, publish the coherent partial bundle, set + ordered result errors and counts, then return the non-nil result plus a + generic aggregate error such as `comparison completed with N failed + profiles`. +5. On cancellation, deadline, preflight, preparation, or publication failure, + publish nothing new. Preserve a recognized prior bundle. Return the most + complete safe result available and an error with useful operation context. +6. Populate absolute manifest, data-package, successful-report, and debug paths + only when those artifacts exist. Keep manifest paths relative basenames. +7. Ensure no Distributor notifier is accepted, created, or invoked anywhere in + this path. + +### Tests And Exit Criteria + +- Add assembled application tests for complete success, mixed profile failure, + all-profile failure, preflight failure, collection/preparation failure, + cancellation, publication failure, and authorized replacement. +- Assert prompt inspection once, profile inspection before collection, weather + collection once, serialized package reuse, completion-independent order, + exact counts/digest/paths, generic aggregate errors, and no model calls before + all preflight succeeds. +- Assert complete and partial comparison never notify and that cancellation or + publication failure preserves an existing bundle. +- Run application and comparison packages with `-race`, then repository-wide + tests. + +## Stage 8: Add Compare Command Parsing And Request Construction + +### Goal + +Introduce the complete command syntax and validation while keeping execution +injected and testable. + +### Work + +1. Extend `internal/cli.Runner` with an injectable comparison function, default + it to `app.CompareDetailed`, and add a private comparison command parser and + request-construction path. Do not route it from the public root switch until + Stage 9 can provide the complete summary and exit contract. +2. Implement a repeatable flag value that preserves every `--profile` token in + exact command-line order. +3. Parse applicable common flags plus `--out-dir` and `--replace`; reject + missing/unknown reports, unsupported date use, fewer than two profiles, + whitespace-only profiles, exact duplicates, unexpected positional + arguments, and unsupported flags such as `--out`. +4. Reuse existing configuration loading, units/timezone overrides, report/date + resolution semantics, working-directory handling, and debug-root behavior. +5. Pass explicit output override and ordered profiles to the application. Do + not merge the configured default profile. Construct one Promptkit executor + only after syntax/config validation and before the application call. +6. Return the application result and error through a private CLI execution + seam for Stage 9 to format. Do not add public root routing or help text in + this stage, so users cannot reach a command with an incomplete output + contract. + +### Tests And Exit Criteria + +- Table-test report/date behavior, repeated profiles and ordering, invalid + lists, output precedence inputs, `--replace`, common overrides, and + rejected flags/arguments. +- Assert invalid syntax and configuration do not construct an executor or call + the application. +- Call the private comparison execution seam in tests and assert one valid + invocation constructs one executor and one request with the exact selections + and paths. +- Run CLI tests and repository-wide tests. + +## Stage 9: Complete CLI Summaries, Quiet Mode, And Exit Behavior + +### Goal + +Finish the end-to-end user-facing command contract, including coherent partial +failure reporting. + +### Work + +1. Add comparison summary and ordered profile-summary types with the exact + locked JSON field order and `omitempty` behavior. +2. Add public root routing and root help text now that parsing, execution, + formatting, and exit behavior can land as one complete command. +3. Map complete, partial, cancelled, and publication-failed application results + into one normal stdout object whenever a structured result exists. Use + absolute returned paths and safe structured errors; do not serialize wrapped + internal or provider details. +4. Return success only for complete execution and publication. Return the + application error after writing a failed structured summary so the process + exits nonzero. Preserve the existing unstructured error path for failures + before a result exists. +5. Implement `--quiet` as output suppression only. It must not alter execution, + comparison artifacts, replacement, error return, or exit status. + +### Tests And Exit Criteria + +- Golden or exact-structure tests cover successful, partial, all-failed, + pre-execution, cancelled, and publication-failed summaries, field omission, + ordered results, and safe errors. +- CLI integration tests assert stdout/stderr discipline, quiet behavior, + nonzero partial failure, default and explicit output paths, and one executor + per invocation. +- Run CLI and application tests with `-race`, then repository-wide tests. + +## Stage 10: Reconcile Canonical Documentation And Run The Final Gate + +### Goal + +Document only the implemented behavior in its canonical owners, remove +roadmap-only language from current-state documentation, and perform the full +repository validation gate. + +### Work + +1. Update `docs/cli.md` with authoritative syntax, flags, profile validation, + JSON summary, quiet behavior, and exit semantics. +2. Update `docs/operations.md` with the prompt-comparison workflow, output + destination precedence by link to `docs/config.md`, sensitivity guidance, + flat bundle lifecycle, guarded replacement, partial results, cancellation, + and cleanup expectations. Do not duplicate the complete CLI reference. +3. Add `docs/integrations/comparison-bundle.md` as the canonical durable schema + and layout contract. Document the exact version, JSON fields, filenames, + path relativity, hashes, success/failure invariants, compatibility boundary, + and sensitive-content caveat. +4. Update `docs/integrations/promptkit.md` with the consumer-visible + prepare-once/execute-many boundary and Promptkit-owned backend capacity, + linking upstream rather than duplicating its concurrency reference. +5. Add or update focused `docs/internal/` documents for application comparison + orchestration, prepared report flow, concurrent execution, prompt debugging, + and bundle publication mechanics. Link to canonical CLI, configuration, + operations, integration, and architecture owners instead of restating them. +6. Update `docs/policy/architecture.md` for the implemented stateless, + concurrency, notification, and publication invariants; update + `docs/development.md` for new package/file routing; update + `docs/policy/documentation.md` only if the existing ownership table no longer + assigns every introduced topic correctly. +7. Check README and other current-state documents for navigation or stale + claims. Add only short orientation and canonical links where useful. +8. Mark the feature roadmap `Implemented; retained temporarily for + post-implementation review` and this plan `Complete`. Retain both until the + requested implementation review moves durable facts to their owners and the + maintainer authorizes roadmap cleanup. + +### Tests And Exit Criteria + +- Verify all changed relative links and fenced examples, and search for stale + claims that comparison is unavailable or planned. +- Confirm current-state docs do not duplicate full command, configuration, + schema, or implementation contracts outside their canonical owners. +- Run the release-equivalent local gate: + + ```sh + set -eu + test -z "$(git ls-files go.work go.work.sum)" + test ! -e vendor + if grep -Eq '^[[:space:]]*replace([[:space:]]|\()' go.mod; then + printf '%s\n' 'go.mod contains a replacement' >&2 + exit 1 + fi + GOWORK=off go test -count=1 ./... + GOWORK=off go test -race -count=1 ./... + GOWORK=off go vet ./... + GOWORK=off go build ./... + GOWORK=off go mod tidy -diff + unformatted="$(git ls-files '*.go' | while IFS= read -r file; do gofmt -l "$file"; done)" + test -z "$unformatted" + git diff --check + ``` + +- Run `GOWORK=off go run ./cmd/weatherreporter --help`; confirm output matches + `docs/cli.md` without requiring credentials or network access. +- Inspect the final diff for accidental generated artifacts, secrets, + workspaces, vendored dependencies, release notes, or unrelated changes. + +## Open Questions + +None. The accepted feature roadmap and the locked contracts in this plan define +the decisions required to implement all ten stages. diff --git a/docs/roadmap/profile-comparison.md b/docs/roadmap/profile-comparison.md index 24eaffd..96cd16f 100644 --- a/docs/roadmap/profile-comparison.md +++ b/docs/roadmap/profile-comparison.md @@ -280,8 +280,8 @@ The completed feature includes: - the `compare` CLI command for every implemented generated-text report; - repeatable explicit profile selection and validation; -- configured and CLI output-directory integration after the prerequisite - feature lands; +- configured and CLI output-directory integration through the implemented + destination policy; - one-time report resolution, collection, deterministic preparation, and YAML serialization; - concurrent execution through one Promptkit executor with backend capacity @@ -353,9 +353,10 @@ owns the statelessness, concurrency, notification, and publication invariants. The [Promptkit integration guide](../integrations/promptkit.md) should describe the consumer-visible multi-profile execution boundary without duplicating -Promptkit's backend-capacity reference. App orchestration, prompt input, -generated text, prompt debugging, and any new bundle implementation details -belong in focused documents under `docs/internal/`. +Promptkit's backend-capacity reference. The versioned manifest and flat bundle +format belong in a focused contract under `docs/integrations/`. App +orchestration, prompt input, generated text, prompt debugging, and bundle +publication mechanics belong in focused documents under `docs/internal/`. Current-state documentation must not describe profile comparison as available until the implementation lands. @@ -393,5 +394,5 @@ affected canonical documentation must describe the implemented behavior. ## Open Questions -None. The scope, prerequisites, user intent, and target behavior required for a -future staged implementation plan are defined above. +None. The scope, prerequisites, user intent, and target behavior are defined +above. diff --git a/internal/comparison/comparison.go b/internal/comparison/comparison.go new file mode 100644 index 0000000..24b5933 --- /dev/null +++ b/internal/comparison/comparison.go @@ -0,0 +1,386 @@ +// Package comparison defines the durable logical contract for profile +// comparison bundles. It deliberately has no filesystem or application +// orchestration dependencies. +package comparison + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "path" + "strings" + "time" + "unicode/utf8" +) + +const ( + // SchemaVersion identifies the supported comparison manifest schema. + SchemaVersion = "weatherreporter.comparison.v1" + + // ManifestFilename is the canonical name of a comparison manifest. + ManifestFilename = "comparison.json" + // DataPackageFilename is the canonical name of the shared prompt input. + DataPackageFilename = "data-package.yml" + + // StatusSucceeded identifies a profile that produced a validated report. + StatusSucceeded = "succeeded" + // StatusFailed identifies a profile that did not produce a report. + StatusFailed = "failed" + + maxProfileSlugBytes = 64 + maxErrorMessageBytes = 1024 +) + +// Manifest is the versioned, authoritative index of a comparison bundle. +// Field declaration order is the JSON field order. +type Manifest struct { + SchemaVersion string `json:"schemaVersion"` + ComparisonID string `json:"comparisonId"` + StartedAt time.Time `json:"startedAt"` + FinishedAt time.Time `json:"finishedAt"` + ReportID string `json:"reportId"` + ValidPeriod ValidPeriod `json:"validPeriod"` + Timezone string `json:"timezone"` + PromptID string `json:"promptId"` + PromptVersion string `json:"promptVersion"` + PromptHash string `json:"promptHash"` + DataPackage DataPackageReference `json:"dataPackage"` + Total int `json:"total"` + Succeeded int `json:"succeeded"` + Failed int `json:"failed"` + Results []Result `json:"results"` +} + +// ValidPeriod records the report's resolved half-open period. +type ValidPeriod struct { + Start time.Time `json:"start"` + End time.Time `json:"end"` +} + +// DataPackageReference identifies and verifies the shared prompt input. +type DataPackageReference struct { + Path string `json:"path"` + SHA256 string `json:"sha256"` +} + +// Result records one explicitly selected profile in selection order. +type Result struct { + Position int `json:"position"` + ProfileID string `json:"profileId"` + BackendID string `json:"backendId,omitempty"` + ModelName string `json:"modelName"` + Status string `json:"status"` + ValidationStatus string `json:"validationStatus,omitempty"` + ReportPath string `json:"reportPath,omitempty"` + Error *SafeError `json:"error,omitempty"` +} + +// SafeError contains bounded, operator-safe failure information only. +type SafeError struct { + Category string `json:"category"` + Message string `json:"message"` +} + +// LogicalBundle contains every byte required to publish a comparison without +// coupling the comparison contract to a filesystem implementation. +type LogicalBundle struct { + Manifest Manifest + DataPackage []byte + Reports []BundleReport +} + +// BundleReport is one rendered Markdown document in a logical bundle. +type BundleReport struct { + Position int + Path string + Markdown []byte +} + +// ValidateProfileIDs requires at least two distinct, nonblank profile IDs. It +// intentionally preserves accepted IDs unchanged and treats case distinctly. +func ValidateProfileIDs(profileIDs []string) error { + if len(profileIDs) < 2 { + return fmt.Errorf("comparison requires at least two profile IDs") + } + + seen := make(map[string]struct{}, len(profileIDs)) + for _, profileID := range profileIDs { + if strings.TrimSpace(profileID) == "" { + return fmt.Errorf("profile ID must not be blank") + } + if _, duplicate := seen[profileID]; duplicate { + return fmt.Errorf("duplicate profile ID %q", profileID) + } + seen[profileID] = struct{}{} + } + + return nil +} + +// ProfileSlug returns a deterministic, filesystem-safe representation of a +// logical profile ID. The logical ID remains authoritative in the manifest. +func ProfileSlug(profileID string) string { + var builder strings.Builder + lastReplacement := false + for _, r := range profileID { + if isSlugRune(r) { + builder.WriteRune(r) + lastReplacement = false + continue + } + if !lastReplacement { + builder.WriteByte('-') + lastReplacement = true + } + } + + slug := strings.Trim(builder.String(), "-_") + if len(slug) > maxProfileSlugBytes { + slug = strings.Trim(slug[:maxProfileSlugBytes], "-_") + } + if slug == "" { + return "profile" + } + return slug +} + +func isSlugRune(r rune) bool { + return r >= 'a' && r <= 'z' || + r >= 'A' && r <= 'Z' || + r >= '0' && r <= '9' || + r == '-' || r == '_' +} + +// OrdinalWidth returns the zero-padding width for a comparison of count +// profiles. +func OrdinalWidth(count int) int { + width := 2 + for value := count; value >= 100; value /= 10 { + width++ + } + return width +} + +// ReportFilename derives a report's deterministic bundle filename. +func ReportFilename(position, profileCount int, profileID string) (string, error) { + if profileCount < 1 { + return "", fmt.Errorf("profile count must be positive") + } + if position < 1 || position > profileCount { + return "", fmt.Errorf("profile position %d is outside 1..%d", position, profileCount) + } + return fmt.Sprintf("%0*d-%s.md", OrdinalWidth(profileCount), position, ProfileSlug(profileID)), nil +} + +// BuildComparisonID derives the stable comparison identity for a resolved +// report run. +func BuildComparisonID(reportRunID string) (string, error) { + if strings.TrimSpace(reportRunID) == "" { + return "", fmt.Errorf("report run ID must not be blank") + } + return "comparison_" + reportRunID, nil +} + +// DefaultDirectoryName derives the bundle directory name for a resolved report +// output filename. +func DefaultDirectoryName(reportOutputName string) (string, error) { + if !strings.HasSuffix(reportOutputName, ".md") { + return "", fmt.Errorf("report output name %q must end in .md", reportOutputName) + } + if !isArtifactBasename(reportOutputName) { + return "", fmt.Errorf("report output name %q must be a basename", reportOutputName) + } + stem := strings.TrimSuffix(reportOutputName, ".md") + if stem == "" { + return "", fmt.Errorf("report output name %q has an empty stem", reportOutputName) + } + return "comparison-" + stem, nil +} + +// SHA256 returns a lowercase hexadecimal SHA-256 digest. +func SHA256(content []byte) string { + digest := sha256.Sum256(content) + return hex.EncodeToString(digest[:]) +} + +// NewSafeError returns bounded, valid UTF-8 error information suitable for a +// comparison manifest. +func NewSafeError(category, message string) SafeError { + return SafeError{Category: category, Message: TruncateErrorMessage(message)} +} + +// TruncateErrorMessage returns a valid UTF-8 message of at most 1,024 bytes. +func TruncateErrorMessage(message string) string { + message = strings.ToValidUTF8(message, "\uFFFD") + if len(message) <= maxErrorMessageBytes { + return message + } + + end := maxErrorMessageBytes + for end > 0 && !utf8.RuneStart(message[end]) { + end-- + } + return message[:end] +} + +// Validate checks every invariant required for a current comparison manifest. +func (manifest Manifest) Validate() error { + if manifest.SchemaVersion != SchemaVersion { + return fmt.Errorf("unsupported comparison schema version %q", manifest.SchemaVersion) + } + if strings.TrimSpace(manifest.ComparisonID) == "" { + return fmt.Errorf("comparison ID must not be blank") + } + if manifest.StartedAt.IsZero() || manifest.FinishedAt.IsZero() { + return fmt.Errorf("comparison timestamps must be nonzero") + } + if manifest.StartedAt.Location() != time.UTC || manifest.FinishedAt.Location() != time.UTC { + return fmt.Errorf("comparison timestamps must use UTC") + } + if manifest.FinishedAt.Before(manifest.StartedAt) { + return fmt.Errorf("comparison finish time precedes start time") + } + if manifest.ValidPeriod.Start.IsZero() || manifest.ValidPeriod.End.IsZero() || !manifest.ValidPeriod.End.After(manifest.ValidPeriod.Start) { + return fmt.Errorf("valid period must have a nonempty increasing range") + } + if strings.TrimSpace(manifest.ReportID) == "" || strings.TrimSpace(manifest.Timezone) == "" { + return fmt.Errorf("report ID and timezone must not be blank") + } + if strings.TrimSpace(manifest.PromptID) == "" || strings.TrimSpace(manifest.PromptVersion) == "" || !isSHA256(manifest.PromptHash) { + return fmt.Errorf("prompt identity is invalid") + } + if manifest.DataPackage.Path != DataPackageFilename || !isSHA256(manifest.DataPackage.SHA256) { + return fmt.Errorf("data package reference is invalid") + } + if manifest.Total < 2 || manifest.Total != len(manifest.Results) { + return fmt.Errorf("comparison result count is invalid") + } + if manifest.Succeeded < 0 || manifest.Failed < 0 || manifest.Succeeded+manifest.Failed != manifest.Total { + return fmt.Errorf("comparison result totals are inconsistent") + } + + profiles := make(map[string]struct{}, len(manifest.Results)) + reportPaths := make(map[string]struct{}, len(manifest.Results)) + succeeded, failed := 0, 0 + for index, result := range manifest.Results { + if result.Position != index+1 { + return fmt.Errorf("result position %d is not ordered", result.Position) + } + if strings.TrimSpace(result.ProfileID) == "" { + return fmt.Errorf("result %d has a blank profile ID", result.Position) + } + if _, duplicate := profiles[result.ProfileID]; duplicate { + return fmt.Errorf("result %d duplicates profile ID %q", result.Position, result.ProfileID) + } + profiles[result.ProfileID] = struct{}{} + if strings.TrimSpace(result.ModelName) == "" { + return fmt.Errorf("result %d has a blank model name", result.Position) + } + + switch result.Status { + case StatusSucceeded: + succeeded++ + if result.ValidationStatus != "passed" { + return fmt.Errorf("successful result %d did not pass validation", result.Position) + } + if result.Error != nil || !isReportPath(result.ReportPath) { + return fmt.Errorf("successful result %d has invalid report details", result.Position) + } + if _, duplicate := reportPaths[result.ReportPath]; duplicate { + return fmt.Errorf("result %d duplicates report path %q", result.Position, result.ReportPath) + } + reportPaths[result.ReportPath] = struct{}{} + case StatusFailed: + failed++ + if result.ReportPath != "" || result.Error == nil || !isValidationStatus(result.ValidationStatus) { + return fmt.Errorf("failed result %d has invalid failure details", result.Position) + } + if err := result.Error.validate(); err != nil { + return fmt.Errorf("failed result %d: %w", result.Position, err) + } + default: + return fmt.Errorf("result %d has unsupported status %q", result.Position, result.Status) + } + } + if succeeded != manifest.Succeeded || failed != manifest.Failed { + return fmt.Errorf("comparison status counts do not match results") + } + + return nil +} + +func (safeError SafeError) validate() error { + if strings.TrimSpace(safeError.Category) == "" || strings.TrimSpace(safeError.Message) == "" { + return fmt.Errorf("safe error category and message must not be blank") + } + if !utf8.ValidString(safeError.Message) || len(safeError.Message) > maxErrorMessageBytes { + return fmt.Errorf("safe error message is not bounded UTF-8") + } + return nil +} + +func isValidationStatus(status string) bool { + return status == "" || status == "failed" || status == "skipped" +} + +func isSHA256(value string) bool { + if len(value) != sha256.Size*2 { + return false + } + for _, r := range value { + if (r < '0' || r > '9') && (r < 'a' || r > 'f') { + return false + } + } + return true +} + +func isReportPath(value string) bool { + return strings.HasSuffix(value, ".md") && isArtifactBasename(value) && value != ManifestFilename && value != DataPackageFilename +} + +func isArtifactBasename(value string) bool { + return value != "" && value != "." && value != ".." && + !strings.ContainsAny(value, "/\\") && path.Base(value) == value +} + +// Validate checks that the manifest and in-memory bundle payloads agree. +func (bundle LogicalBundle) Validate() error { + if err := bundle.Manifest.Validate(); err != nil { + return err + } + if SHA256(bundle.DataPackage) != bundle.Manifest.DataPackage.SHA256 { + return fmt.Errorf("data package digest does not match manifest") + } + + if len(bundle.Reports) != bundle.Manifest.Succeeded { + return fmt.Errorf("bundle report count does not match manifest") + } + reportIndex := 0 + for _, result := range bundle.Manifest.Results { + if result.Status != StatusSucceeded { + continue + } + report := bundle.Reports[reportIndex] + if report.Position != result.Position || report.Path != result.ReportPath || !isReportPath(report.Path) { + return fmt.Errorf("bundle report for result %d does not match manifest", result.Position) + } + reportIndex++ + } + + return nil +} + +// EncodeManifest validates and deterministically encodes a manifest as the +// canonical two-space-indented JSON document with one trailing newline. +func EncodeManifest(manifest Manifest) ([]byte, error) { + if err := manifest.Validate(); err != nil { + return nil, err + } + encoded, err := json.MarshalIndent(manifest, "", " ") + if err != nil { + return nil, fmt.Errorf("encode comparison manifest: %w", err) + } + return append(encoded, '\n'), nil +} diff --git a/internal/comparison/comparison_test.go b/internal/comparison/comparison_test.go new file mode 100644 index 0000000..5671780 --- /dev/null +++ b/internal/comparison/comparison_test.go @@ -0,0 +1,314 @@ +package comparison + +import ( + "encoding/json" + "strings" + "testing" + "time" + "unicode/utf8" +) + +func TestValidateProfileIDs(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + profileIDs []string + wantErr bool + }{ + {name: "accepts exact case distinct IDs", profileIDs: []string{"weather-light", "Weather-Light"}}, + {name: "preserves surrounding whitespace", profileIDs: []string{" weather-light", "weather-deep "}}, + {name: "rejects one ID", profileIDs: []string{"weather-light"}, wantErr: true}, + {name: "rejects blank ID", profileIDs: []string{"weather-light", " \t\n"}, wantErr: true}, + {name: "rejects exact duplicate", profileIDs: []string{"weather-light", "weather-light"}, wantErr: true}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + err := ValidateProfileIDs(test.profileIDs) + if (err != nil) != test.wantErr { + t.Fatalf("ValidateProfileIDs(%q) error = %v, want error %t", test.profileIDs, err, test.wantErr) + } + }) + } +} + +func TestProfileSlug(t *testing.T) { + t.Parallel() + + long := strings.Repeat("a", 62) + "-_more" + tests := []struct { + profileID string + want string + }{ + {profileID: "weather-light", want: "weather-light"}, + {profileID: "model.v1", want: "model-v1"}, + {profileID: "nested/path\\name", want: "nested-path-name"}, + {profileID: " weather\tlight\n", want: "weather-light"}, + {profileID: "\x00\x01", want: "profile"}, + {profileID: "météo-東京", want: "m-t-o"}, + {profileID: "___", want: "profile"}, + {profileID: long, want: strings.Repeat("a", 62)}, + } + + for _, test := range tests { + t.Run(test.profileID, func(t *testing.T) { + t.Parallel() + if got := ProfileSlug(test.profileID); got != test.want { + t.Fatalf("ProfileSlug(%q) = %q, want %q", test.profileID, got, test.want) + } + }) + } +} + +func TestReportNaming(t *testing.T) { + t.Parallel() + + if got := OrdinalWidth(99); got != 2 { + t.Fatalf("OrdinalWidth(99) = %d, want 2", got) + } + if got := OrdinalWidth(100); got != 3 { + t.Fatalf("OrdinalWidth(100) = %d, want 3", got) + } + if got, err := ReportFilename(1, 3, "weather.light"); err != nil || got != "01-weather-light.md" { + t.Fatalf("ReportFilename() = %q, %v, want %q, nil", got, err, "01-weather-light.md") + } + if got, err := ReportFilename(100, 100, "weather-light"); err != nil || got != "100-weather-light.md" { + t.Fatalf("ReportFilename() = %q, %v, want %q, nil", got, err, "100-weather-light.md") + } + first, err := ReportFilename(1, 2, "model.v1") + if err != nil { + t.Fatalf("first ReportFilename() error = %v", err) + } + second, err := ReportFilename(2, 2, "model/v1") + if err != nil { + t.Fatalf("second ReportFilename() error = %v", err) + } + if first == second { + t.Fatalf("normalized profile collisions produced the same filename %q", first) + } + if _, err := ReportFilename(0, 2, "weather-light"); err == nil { + t.Fatal("ReportFilename accepted position zero") + } + if _, err := ReportFilename(1, 0, "weather-light"); err == nil { + t.Fatal("ReportFilename accepted zero profile count") + } + if got, err := BuildComparisonID("daily-2026-08-24"); err != nil || got != "comparison_daily-2026-08-24" { + t.Fatalf("BuildComparisonID() = %q, %v", got, err) + } + if _, err := BuildComparisonID(" \t"); err == nil { + t.Fatal("BuildComparisonID accepted blank run ID") + } + if got, err := DefaultDirectoryName("daily-2026-08-24.md"); err != nil || got != "comparison-daily-2026-08-24" { + t.Fatalf("DefaultDirectoryName() = %q, %v", got, err) + } + for _, name := range []string{"daily.txt", "nested/daily.md", ".md"} { + if _, err := DefaultDirectoryName(name); err == nil { + t.Fatalf("DefaultDirectoryName(%q) accepted invalid name", name) + } + } +} + +func TestManifestEncodingAndRoundTrip(t *testing.T) { + t.Parallel() + + manifest := validManifest() + encoded, err := EncodeManifest(manifest) + if err != nil { + t.Fatalf("EncodeManifest() error = %v", err) + } + want := "{\n" + + " \"schemaVersion\": \"weatherreporter.comparison.v1\",\n" + + " \"comparisonId\": \"comparison_daily-2026-08-24\",\n" + + " \"startedAt\": \"2026-08-24T12:00:00Z\",\n" + + " \"finishedAt\": \"2026-08-24T12:01:00Z\",\n" + + " \"reportId\": \"daily\",\n" + + " \"validPeriod\": {\n" + + " \"start\": \"2026-08-24T00:00:00-04:00\",\n" + + " \"end\": \"2026-08-25T00:00:00-04:00\"\n" + + " },\n" + + " \"timezone\": \"America/New_York\",\n" + + " \"promptId\": \"daily-report\",\n" + + " \"promptVersion\": \"2026-08-01\",\n" + + " \"promptHash\": \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\",\n" + + " \"dataPackage\": {\n" + + " \"path\": \"data-package.yml\",\n" + + " \"sha256\": \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\"\n" + + " },\n" + + " \"total\": 2,\n" + + " \"succeeded\": 1,\n" + + " \"failed\": 1,\n" + + " \"results\": [\n" + + " {\n" + + " \"position\": 1,\n" + + " \"profileId\": \"weather-light\",\n" + + " \"backendId\": \"openai\",\n" + + " \"modelName\": \"gpt-5-mini\",\n" + + " \"status\": \"succeeded\",\n" + + " \"validationStatus\": \"passed\",\n" + + " \"reportPath\": \"01-weather-light.md\"\n" + + " },\n" + + " {\n" + + " \"position\": 2,\n" + + " \"profileId\": \"weather-deep\",\n" + + " \"modelName\": \"gpt-5\",\n" + + " \"status\": \"failed\",\n" + + " \"error\": {\n" + + " \"category\": \"application\",\n" + + " \"message\": \"generated text was rejected\"\n" + + " }\n" + + " }\n" + + " ]\n" + + "}\n" + if string(encoded) != want { + t.Fatalf("EncodeManifest() =\n%s\nwant\n%s", encoded, want) + } + + var decoded Manifest + if err := json.Unmarshal(encoded, &decoded); err != nil { + t.Fatalf("json.Unmarshal() error = %v", err) + } + if err := decoded.Validate(); err != nil { + t.Fatalf("decoded manifest validation error = %v", err) + } +} + +func TestManifestValidateRejectsInvariants(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + mutate func(*Manifest) + }{ + {name: "schema version", mutate: func(manifest *Manifest) { manifest.SchemaVersion = "v0" }}, + {name: "non UTC timestamp", mutate: func(manifest *Manifest) { manifest.StartedAt = manifest.StartedAt.In(time.FixedZone("UTC", 0)) }}, + {name: "reversed timestamps", mutate: func(manifest *Manifest) { manifest.FinishedAt = manifest.StartedAt.Add(-time.Second) }}, + {name: "empty valid period", mutate: func(manifest *Manifest) { manifest.ValidPeriod.End = manifest.ValidPeriod.Start }}, + {name: "bad prompt hash", mutate: func(manifest *Manifest) { manifest.PromptHash = "ABC" }}, + {name: "bad data package path", mutate: func(manifest *Manifest) { manifest.DataPackage.Path = "nested/data-package.yml" }}, + {name: "inconsistent totals", mutate: func(manifest *Manifest) { manifest.Succeeded = 2 }}, + {name: "unordered position", mutate: func(manifest *Manifest) { manifest.Results[1].Position = 3 }}, + {name: "duplicate profile", mutate: func(manifest *Manifest) { manifest.Results[1].ProfileID = manifest.Results[0].ProfileID }}, + {name: "unsupported status", mutate: func(manifest *Manifest) { manifest.Results[1].Status = "skipped" }}, + {name: "successful result without report", mutate: func(manifest *Manifest) { manifest.Results[0].ReportPath = "" }}, + {name: "successful result with error", mutate: func(manifest *Manifest) { + manifest.Results[0].Error = &SafeError{Category: "application", Message: "bad"} + }}, + {name: "successful result without passed validation", mutate: func(manifest *Manifest) { manifest.Results[0].ValidationStatus = "failed" }}, + {name: "failed result with report", mutate: func(manifest *Manifest) { manifest.Results[1].ReportPath = "02-weather-deep.md" }}, + {name: "failed result without error", mutate: func(manifest *Manifest) { manifest.Results[1].Error = nil }}, + {name: "traversal report path", mutate: func(manifest *Manifest) { manifest.Results[0].ReportPath = "../report.md" }}, + {name: "duplicate report path", mutate: func(manifest *Manifest) { + manifest.Results[1] = Result{Position: 2, ProfileID: "weather-deep", ModelName: "gpt-5", Status: StatusSucceeded, ValidationStatus: "passed", ReportPath: manifest.Results[0].ReportPath} + manifest.Succeeded = 2 + manifest.Failed = 0 + }}, + {name: "oversized error", mutate: func(manifest *Manifest) { manifest.Results[1].Error.Message = strings.Repeat("x", 1025) }}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + manifest := validManifest() + test.mutate(&manifest) + if err := manifest.Validate(); err == nil { + t.Fatal("Manifest.Validate() succeeded for invalid manifest") + } + }) + } +} + +func TestLogicalBundleValidate(t *testing.T) { + t.Parallel() + + dataPackage := []byte("report: daily\n") + manifest := validManifest() + manifest.DataPackage.SHA256 = SHA256(dataPackage) + bundle := LogicalBundle{ + Manifest: manifest, + DataPackage: dataPackage, + Reports: []BundleReport{{ + Position: 1, + Path: "01-weather-light.md", + Markdown: []byte("# Daily\n"), + }}, + } + if err := bundle.Validate(); err != nil { + t.Fatalf("LogicalBundle.Validate() error = %v", err) + } + + bundle.Reports[0].Path = "other.md" + if err := bundle.Validate(); err == nil { + t.Fatal("LogicalBundle.Validate() accepted mismatched report path") + } + bundle.Reports[0].Path = "01-weather-light.md" + bundle.Reports[0].Position = 2 + if err := bundle.Validate(); err == nil { + t.Fatal("LogicalBundle.Validate() accepted unordered report position") + } +} + +func TestSHA256AndTruncateErrorMessage(t *testing.T) { + t.Parallel() + + if got, want := SHA256([]byte("weather")), "e5e72beb4e3c6926d3dc9e3e2ef7833ba50cd919c2460a782b244fd071e920de"; got != want { + t.Fatalf("SHA256() = %q, want %q", got, want) + } + message := strings.Repeat("€", 400) + got := TruncateErrorMessage(message) + if len(got) > 1024 || !utf8.ValidString(got) { + t.Fatalf("TruncateErrorMessage() returned %d bytes of valid UTF-8 = %t", len(got), utf8.ValidString(got)) + } + if want := strings.Repeat("€", 341); got != want { + t.Fatalf("TruncateErrorMessage() = %q, want %q", got, want) + } + invalid := string([]byte{'x', 0xff, 'y'}) + if got := TruncateErrorMessage(invalid); !utf8.ValidString(got) { + t.Fatal("TruncateErrorMessage() retained invalid UTF-8") + } +} + +func validManifest() Manifest { + newYork := time.FixedZone("-0400", -4*60*60) + return Manifest{ + SchemaVersion: SchemaVersion, + ComparisonID: "comparison_daily-2026-08-24", + StartedAt: time.Date(2026, time.August, 24, 12, 0, 0, 0, time.UTC), + FinishedAt: time.Date(2026, time.August, 24, 12, 1, 0, 0, time.UTC), + ReportID: "daily", + ValidPeriod: ValidPeriod{ + Start: time.Date(2026, time.August, 24, 0, 0, 0, 0, newYork), + End: time.Date(2026, time.August, 25, 0, 0, 0, 0, newYork), + }, + Timezone: "America/New_York", + PromptID: "daily-report", + PromptVersion: "2026-08-01", + PromptHash: strings.Repeat("a", 64), + DataPackage: DataPackageReference{ + Path: DataPackageFilename, + SHA256: strings.Repeat("a", 64), + }, + Total: 2, + Succeeded: 1, + Failed: 1, + Results: []Result{ + { + Position: 1, + ProfileID: "weather-light", + BackendID: "openai", + ModelName: "gpt-5-mini", + Status: StatusSucceeded, + ValidationStatus: "passed", + ReportPath: "01-weather-light.md", + }, + { + Position: 2, + ProfileID: "weather-deep", + ModelName: "gpt-5", + Status: StatusFailed, + Error: &SafeError{Category: "application", Message: "generated text was rejected"}, + }, + }, + } +}