diff --git a/docs/roadmap/diff.md b/docs/roadmap/diff.md index e8f48a2..64d07c4 100644 --- a/docs/roadmap/diff.md +++ b/docs/roadmap/diff.md @@ -138,5 +138,5 @@ analysis are outside this unit of work. The feature is complete when consumers can compare captured inspection views through PromptKit, receive ConfigKit's difference records without an additional local interpretation layer, and find accurate documentation for the operation -and its boundaries. A separate staged implementation plan can define the -implementation sequence. +and its boundaries. The companion [implementation plan](implementation.md) +defines the staged implementation sequence. diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md new file mode 100644 index 0000000..4e0d310 --- /dev/null +++ b/docs/roadmap/implementation.md @@ -0,0 +1,202 @@ +# Captured Configuration Comparison Implementation Plan + +Implement [the feature roadmap](diff.md) in the numbered order below. Each stage +is one bounded implementation prompt for a Luna coding agent. The feature +roadmap owns the purpose and target contract; this document owns the steps. + +## Working Instructions + +- Start with [the development guide](../development.md) and follow the + [agent policy](../policy/agents.md). Read the architecture, testing, + documentation, and OSS policies for the stages they govern. +- Preserve [ADR 0001](../adr/0001-configuration-boundary.md): compare captured + ConfigKit effective values for explicit inspection. Leave domain execution, + configuration validation, and source processing unchanged. +- Use the released ConfigKit dependency, not a committed local replacement or + workspace file. The sibling checkout is a reference, not a build prerequisite. +- Keep the upstream comparator and existing sensitivity classifier authoritative. + Do not add local diff algorithms, masking rules, output limits, reloads, + formatters, or execution-equivalence checks in this work. +- After each stage, review the complete relevant diff, run its checks, and commit + the completed unit with a plain-English message. Record actual validation + results in the handoff; do not bypass hooks or publish a release. + +## Current Baseline And Selected Decisions + +At planning time, PromptKit uses ConfigKit v0.6.0. The public inspection methods +are in `configuration_inspection.go`; `internal/configuration/inspection.go` +defines their narrow `Inspection` interface and currently returns the retained +`*configkit.Result[document]` directly. The internal engine stores that interface. +Root inspection contract tests live in +`configuration_inspection_contract_test.go`. + +Use ConfigKit v0.7.0, whose reviewed commit is +`8f3396d6f0cbe03af4f92bb2c1a1c9798171dd52`. +The public addition is exactly: + +```go +func (v ConfigurationInspection) Diff(other ConfigurationInspection) ([]configkit.Difference, error) +``` + +The receiver is left/earlier and the argument is right/later. Either zero view +returns `ErrInvalidConfig` and no records. Successful calls forward ConfigKit's +records; errors from ConfigKit remain inspectable through ordinary Go error +handling. Do not promise a particular nil-versus-empty slice on success or exact +error wording. No context parameter or PromptKit-specific difference type is +needed. + +For the private bridge, retain the existing `Inspection` interface and add an +unexported accessor for the captured `*configkit.Result[document]`. Implement the +interface with a private wrapper around that result. Existing inspection methods +can be promoted from an embedded result; the accessor supplies the result to an +internal-package comparison function. This keeps the implementation small without +runtime type assertions or exposing the configuration type through the facade. +This wrapper is an implementation choice, not a new consumer contract. + +## Stage 1: Adopt ConfigKit v0.7.0 + +Scope: dependency and attribution changes only. + +1. Read the upstream v0.7.0 release notes and `diff.go`, and verify the selected + tag's commit and licensing notices against the baseline above. +2. Update the dependency with + `go get gitea.maximumdirect.net/eric/ConfigKit@v0.7.0`, then `go mod tidy`. + Review `go.mod` and `go.sum`; avoid unrelated upgrades, Go-version changes, + or local replacements. +3. Update the ConfigKit entry in `docs/oss/attribution.md`: selected version and + all upstream source, COPYRIGHT, and LICENSE links must use the exact imported + commit. Preserve the house format and actual upstream notices. +4. Run `go test ./...`, `go vet ./...`, and + `python3 scripts/check-examples.py`. Investigate regressions rather than + weakening existing loading or execution contracts to accommodate them. +5. Review the dependency/notice diff and run `git diff --check` before committing. + +Completion: PromptKit uses the published v0.7.0 module, current behavior passes +existing validation, and attribution matches the selected source revision. + +## Stage 2: Add The Private Bridge And Public Method + +Scope: the small integration path and basic facade tests. + +1. In `internal/configuration/inspection.go`, add the private wrapper and + captured-result accessor described above. Update `loaded.Inspection` to + return the wrapper for a captured result, keeping absent state represented by + a nil inspection. Preserve all existing inspection operations. +2. Add an internal-package `Diff(left, right Inspection)` function returning + `([]configkit.Difference, error)`. Obtain the captured results through the + accessor and call `configkit.Diff[document]` once. If an internal operand is + absent, pass a nil result to ConfigKit so its existing error behavior applies. + Do not reload, reconstruct snapshots, or independently compare values. +3. Add `ConfigurationInspection.Diff` to the root facade. Check both source + interfaces for nil first and return `nil, ErrInvalidConfig` for a zero view; + otherwise delegate to the bridge. Return upstream errors directly unless + useful context is needed, in which case wrap with `%w`. +4. Add GoDoc for direction, zero-view errors, frozen effective-value scope, and + ConfigKit-owned difference semantics. Note that reports can include ordinary + application data and are not evidence of equivalent execution behavior. +5. Add external-package root tests using independent engines with synthetic + mapping inputs and an explicitly defined local backend/profile. Verify a + model-value change at a known path, reversed operand direction, equal values, + self-comparison, and the three zero-view combinations. Use `errors.Is` for + `ErrInvalidConfig` and `len(records) == 0` for successful equality. +6. Run `gofmt` on changed Go files and + `go test . ./internal/configuration ./internal/engine`, then + `git diff --check`. Existing inspection tests must continue to pass. + +Completion: the public signature works from package `promptkit_test`, the private +result remains inaccessible through the facade, and unchanged inspection methods +still behave as before. No runner or domain-package integration is needed. + +## Stage 3: Verify Result Semantics And Sensitive Changes + +Scope: focused facade integration tests, not a second comparator test suite. + +1. Extend the comparison tests with two valid engine configurations whose + effective values are equal but whose underlays/source labels differ. Verify + an empty diff; history differences must not become value differences. +2. Use valid JSON-compatible values in a synthetic provider `extra_params` map + to exercise absence versus explicit null, and a changed sequence. Verify the + forwarded `Present` flags and one atomic sequence-region record. Compare + semantic values rather than incidental YAML whitespace. +3. Add a focused sensitivity case using an existing credential-shaped key such + as `backends.local.extra_params.token`. Change its synthetic secret value and + a public sibling. Verify that neither operand's secret appears in either + snapshot, that redaction information and `HiddenChanged` survive the bridge, + and that the public change remains visible. +4. Include one valid shape-change case: an `extra_params.details` mapping with + a nested `token` on one side and a scalar at `details` on the other. This + places the sensitive descendant on only one side and exercises paired + protection of the changed region. Use ConfigKit's documented masking and + record granularity; do not require public descendants to remain visible in + an incompatible shape change or alter `secretProviderPath`. +5. Mutate a returned record's snapshot data and any mutable path representation + offered by the upstream public API, then compare again. Verify retained + state is unaffected without asserting upstream private allocation details. +6. Run `go test . ./internal/configuration` and `git diff --check`; format any + changed Go files. Reuse existing fixture helpers where they fit naturally. + +Completion: tests protect PromptKit's forwarding, operand pairing, sensitive-data +handling, and result ownership. Upstream retains ownership of exhaustive mapping, +sequence, sensitivity, and ordering cases. + +## Stage 4: Verify Frozen State And Concurrent Inspection + +Scope: lifecycle and concurrency integration tests. + +1. Reuse or extend the existing captured-source inspection fixture and counting + injected model client. Construct both engines while their sources are + readable; afterward make those sources unreadable or change their contents. + Obtain views and compare them repeatedly. Assert the captured differences + remain unchanged and comparison causes no additional source reads or model + calls. Set any environment values during setup, not concurrent operations. +2. Exercise several concurrent comparisons on the same pair of views, together + with an existing inspection operation. Validate returned semantic results + and collect goroutine errors safely. Avoid sleeps, timing-based thresholds, + or assertions about exact internal call sequences. +3. Where useful, extend the existing concurrent inspection/execution fixture + to include comparisons alongside offline runs. Reuse its coverage instead of + building a duplicate lifecycle harness. +4. Run `go test -race . ./internal/configuration ./internal/engine` and + `git diff --check`; format changed Go files. + +Completion: the comparison path demonstrably uses captured state and supports +concurrent read-only use without involving prompt execution machinery. + +## Stage 5: Document The Feature And Validate The Integrated Change + +Scope: canonical documentation and final verification. + +1. Add a small comparison example to the configuration-inspection section of + `docs/consumers/pkg-promptkit.md`. Show two captured views, error handling, + and iteration over returned records. Link to GoDoc and ConfigKit's v0.7.0 + comparison guidance for exact semantics rather than duplicating its API. +2. Explain the private inspection wrapper and comparison bridge in + `docs/internal/sources.md`. Refresh the existing ConfigKit v0.6.0-specific + normalization discussion against v0.7.0 without changing message resource + origins or execution normalization as part of this feature. +3. Keep the example and GoDoc explicit about captured configuration equality: + external file contents, resolved credential contents, post-load execution + preparation, and per-request overrides are not execution-equivalence inputs. + Retain the existing caller responsibility for report disclosure. +4. If the pending v0.13.0 release document is still unpublished and is the chosen + release for this work, add a short feature/dependency note with a canonical + documentation link. Otherwise leave published notes unchanged and record the + feature during the next release preparation. Do not tag, push, or run a + publication command in this stage. +5. Run `go test -race ./...`, `go vet ./...`, + `python3 scripts/check-examples.py`, and `git diff --check`. Check affected + local links and upstream links against the selected release's files. Review + exported declarations to confirm only the intended method was added. +6. Review the final diff against every work item in [the feature roadmap](diff.md), + confirming comparison/masking are delegated upstream and runtime/domain + components do not acquire ConfigKit inspection dependencies. Commit the + completed documentation and any required integration corrections. + +Completion: the dependency, API, tests, attribution, and canonical documentation +are coherent and the full existing suite and maintained examples pass. Report +stage commits and actual check results; release publication remains separate. + +## Open Questions + +None. The accepted roadmap and the existing inspection error convention supply +the decisions required for these stages.