diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md deleted file mode 100644 index fb1ae7c..0000000 --- a/docs/roadmap/implementation.md +++ /dev/null @@ -1,194 +0,0 @@ -# Workspace Layout Implementation Plan - -## Purpose - -This document is the staged implementation plan for -[workspace.md](workspace.md). It is written for an LLM coding agent that will -implement the workspace artifact layout refactor in order. - -The feature is complete when every managed local run artifact uses the -kind-first, date-scoped layout defined in `docs/roadmap/workspace.md`, metadata -and inspection link those paths correctly, and distributor upload behavior is -unchanged. - -## Ground Rules - -- Review `docs/policy/architecture.md`, `docs/policy/development.md`, and - `docs/policy/documentation.md` before editing code. -- Keep path construction owned by `internal/state`. -- Do not add user-visible workspace path template configuration. -- Do not use distributor bundle paths for managed workspace paths. -- Do not implement cleanup, migration tooling, or a local publication mirror in - this work. -- Keep optional `--out` and `--out-dir` copies separate from managed workspace - artifacts. - -## Target Path Contract - -State-managed run artifacts must use these relative path shapes under -`workspace.root`: - -```text -reports///report..md - -snapshots///modules..json -snapshots///metadata..json -snapshots///generated_text_raw..json -snapshots///generated_text_result..json -snapshots///generated_text..json -snapshots///render_context..json - -data-packages///data_package..yaml -preflight///render..json - -notifications///distributor..json -notifications/batches///distributor..json -``` - -`valid_start_date` and `batch_start_date` are local dates in the effective -report timezone already carried by the resolved report or batch reference. - -## Stage 1: State Path Construction - -Goal: switch the canonical managed path builder to the target layout. - -Implementation: - -- Update `internal/state.FilesystemStore.Paths` to build all `ArtifactPaths` - using the target path contract above. -- Keep existing validation for workspace subdirectories, RunID, and report - artifact group. -- Keep `PrepareRenderedReport` responsible for creating the parent report - directory before Scriptorium or template rendering writes Markdown. -- Update `BatchDistributorNotificationPath` to use - `distributor..json`. - -Tests: - -- Update state path tests to assert the exact new relative paths for at least - `daily`, `today`, `tomorrow`, and `hourly`. -- Update batch notification path tests for - `notifications/batches///distributor..json`. -- Run: - -```bash -go test ./internal/state -``` - -Completion criteria: - -- `FilesystemStore.Paths` returns only target-layout managed paths. -- Existing save methods write to the new paths through the existing - `ArtifactPaths` fields. - -## Stage 2: Metadata Discovery And Prior Lookup - -Goal: make inspection and prior lookup discover the new metadata filenames. - -Implementation: - -- Replace metadata filename checks that currently rely on the old - `.metadata.json` suffix with a helper that matches only - `metadata..json`. -- Use that helper in `ListReports` and `FindPriorSnapshot`. -- Do not add legacy metadata filename fallback in this refactor; migration is - explicitly deferred by the feature roadmap. -- Keep `LoadMetadataByRunID` behavior based on `ListReports`. - -Tests: - -- Update report listing tests to create/read `metadata..json`. -- Update prior lookup tests to confirm comparable metadata is found under the - new filename. -- Add or adjust one negative test so non-metadata JSON in the snapshot tree is - ignored. -- Run: - -```bash -go test ./internal/state -``` - -Completion criteria: - -- Inspection and prior lookup work with new metadata names. -- Generated-text, module, and render-context JSON files are not mistaken for - metadata. - -## Stage 3: App Workflow Expectations - -Goal: update app-level tests and any path assumptions outside `internal/state`. - -Implementation: - -- Do not duplicate path-building logic in `internal/app`; continue asking the - state store for managed paths. -- Update app tests that assert managed report paths, data package paths, - generated-text paths, render context paths, notification source paths, batch - included report source paths, and result JSON paths. -- Preserve the invariant that distributor uploads use the managed Markdown - report path as the upload source and map it to separate distributor bundle - paths. -- Preserve `--out` and `--out-dir` behavior as extra copies outside the managed - report path. - -Tests: - -- Run: - -```bash -go test ./internal/app ./internal/cli -``` - -Completion criteria: - -- App workflows still generate, save metadata, notify distributor, and report - paths using state-owned managed paths. -- No app code constructs the new managed path layout manually. - -## Stage 4: Documentation - -Goal: move the implemented layout into maintained docs after code behavior has -changed. - -Documentation changes: - -- Update `docs/internal/state.md` with the new target managed layout and - filename convention. -- Update `docs/operations.md` filesystem layout examples and artifact wording. -- Update `docs/config.md` only if workspace field descriptions need - clarification. -- Keep cleanup, migration, and local publication mirror details in roadmap - docs until those features are implemented. - -Validation: - -```bash -go test ./internal/state ./internal/app ./internal/config -go test ./... -go run ./cmd/weatherreporter --help -git diff --check -``` - -Completion criteria: - -- Implemented docs match the new behavior. -- Non-roadmap docs do not describe deferred cleanup or local publication mirror - behavior as implemented. - -## Final Verification - -Before considering the feature complete, run: - -```bash -go test ./... -go run ./cmd/weatherreporter --help -git diff --check -rg "\\.metadata\\.json|\\.modules\\.json|\\.data_package\\.yaml|\\.render\\.json|\\.distributor\\.json|generated_text\\.raw|generated_text\\.run" internal docs examples --glob '!docs/roadmap/**' -``` - -The final `rg` should find no old managed filename patterns in implemented -code, maintained docs, or examples. - -## Open Questions - -None. The feature roadmap decisions are sufficient for implementation. diff --git a/docs/roadmap/workspace.md b/docs/roadmap/workspace.md deleted file mode 100644 index a2a25b0..0000000 --- a/docs/roadmap/workspace.md +++ /dev/null @@ -1,119 +0,0 @@ -# Workspace Artifact Layout Roadmap - -## Purpose - -This roadmap defines the target local workspace layout for artifacts generated -by weatherreporter. - -Weatherreporter currently writes managed local artifacts for each run and also -maps the managed Markdown report into distributor bundle paths. Distributor -bundle paths are publication paths and may include overwrite-oriented aliases -such as `hourly/index.md`, `today/index.md`, and -`daily//index.md`. Local workspace paths have a different -job: they preserve run-specific source artifacts for inspection, troubleshooting, -retry diagnosis, and future cleanup. - -## Locked Decisions - -- Keep managed local workspace paths separate from distributor bundle paths. -- Do not use distributor-style `index.md` aliases as the canonical managed - local artifact paths. -- Store managed local artifacts as immutable run artifacts. -- Group every durable run artifact by artifact family, report artifact group, - and local `valid_start_date`. -- Use `valid_start_date` in the effective report timezone as the directory key. -- Include the RunID in every durable run artifact filename. -- Prefer artifact-kind-first filenames: `..`. -- Keep optional `--out` and `--out-dir` copies separate from managed workspace - artifacts. -- Future cleanup should delete old managed artifact date directories by - `valid_start_date`; cleanup should be narrow and opt-in. -- If a local publication preview is added later, put it in a separate mirror - tree and treat it as rebuildable output, not canonical run state. - -## Target Managed Layout - -The target managed workspace layout is: - -```text -/ - reports///report..md - - snapshots///modules..json - snapshots///metadata..json - snapshots///generated_text_raw..json - snapshots///generated_text_result..json - snapshots///generated_text..json - snapshots///render_context..json - - data-packages///data_package..yaml - preflight///render..json - - notifications///distributor..json - notifications/batches///distributor..json -``` - -`artifact_group` comes from the report definition. `valid_start_date` is the -resolved report valid period start date in the effective report timezone. -`batch_start_date` is the batch start date in the effective report timezone. - -Generated-text-template reports use the generated-text artifacts. Markdown-path -reports omit generated-text artifacts they do not produce. - -## Cleanup Compatibility - -The target layout is designed to support a future cleanup command that removes -managed local artifacts where `valid_start_date` is more than two days before -the current local date. - -Cleanup can operate by deleting matching date directories across managed -artifact families: - -```text -reports/// -snapshots/// -data-packages/// -preflight/// -notifications/// -``` - -The command should not infer dates from RunIDs when the directory date is -available. It should use the effective configured report timezone for "current -date" and for interpreting `valid_start_date` directories. - -Batch notification cleanup should use `batch_start_date` under -`notifications/batches//`. A future local publication mirror, if added, -should be ignored by cleanup or rebuilt after cleanup; it should not be treated -as run history. - -## Intended Final State - -`internal/state` owns the managed workspace layout and path construction. The -app layer continues to ask the state store for managed paths and records those -paths in metadata and command results. - -Metadata links every artifact produced for a run using the new managed paths. -Inspection and prior-snapshot lookup continue to read metadata rather than -rendered Markdown text. Distributor uploads continue to use the managed -Markdown report as the source file and map it to separately resolved -distributor bundle paths. - -Existing workspace configuration fields remain meaningful: - -- `workspace.root` -- `workspace.snapshots_dir` -- `workspace.reports_dir` -- `workspace.data_packages_dir` -- `workspace.preflight_dir` -- `workspace.notifications_dir` - -This roadmap does not require a new user-visible workspace path template config -surface. - -## Deferred Work - -- A cleanup command for managed local artifacts. -- Cleanup dry-run output, retention configuration, and deletion confirmation - behavior. -- A rebuildable local publication mirror that uses distributor-style paths. -- Migration tooling for existing pre-refactor workspace directories. diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 6781ec1..02c1b11 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -3346,15 +3346,22 @@ func plannedBatchNotificationReports(t *testing.T, cfg config.Config, batch Batc if err != nil { t.Fatalf("planBatchRun() error = %v", err) } - reportDir := filepath.Join(t.TempDir(), "managed-reports") + store, err := state.NewFilesystemStore(cfg.Workspace) + if err != nil { + t.Fatalf("NewFilesystemStore() error = %v", err) + } results := make([]BatchReportResult, 0, len(planned)) for _, item := range planned { metadata := item.Resolved.Metadata() + paths, err := store.Paths(item.Resolved) + if err != nil { + t.Fatalf("Paths(%s) error = %v", item.Resolved.Definition.ID, err) + } results = append(results, BatchReportResult{ ReportID: item.Resolved.Definition.ID, RunID: metadata.RunID, Status: "succeeded", - ReportPath: filepath.Join(reportDir, string(item.Resolved.Definition.ID), metadata.RunID+".md"), + ReportPath: paths.RenderedReport, }) } return planned, results diff --git a/internal/state/filesystem.go b/internal/state/filesystem.go index 0f6fb0b..a8f60f3 100644 --- a/internal/state/filesystem.go +++ b/internal/state/filesystem.go @@ -83,8 +83,8 @@ func (s *FilesystemStore) Paths(resolved report.Resolved) (ArtifactPaths, error) return ArtifactPaths{}, fmt.Errorf("state store is required") } metadata := resolved.Metadata() - if metadata.RunID == "" { - return ArtifactPaths{}, fmt.Errorf("run id is required") + if err := validatePathSegment("run id", metadata.RunID); err != nil { + return ArtifactPaths{}, err } group := resolved.Definition.ArtifactGroup if group == "" { diff --git a/internal/state/filesystem_test.go b/internal/state/filesystem_test.go index fe6be7e..733e984 100644 --- a/internal/state/filesystem_test.go +++ b/internal/state/filesystem_test.go @@ -69,6 +69,20 @@ func TestDailyPathsUseRunIDValidDateDisambiguator(t *testing.T) { } } +func TestPathsRejectRunIDPathSeparators(t *testing.T) { + store := newTestStore(t) + resolved := resolveDailyAt(t, "2026-05-29T05:00:00-05:00") + resolved.Definition.ID = report.ID("daily/bad") + + _, err := store.Paths(resolved) + if err == nil { + t.Fatal("Paths() error = nil, want invalid run id error") + } + if !strings.Contains(err.Error(), "run id must not contain path separators") { + t.Fatalf("error = %q, want run id path separator context", err.Error()) + } +} + func TestBatchDistributorNotificationPathUsesWorkspaceBatchDateAndRunID(t *testing.T) { store := newTestStore(t) location := mustLoadStateLocation(t, "America/Chicago")