313 lines
19 KiB
Markdown
313 lines
19 KiB
Markdown
# Public Facade Refactor Implementation Plan
|
|
|
|
## Scope And Authority
|
|
|
|
Implement the [public facade roadmap](public-facade.md) without changing the
|
|
consumer API or execution behavior. This plan owns implementation sequence;
|
|
the feature roadmap owns the target state and compatibility requirements.
|
|
The provisional [profile policy](profile-policy.md) is a separate, subsequent
|
|
scope. Do not update catalog dependencies, introduce standard profiles, change
|
|
profile stability promises, or prepare a release in these stages.
|
|
|
|
Read `AGENTS.md`, the [development guide](../development.md), and all three
|
|
policies before starting: [architecture](../policy/architecture.md),
|
|
[documentation](../policy/documentation.md), and [testing](../policy/testing.md).
|
|
Use existing GoDoc and canonical consumer/format documentation to distinguish
|
|
intentional contracts from incidental implementation. If a relocation appears
|
|
to require a consumer-facing change, stop and report it rather than silently
|
|
changing the contract. Existing exported declarations are not removal candidates
|
|
merely because a guide does not mention them.
|
|
|
|
Each numbered stage is one implementation prompt. Complete stages in order;
|
|
keep the repository building and maintain one active production path after
|
|
each stage. Update affected durable documentation when its described ownership
|
|
changes, rather than leaving inaccurate descriptions until the final stage.
|
|
For each stage, run focused tests while developing and the complete
|
|
[maintainer validation workflow](../development.md#maintainer-validation)
|
|
before acceptance. Report changes, validation results, and any unresolved
|
|
failures. Do not use live providers, credentials, sibling repositories, module
|
|
replacements, or a Go workspace to validate the result.
|
|
|
|
## Implementation Decisions
|
|
|
|
- Add `internal/engine` as the composition owner. It owns engine construction,
|
|
defaults, source assembly, catalog loading, registry and capacity construction,
|
|
and runner/repairer wiring. It delegates execution to `internal/usecase`;
|
|
it does not become a second runner.
|
|
- Keep root-defined public types, errors, options, conversions, collaborator
|
|
adapters, and formatting. Root `Engine` holds a private `*engine.Engine`.
|
|
The internal engine exposes only the domain-level operations needed for
|
|
the existing facade methods. No public bridge methods or internal aliases.
|
|
- Preserve root `Option` and its sealed `apply(*engineOptions) error` mechanism.
|
|
Ultimately, private root `engineOptions` wraps an internal `engine.Options`
|
|
value whose private state and setters own source construction and option
|
|
accumulation. Root options remain closures applying in caller order; do not
|
|
replace them with a deferred list that changes when errors or copies occur.
|
|
- Put the memory profile repository in `internal/profile`. Existing domain,
|
|
profile validation, and JSON-copy helpers remain the owners of their rules.
|
|
Do not duplicate validation in the facade and repository.
|
|
- Keep public error classification and redacted public error construction in
|
|
the root. Internal construction returns descriptive errors; `NewEngine`
|
|
translates construction failures to `ErrInvalidConfig` as it does today.
|
|
Runtime errors continue through `mapPublicError` at the public boundary.
|
|
- Keep `PreparedExecution` root-defined with its existing private use-case
|
|
handle. Pass that internal handle through engine delegation without copying
|
|
its execution state or creating another lifecycle owner. Private root
|
|
references to specialized internal packages are allowed.
|
|
- Do not impose a single internal import on the root, introduce a generic
|
|
builder framework, add caching, broaden validation, or migrate all private
|
|
helpers merely to reduce root line count. Straightforward public convenience
|
|
constructors and boundary conversions may remain where they are.
|
|
|
|
## Stage 1: Establish The Compatibility Baseline
|
|
|
|
Purpose: make the compatibility review concrete before moving ownership.
|
|
|
|
1. Read the root declarations and tests, the
|
|
[consumer guide](../consumers/pkg-promptkit.md),
|
|
[component overview](../internal/overview.md),
|
|
[sources document](../internal/sources.md), and
|
|
[runner document](../internal/runner.md). Identify existing contract coverage
|
|
for constructor options, profile sources, injection, errors, and prepared
|
|
execution. Read their owning internal tests before adding new coverage.
|
|
2. Record the pre-refactor commit ID in this plan and capture an exported API
|
|
inventory for the final comparison. Include exported declarations, named
|
|
types, fields and tags, interface signatures, value and pointer method sets,
|
|
constants, error variables, and serialization/formatting contracts. `go doc
|
|
-all` is a useful starting point, not a complete method-set comparison.
|
|
Temporary comparison output belongs outside tracked source; do not introduce
|
|
a permanent snapshot of private structure or freeze every future API addition.
|
|
3. Use existing tests first. Add only missing, consequential public regression
|
|
coverage for relocation risks: source replacement versus backend accumulation,
|
|
option application/capture timing, ownership of injected values and profiles,
|
|
nil/zero facade behavior, and typed error translation. Keep public behavior
|
|
tests in the external test package where practical. Do not duplicate full
|
|
source, transport, capacity, or runner matrices.
|
|
4. In particular, inspect the existing contracts around `WithProfiles` copying
|
|
during option application and `WithBackend` capturing the queue-capacity
|
|
pointer value during option creation. Preserve explicit zero versus unset.
|
|
Keep errors from an invalid earlier option observable even if a later option
|
|
would replace it. Do not add reflection-based typed-nil rejection or eagerly
|
|
read lazy consumer sources as part of the refactor.
|
|
|
|
Acceptance: the pre-refactor baseline is identified, necessary regression
|
|
coverage passes, and no production ownership or behavior has changed. If the
|
|
existing tests already cover these risks, do not add tests solely to fill this
|
|
stage; record the coverage reviewed and proceed.
|
|
|
|
### Completion
|
|
|
|
- [x] Pre-refactor baseline commit: `7a858c2365e1a3625de67a46f5c488e0fe31856b`.
|
|
- [x] Captured the exported API inventory outside the tracked repository for
|
|
the final comparison.
|
|
- [x] Reviewed existing public coverage for source replacement, backend
|
|
accumulation, option error and capture timing, injected-value and profile
|
|
ownership, nil and zero facade behavior, prepared execution, and typed error
|
|
translation. The existing tests cover these relocation risks without a
|
|
duplicate matrix.
|
|
|
|
## Stage 2: Relocate The Memory Profile Repository
|
|
|
|
Purpose: move a self-contained repository before changing engine assembly.
|
|
|
|
1. Introduce `internal/profile.NewMemoryRepository` accepting internal profile
|
|
definitions. Move the map-backed repository and lookup implementation from
|
|
root `profiles.go` to that package. Preserve normalized duplicate-ID detection,
|
|
definition validation, missing-profile classification, inheritance metadata,
|
|
and independently owned values on storage and lookup.
|
|
2. Separate public field conversion from repository normalization as necessary.
|
|
Root `WithProfiles` converts at application time and invokes the new internal
|
|
constructor. Use the existing definition-normalization and JSON-copy owners
|
|
once at the repository boundary; preserve rejection of invalid extra values
|
|
and deep-copy guarantees without an unnecessary second validation pass.
|
|
Audit other callers before changing the responsibilities of `toDomainProfile`.
|
|
3. Remove the root repository implementation. Leave public `Profile` and
|
|
`OpenAICompatibleProfile` unchanged, including the convenience constructor's
|
|
documented copy behavior; do not strengthen that contract incidentally.
|
|
4. Move private repository tests to `internal/profile`, adapting inputs to
|
|
internal values. Retain representative public `WithProfiles` integration and
|
|
conversion coverage at the root. Add only uncovered copy/validation cases.
|
|
Update the internal source document and inventory for the new owner.
|
|
|
|
Acceptance: `WithProfiles` uses the internal repository in the active engine;
|
|
public behavior is unchanged and no duplicate root repository remains.
|
|
|
|
### Completion
|
|
|
|
- [x] Moved the in-memory repository, normalization, validation, and defensive
|
|
copies into `internal/profile`.
|
|
- [x] Kept public conversion and `WithProfiles` application timing at the root.
|
|
- [x] Moved focused repository coverage to `internal/profile` and retained
|
|
facade-level copy timing coverage.
|
|
- [x] Updated the implemented component and source ownership documentation.
|
|
|
|
## Stage 3: Move Engine Assembly And Delegation Inward
|
|
|
|
Purpose: establish the internal engine while leaving source-option relocation
|
|
for the next prompt.
|
|
|
|
1. Add internal `Config`, `Options`, and `Engine` types. Internal `Config` carries
|
|
the existing construction settings, not a new consumer configuration API.
|
|
For this stage, `Options` may carry the already-prepared collaborators,
|
|
repositories, backend definitions, and source-presence flags from the root
|
|
option state. Do not lose distinctions between an absent source and an
|
|
explicitly supplied empty source. Stage 4 removes this transitional mapping.
|
|
2. Move the assembly portion of root `NewEngine`, after option application,
|
|
into `internal/engine.New`, including maintained module imports and
|
|
`newProfileRepository`. Preserve the existing assembly semantics:
|
|
- prompt-source requirement and configured source defaults;
|
|
- validated maintained catalogs and engine-local backend registry;
|
|
- raw profile precedence: memory, ordinary configured source, application
|
|
fallback, maintained catalog; an explicit ordinary source replaces
|
|
`Config.ProfileDir`, and one resolver wraps the complete overlay;
|
|
- schema default and supplied validator selection;
|
|
- supplied or default client and artifact reader selection; and
|
|
- one capacity manager per engine, one capacity wrapper around the selected
|
|
client, and that same wrapped client used by runner and repairer.
|
|
3. Give internal `Engine` domain-level forwarding methods for the existing
|
|
prepare, run, prepared-execution, and inspection operations. Keep runner
|
|
orchestration and prepared-handle lifecycle in `internal/usecase`. Do not
|
|
add another queue, retry loop, mutex, source cache, or execution state machine.
|
|
4. Change root `Engine` to hold the internal engine. Root `NewEngine` still
|
|
applies options and maps constructor errors. Root methods still check
|
|
nil/zero receivers, convert/validate public requests at their existing
|
|
boundary, convert results, and translate runtime errors. Do not wrap an
|
|
error twice or expose internal typed provider errors instead of public ones.
|
|
5. Adapt tests that inspect private runner fields to the new owner or, where
|
|
feasible, observable contract assertions. Reuse existing tests for prepared
|
|
handle engine binding, shared copied-handle state, consumption/discard,
|
|
cancellation, redaction, repair, and capacity. Add an assembly test only if
|
|
a consequential wiring obligation is otherwise untested.
|
|
6. Update architecture, component inventory, sources, and runner documentation
|
|
to describe the implemented split. Remove the superseded root assembly path.
|
|
|
|
Acceptance: all public operations route through one internal engine and the
|
|
existing runner; maintained catalogs and component wiring are internally owned.
|
|
Prepared-execution behavior and public error contracts remain unchanged.
|
|
|
|
### Completion
|
|
|
|
- [x] Added `internal/engine` as the owner of engine assembly and runner
|
|
delegation.
|
|
- [x] Kept the public facade's option application, conversions, nil checks,
|
|
prepared-execution wrapper, and error translation at the root.
|
|
- [x] Updated architecture, component, source, and runner documentation for
|
|
the implemented ownership split.
|
|
|
|
## Stage 4: Move Option State And Source Construction Inward
|
|
|
|
Purpose: finish the ownership split without changing the public option API.
|
|
|
|
1. Replace the root collaborator/flag record with a private wrapper around
|
|
`engine.Options`. Move the construction state into private fields of that
|
|
internal type. Remove Stage 3's transitional root-to-internal state mapping.
|
|
Internal setters correspond to the existing source, profile, backend, client,
|
|
and artifact options; no generic option registry or callback framework.
|
|
2. Move filesystem/`fs.FS` repository and validator construction, `fileSource`,
|
|
source-presence flags, replacement rules, and backend accumulation into
|
|
internal option methods. Invoke them during each root option's `apply`, not
|
|
collectively after all options. Preserve current file checks and lazy source
|
|
loading; do not add path restrictions or normalize actual paths beyond the
|
|
existing contract. Preserve separate blank checks for whitespace-bearing
|
|
valid paths.
|
|
3. Keep public-to-internal conversion and public collaborator adapters at the
|
|
root. Preserve closure capture and validation timing from Stage 1, including
|
|
queue-capacity capture, profile copies, nil options, and ordinary interface
|
|
nil checks. Internal methods consume internal values/interfaces without
|
|
importing root types. Do not copy injected client or reader implementations.
|
|
4. Root `NewEngine` now performs only ordered option application, configuration
|
|
translation, internal construction, and public error translation. Review the
|
|
remaining root imports/helpers: retain boundary work, remove dead assembly
|
|
helpers, and do not relocate formatting or conversion for appearance alone.
|
|
5. Move implementation-only option/source setup tests with their owner, while
|
|
retaining public timing and error assertions at the facade. Update affected
|
|
documentation immediately. No exact private field layout becomes a test
|
|
requirement.
|
|
|
|
Acceptance: the root no longer constructs repositories, validators, registries,
|
|
capacity managers, catalogs, or runners. Public options still have their existing
|
|
names, signatures, ordering, capture, and error behavior. No transitional state
|
|
mapping or duplicate source-assembly path remains.
|
|
|
|
### Completion
|
|
|
|
- [x] Wrapped the root option state around internal engine options with private
|
|
construction state.
|
|
- [x] Moved filesystem and file source setup, source replacement, profile
|
|
repository construction, validator construction, and backend accumulation
|
|
into internal option methods.
|
|
- [x] Kept ordered public option application, boundary conversion, collaborator
|
|
adaptation, and error translation in the root facade.
|
|
- [x] Updated current architecture and internal ownership documentation.
|
|
|
|
## Stage 5: Guard The Public Boundary And Dependency Direction
|
|
|
|
Purpose: protect the intended architecture without freezing private layout.
|
|
|
|
1. Add a focused root architecture test using Go type information. Load the
|
|
compiled root package with standard-library `go list` export data and
|
|
`go/importer`/`go/types`; avoid a new production dependency or parsing only
|
|
surface syntax. It must work in the ordinary offline maintainer workflow
|
|
once normal module dependencies are available.
|
|
2. Inspect exported declarations, public field types, function signatures,
|
|
interface methods, and both pointer and value method sets. Follow nested
|
|
type expressions and aliases with a visited set to avoid recursion. Reject
|
|
references to this module's internal types in accessible public surfaces,
|
|
public aliases to those types, and internal anonymous embedding that exposes
|
|
implementation methods. Account for generics if encountered rather than
|
|
silently ignoring their signatures or constraints.
|
|
3. Permit private internal fields and the sealed `Option` method. Do not follow
|
|
inaccessible fields as if they were public contracts or reject all aliases
|
|
indiscriminately. Check accessible promoted methods even if the embedded
|
|
field itself is unexported. Do not require the root to import only
|
|
`internal/engine`, or impose rules on unrelated third-party internals.
|
|
4. Check production package imports, using package metadata, for any dependency
|
|
from this module's `internal/` packages to its root. Exclude external test
|
|
packages from that restriction. Do not assert an exact internal package list.
|
|
5. Exercise the boundary checker with a small in-memory fixture set: allowed
|
|
private internal state and sealed option; forbidden direct/nested internal
|
|
signature or field; forbidden internal alias and promoted method. Keep the
|
|
test helper local and proportionate, not a reusable architecture framework.
|
|
|
|
Acceptance: the completed refactor passes the checks; representative violations
|
|
fail for the intended reason, and ordinary private implementation changes remain
|
|
possible. No ConfigKit checkout or other sibling is needed.
|
|
|
|
## Stage 6: Complete Compatibility And Documentation Review
|
|
|
|
Purpose: verify the whole refactor, not just successful compilation of each move.
|
|
|
|
1. Compare the final exported API against the Stage 1 commit, including method
|
|
sets, fields/tags, type ownership, interfaces, and public error identities.
|
|
Review any difference; no API additions/removals/redesigns are authorized.
|
|
Inspect supported serialization and formatting alongside the existing tests.
|
|
2. Review the final root package for retained business assembly or duplicated
|
|
repositories, and the internal engine for duplicated runner behavior. Review
|
|
test moves for lost assertions and duplicate matrices. Existing specialized
|
|
packages should still own their detailed behavior tests; facade tests should
|
|
prove conversion, compatibility, and representative assembled behavior.
|
|
3. Finish the durable documentation review: architecture owns dependency rules;
|
|
the internal overview inventories owners; sources and runner docs explain
|
|
their current implementation. Correct stale links and root-assembly claims.
|
|
Keep public GoDoc authoritative and avoid adding consumer migration steps,
|
|
format rules, or new guarantees for an internal relocation. No separate ADR
|
|
is needed merely to restate the existing facade policy.
|
|
4. Run the entire maintainer workflow, including race tests, vet, build, both
|
|
offline examples, formatting, local links, and repository hygiene checks.
|
|
Review the complete diff against the baseline. Report any environmental
|
|
validation blocker explicitly; do not call an unrun check successful.
|
|
5. Report completion against the feature roadmap criteria. Retain these two
|
|
temporary documents for the maintainer's completion review; their later
|
|
retirement follows documentation policy. Do not start the profile-policy
|
|
work or tag a release as part of this plan.
|
|
|
|
Acceptance: compatibility review finds no unauthorized changes, all required
|
|
validation passes, documentation matches the implemented architecture, and the
|
|
refactor is ready for independent review before subsequent feature work.
|
|
|
|
## Open Questions
|
|
|
|
None requiring a product decision for this scope. The refactor preserves the
|
|
existing API and behavior; the provisional catalog policy and its release
|
|
decisions remain outside this plan.
|