96 lines
7.0 KiB
Markdown
96 lines
7.0 KiB
Markdown
# Resource Inspection Implementation Review
|
||
|
||
## Scope And Results
|
||
|
||
Reviewed the twelve stages of [implementation.md](implementation.md) against
|
||
the accepted [feature roadmap](inspect-backend.md) and the current code and
|
||
documentation at `f083ad5`. The implementation commits run from `8add7c4`
|
||
through `f083ad5`, following planning commit `8563379`.
|
||
|
||
The selected code architecture is implemented. Resource inspection delegates
|
||
from the facade through the internal engine to captured configuration records;
|
||
execution keeps its backend-completed targets, and requirement discovery keeps
|
||
its lightweight metadata path. Public projections detach mutable values.
|
||
Typed sources use ordinary ConfigKit mapping, composition, and resource context.
|
||
|
||
One documentation finding remains. No correctness defect or unintended new
|
||
restriction or guarantee was identified in the reviewed implementation. The
|
||
resource-oriented profile result and changed post-handoff null fallback are
|
||
deliberately selected compatibility changes, not accidental regressions.
|
||
|
||
## Findings
|
||
|
||
### R1. Reconcile Remaining Inspection Contract Documentation [P2]
|
||
|
||
**Status:** Open. **Affected stages:** 1, 11, and final reconciliation in 12.
|
||
|
||
**Locations:**
|
||
|
||
- [Package GoDoc](../../doc.go), lines 50–68: the JSON section still explicitly
|
||
lists `ProfileInspection` and `PromptInspection` among values without stable
|
||
JSON representations.
|
||
- [Configuration migration guidance](../consumers/configuration-migration.md),
|
||
lines 133–139: it still says that `InspectProfile` applies backend completion.
|
||
|
||
These statements contradict the implemented result contracts in
|
||
[resource_inspection.go](../../resource_inspection.go), the method GoDoc in
|
||
[engine.go](../../engine.go), and the updated
|
||
[consumer guidance](../consumers/pkg-promptkit.md#inspect-a-profile-before-prompt-work).
|
||
The result types now deliberately own JSON/YAML keys, nesting, and omission
|
||
semantics. Profile inspection reports only the resolved profile's fields;
|
||
backend-completed settings belong to request preparation.
|
||
|
||
**Impact:** Consumers consulting package documentation are told that the new
|
||
generic rendering contract is unavailable. Consumers following the migration
|
||
guide may still expect backend endpoints, credential sources, or provider
|
||
defaults in a profile report even though those fields can now be absent. This
|
||
undermines the primary migration and reporting purpose of the feature.
|
||
|
||
**Required correction:**
|
||
|
||
1. Update the package GoDoc's serialization description to include the selected
|
||
resource-result contract and remove the contradictory exclusions. Link to
|
||
the result/method owners for details. Preserve the distinction from source
|
||
inputs, provenance inspection, and unrelated values without a serialization
|
||
contract; do not add byte-format, ordering, or round-trip guarantees.
|
||
2. Replace the migration guide's backend-completion claim with the resolved
|
||
resource boundary and point readers needing completed request settings to
|
||
the existing preparation APIs/current consumer guidance.
|
||
3. Check the remaining current documentation for equivalent stale claims.
|
||
Published historical release notes should remain unchanged.
|
||
|
||
**Verification:** Review the rendered package GoDoc and the affected migration
|
||
passage against the implemented types and method behavior; check edited local
|
||
links and whitespace. This is a documentation correction and does not require
|
||
another implementation path, compatibility field, or new runtime test.
|
||
|
||
## Stage-By-Stage Review Record
|
||
|
||
| Stage | Evidence inspected and result |
|
||
| --- | --- |
|
||
| 1 — Result declarations | `resource_inspection.go` and nested diagnostic tags in `backends.go` match the selected fields and optional-pointer representation. Package-level serialization documentation still needs R1. |
|
||
| 2 — Null interpretation | `captureExecutionProfile` uses configured pointer/map presence; profile-only tri-state capture is removed. Target and isolation tests cover null/omission fallback, inherited null, empty-string clearing, map overlay, and the required-key rule. |
|
||
| 3 — Backend/profile resources | `internal/configuration/resources.go` reads indexed resolved records, not completed targets. Adapter tests cover partial/inherited profiles, derived queue defaults, missing IDs, and configured/inherited diagnostics. |
|
||
| 4 — Prompt resources | Prompt capture retains the resolved record, materialized messages, and required/repair presence. Prompt tests cover inherited imported text and omitted versus explicit values; inspection does not reopen sources or resolve referenced profiles/schemas. |
|
||
| 5 — Facade wiring | The internal engine's resource interface delegates to the snapshot after namespace qualification. Root conversion detaches pointers, collections, and nested diagnostics; backend not-found mapping is local to `InspectBackend`. Public error and cancellation tests pass. |
|
||
| 6 — Existing consumers | Superseded runner wrappers and their dedicated tests are removed. Execution helpers and the narrow requirement repository remain. Existing profile, inheritance, source, namespace, bundle, and preparation-example tests are migrated. |
|
||
| 7 — Generic encoding | `resource_serialization_test.go` encodes real results with ordinary JSON and YAML encoders and checks semantic keys, omission, explicit values, empty collections, diagnostic objects, and provider nulls. The existing YAML dependency is direct; its version is unchanged. |
|
||
| 8 — Isolation/selection | Public tests cover scoped backend lookup, qualified bypass, no namespace fallback, independent results, concurrent inspection/mutation, offline references, execution fallback, and retained null provenance. |
|
||
| 9 — Typed sources | `prompts.go` and `BackendSource` use explicit mapping construction with optional pointers/collections and sibling `extends`. They do not validate incomplete records, resolve inheritance, or read content files. |
|
||
| 10 — Real composition | Typed-source tests cover snapshots, layered completion, nested settings, order, inheritance, explicit values, relative file context, invalid final definitions, and empty top-level input. |
|
||
| 11 — Documentation | Main consumer, format, and internal guidance, README, and the unpublished v0.20.0 release note reflect the selected changes. Two remaining current-documentation contradictions require R1. |
|
||
| 12 — Combined validation | Race-enabled suite, vet, module tidy comparison, formatting, and changed Markdown link-target checks pass. The documentation finding prevents declaring reconciliation fully complete. |
|
||
|
||
## Validation Performed
|
||
|
||
Using Go 1.27.3 with `GOTOOLCHAIN=local`, `GOWORK=off`, and `GOPROXY=off`:
|
||
|
||
- `go test -race ./...` passed, including the maintained offline examples.
|
||
- `go vet ./...` passed.
|
||
- `go mod tidy -diff` reported no changes.
|
||
- Formatting checks on surviving changed Go files and `git diff --check` passed.
|
||
- Local link targets in the changed Markdown documents exist.
|
||
|
||
The review made no production-code or test changes. These checks do not include
|
||
live provider diagnostics, release publication, or downstream application changes.
|