# LLM Profile Comparison Implementation Plan Status: Complete. ## Purpose And Authority This document is the ordered implementation plan for the accepted [LLM Profile Comparison Roadmap](profile-comparison.md). The roadmap owns the feature purpose, policy, scope, and desired end state. This plan records the completed implementation and records the corrective work completed during post-implementation review. All implementation work in this plan is complete. The recorded work leaves the repository compiling, tested, documented to its implemented boundary, and internally coherent. ## Implementation Rules The following rules governed every implementation stage: - Read `docs/development.md`, the task-specific documents it identifies, all files under `docs/policy/`, and the feature roadmap before changing code. - Preserve the existing `generate`, `run`, and `compare` command contracts except for the explicit comparison corrections defined below. - Keep Promptkit types and calls behind `internal/adapters/promptkit` and the dependency-neutral `internal/promptexec` interface. - 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. - Preserve comparison's prepare-once, execute-concurrently, order-results-by- selection, publish-on-profile-failure, and never-notify invariants. - Do not add a Weatherreporter concurrency limit. Promptkit owns backend capacity. - Never expose provider bodies, prompts, schemas, model output, endpoints, credentials, or arbitrary wrapped error text in normal JSON summaries or manifests. - Use deterministic, offline, credential-free tests. Test filesystem safety, concurrency, and recovery through the narrowest stable behavioral boundary; do not rely on timing-only sleeps or host permission behavior. - Run `gofmt` on changed Go files and `git diff --check` in every stage. Run focused tests while developing and `GOWORK=off go test -count=1 ./...` before completing each stage. Stages involving concurrency or filesystem mutation must also run affected packages with `-race`. - Do not commit, tag, push, or prepare a release unless the implementing prompt separately requests it. ## Completed Stages ### Stage 1: Comparison Artifact And Naming Contracts Added the dependency-neutral comparison model, schema version, manifest validation and encoding, safe errors, deterministic profile filenames, comparison identities, default directory names, and content hashing. ### Stage 2: Destination Recognition And Transactional Publication Added read-only destination planning, strict recognition of current comparison bundles, private sibling staging, guarded replacement, rollback, and atomic directory publication. ### Stage 3: Ordered Multi-Profile Preflight Added exact prompt inspection followed by sequential profile inspection before weather collection, including effective backend, model, and credential checks. ### Stage 4: Immutable Shared Report Preparation Extracted one immutable prepared-report value so comparison collection, derivation, module construction, and data-package serialization happen once. ### Stage 5: Profile Execution And In-Memory Rendering Separated profile-specific Promptkit execution, generated-text validation, and Markdown rendering from output publication while preserving ordinary report generation behavior. ### Stage 6: Concurrent Ordered Profile Execution Added one goroutine per selected profile using one shared executor and one prepared input, deterministic debug identities, isolated profile failures, joined cancellation, and selection-ordered results. ### Stage 7: Application-Level Comparison Added `app.CompareDetailed`, coherent complete and partial bundle construction, aggregate profile-failure behavior, absolute published paths, and the application-level guarantee that comparison never notifies Distributor. ### Stage 8: Compare Command Parsing Added the `compare` command request path, repeatable ordered `--profile`, exact `--out-dir`, guarded `--replace`, applicable common flags, validation, and one executor construction per invocation. ### Stage 9: CLI Results And Exit Behavior Added structured success and failure summaries, quiet-mode suppression, ordered per-profile results, safe bounded errors, and nonzero exit behavior for partial or command-level failure. ### Stage 10: Canonical Documentation And Initial Validation Documented the implemented CLI, operations, Promptkit integration, comparison bundle, application orchestration, execution, publication, architecture, and development contracts, then passed the original repository-wide validation gate. ## Stage 11: Make Concurrent Prompt Debug Creation Race-Safe ### Goal Ensure concurrent comparison profiles can create their distinct debug runs under one new report/date directory without spuriously failing or leaving a test goroutine blocked. ### Work 1. Update `internal/promptdebug.ensureSecureDirectory` so concurrent creation of the same missing directory is idempotent. If `os.Mkdir` reports that the path already exists, inspect the path with `Lstat` and accept it only when it is the expected real directory. Continue to reject symlinks, non-directories, unsafe modes, and every unrelated filesystem error. 2. Preserve the existing absolute-path, containment, `0700` directory, `0600` file, and no-symlink guarantees. Do not weaken debug-root validation or make all `EEXIST` errors successful. 3. Add a focused prompt-debug concurrency regression that starts multiple writers beneath a shared missing ancestor, joins every goroutine, and verifies every expected artifact and permission invariant. 4. Make comparison execution test barriers time-bounded and failure-aware. A callback failure before executor entry must fail the test promptly rather than leave `waitForProfileStarts` waiting forever. 5. Retain distinct deterministic debug references and profile-local debug failure behavior. ### Tests And Exit Criteria - The focused prompt-debug concurrency test passes repeatedly and with the race detector. - `TestExecuteComparisonProfilesUsesDistinctDeterministicDebugReferences` cannot hang when a profile fails before reaching the fake executor. - Run, at minimum: ```sh GOWORK=off go test -count=100 ./internal/promptdebug GOWORK=off go test -count=100 -run TestExecuteComparisonProfilesUsesDistinctDeterministicDebugReferences ./internal/app GOWORK=off go test -race -count=1 ./internal/promptdebug ./internal/app GOWORK=off go test -count=1 ./... ``` ## Stage 12: Make Replacement Authorization Commit-Safe ### Goal Prevent a destination changed after the final read-only preflight from being treated as the previously authorized empty directory or recognized bundle and then deleted during replacement. ### Work 1. Replace `DestinationPlan.Exists` as the publication decision with an explicit destination-state classification: absent, empty real directory, or recognized current-schema bundle. Keep `Replace` in the plan so publication can apply the same authorization policy at commit time. 2. Continue to call `PlanDestination` immediately before publication. For an absent target, install staging with one rename; a concurrently created target must cause that rename to fail without modifying the new target. 3. For an existing target, rename that exact filesystem entry to the unique sibling backup before deleting or installing anything. Classify the moved backup while it is at its stable backup path and authorize it under the original replacement policy: - an empty real directory is allowed with or without `Replace`; - a recognized current-schema comparison bundle is allowed only with `Replace`; and - a file, symlink, unrecognized/nonempty directory, unreadable entry, or other classification failure is not allowed. 4. Treat this post-move classification as the destructive-action authorization point. If it fails, restore the moved entry to the target and return an error without installing staging. If the target has concurrently reappeared or restoration otherwise fails, retain the backup and return an actionable joined or typed error that identifies its recovery path; never delete either entry to force restoration. 5. Install staging only after the moved target has passed authorization. Never remove a backup that did not pass post-move authorization. 6. Preserve the existing final cancellation linearization rule: cancellation observed before the rename transaction prevents replacement; after the transaction starts, finish commit or rollback rather than abandoning it. 7. Add a package-private filesystem-operation seam only if needed for deterministic tests. Keep the public destination and publication APIs free of test-only hooks. ### Tests And Exit Criteria - Deterministically replace an initially accepted destination after final preflight but before its move with each consequential unauthorized type: unrelated nonempty directory, regular file, and symlink. Publication must fail, staging must not become the target, and the moved entry must be restored or retained at a reported recovery path. - Cover an initially empty directory whose contents change before its move and a recognized bundle swapped for an unrecognized directory. - Retain coverage for absent targets, empty directories, recognized bundle replacement, cancellation before commit, install failure, successful rollback, failed rollback, and cleanup of ordinary staging failures. - Run `GOWORK=off go test -race -count=1 ./internal/comparison` and the repository-wide standard test command. ## Stage 13: Represent Committed Publication Cleanup Failures Accurately ### Goal Keep application and CLI results truthful when the new comparison bundle has been committed but removal of the old sibling backup fails. ### Work 1. Change comparison publication to return a dependency-neutral result as well as an error: ```go type PublicationResult struct { Committed bool RetainedBackupPath string } func Publish( ctx context.Context, plan DestinationPlan, bundle LogicalBundle, ) (PublicationResult, error) ``` 2. Define `Committed` as meaning the complete staged bundle is now installed at the target. Pre-commit, staging, authorization, install, and successful- rollback failures return `Committed == false`. A successful install returns `Committed == true` even if later backup cleanup fails. 3. Add a typed post-commit cleanup error that unwraps its filesystem cause and records the retained backup path for operator recovery. On this error, return `Committed == true` and the absolute retained backup path. Do not roll back or remove the newly committed valid bundle merely because old backup cleanup failed. 4. In `app.CompareDetailed`, populate `ManifestPath`, `DataPackagePath`, and successful profile `ReportPath` values whenever publication reports `Committed == true`, before returning any cleanup error. 5. Treat post-commit cleanup failure as a command-level operational failure: return the non-nil structured result plus an error, produce status `failed`, and exit nonzero even though the published artifact paths are present. The ordinary safe JSON error must not contain the raw filesystem cause or backup path; the wrapped diagnostic returned on stderr may identify the retained backup for recovery. 6. Keep `RetainedBackupPath` out of the versioned comparison manifest. It describes an incomplete local transaction cleanup, not the logical bundle. ### Tests And Exit Criteria - Inject a deterministic backup-removal failure after successful installation and assert the target is the new recognized bundle, the old bundle remains at the reported backup, `Committed` is true, and the error is inspectable by type. - At the application boundary, assert all committed artifact paths are absolute and populated while the method still returns an error. - At the CLI boundary, assert status `failed`, nonzero return, present artifact paths, and a bounded generic safe error with no raw filesystem detail. - Retain tests showing every pre-commit or rolled-back failure omits published artifact paths. - Run comparison, application, and CLI tests with `-race`, then the repository-wide standard test command. ## Stage 14: Complete Structured Failure Metadata And Classification ### Goal Make every non-nil comparison result a reliable description of the attempted run and preserve useful safe error categories in the top-level CLI summary. ### Work 1. In `app.CompareDetailed`, assign the absolute resolved `OutputDirectory` immediately after output-directory resolution and before destination preflight. Do not wait for `PlanDestination` to succeed. 2. Once the initial `ComparisonResult` exists, guarantee that every return path sets a nonzero UTC `FinishedAt` that is not before `StartedAt`. Use one centralized finalization path or a defer; do not scatter timestamp writes across individual failures. 3. Continue to omit manifest, data-package, and report paths until publication commits. Preserve whatever prompt identity fields have actually been resolved; never invent a hash or profile result for a phase that did not run. 4. Update `safeComparisonSummaryError` to use this stable mapping, always passing messages through `comparison.NewSafeError`: | Error | Category | Safe message | | --- | --- | --- | | aggregate profile failure | `application` | existing bounded aggregate message | | `context.Canceled` | `canceled` | `comparison canceled` | | `context.DeadlineExceeded` | `deadline_exceeded` | `comparison deadline exceeded` | | categorized `promptexec` error | exact `promptexec.CategoryOf` value | `comparison prompt operation failed` | | `comparison.DestinationError` | `destination_` | `comparison destination preflight failed` | | post-commit cleanup error | `publication_cleanup` | `comparison published but cleanup did not complete` | | any unknown error | `application` | `comparison did not complete` | 5. Apply the most specific mapping before a more general wrapped match. In particular, detect the post-commit cleanup and destination types before falling back to a nested filesystem or context cause. 6. Do not copy `DestinationError.Target`, wrapped causes, or arbitrary `error.Error()` text into normal JSON. Detailed returned errors remain available on stderr and through Go error inspection. ### Tests And Exit Criteria - Add application tests for destination-preflight, debug initialization, prompt-preflight, collection, and preparation failures. Whenever a non-nil result is returned, assert an absolute output directory, nonzero ordered UTC timestamps, and omission of unpublished artifact paths. - Add table-driven CLI tests for every mapping row, including wrapped errors, and assert that unsafe sentinel text cannot enter serialized output. - Preserve existing ordered profile-level categories and safe messages. - Run application and CLI tests with `-race`, then the repository-wide standard test command. ## Stage 15: Remove Temporary Seams, Reconcile Documentation, And Validate ### Goal Remove review-discovered maintenance debt, document the corrected implemented behavior in its canonical owners, and complete the release-equivalent gate. ### Work 1. Remove the unused `Runner.resolveComparison` wrapper. Remove the test-oriented `Runner.executeComparison` seam if it has no production caller, and rewrite its remaining coverage through `Runner.Run`, `resolveComparisonAction`, or another stable behavioral boundary. 2. Remove `comparisonProfileOutcome.err` if production code still does not use it. Keep raw failures in returned/wrapped errors or explicit internal error types; do not retain an otherwise dead field solely for private test assertions. 3. Correct the `internal/comparison` package comment so it describes the package's actual ownership of both logical comparison contracts and filesystem destination/publication behavior. 4. Update only the canonical current-state documents affected by Stages 11 through 14: - `docs/internal/comparison-publication.md` owns post-move authorization, commit state, rollback, retained backups, and cleanup mechanics; - `docs/internal/comparison-execution.md` owns concurrency and debug-write behavior; - `docs/internal/app-orchestration.md` owns partial results and committed publication error handling; - `docs/cli.md` owns structured status, safe category, path, and exit behavior; and - `docs/operations.md` owns operator recovery for a retained sibling backup. Link rather than duplicating complete contracts, and update architecture or integration documentation only if its existing invariant is inaccurate. 5. Mark this plan `Complete` and restore the feature roadmap's implemented status after every exit criterion below passes. Retain or remove the two roadmap documents only according to a later maintainer-directed roadmap cleanup; do not archive them as a second current-state reference in this stage. ### Tests And Exit Criteria - Confirm no production-only helper or field remains solely to support tests, and no test loses meaningful behavioral coverage during cleanup. - Verify changed relative links and fenced examples. Search current-state docs for stale claims about comparison publication, debug behavior, results, or recovery. - 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` and `GOWORK=off go run ./cmd/weatherreporter compare --help` without credentials or network access, and confirm that help agrees with `docs/cli.md`. - Inspect the final diff for accidental generated artifacts, secrets, workspaces, vendored dependencies, release notes, or unrelated changes. ## Open Questions None. The roadmap and the contracts in Stages 11 through 15 define the remaining decisions needed to complete the corrective work.