diff --git a/docs/internal/overview.md b/docs/internal/overview.md index 790c25c..0d4bbb9 100644 --- a/docs/internal/overview.md +++ b/docs/internal/overview.md @@ -22,7 +22,7 @@ contributor workflow and validation. | `internal/filecatalog` | Provides deterministic YAML discovery and path helpers for operating-system filesystems and `fs.FS` sources. | [File catalog](../../internal/filecatalog/catalog.go) | | `internal/jsonvalue` | Validates and deeply copies bounded JSON-compatible extra-parameter and prepared-schema trees while preserving supported concrete value types and rejecting cycles or excessive depth and work. | [JSON values](../../internal/jsonvalue/jsonvalue.go) | | `internal/promptdef` | Loads strictly decoded, validated prompt definitions from filesystem and `fs.FS` sources, including version selection and contained file-backed message content. | [Framework formats](../formats.md), [prompt-definition repository](../../internal/promptdef/filesystem_repository.go) | -| `internal/profile` | Loads strictly decoded, locally validated execution profiles from filesystem and `fs.FS` sources, overlays raw sources with error-preserving fallback, and resolves inherited profiles. | [Framework formats](../formats.md), [profile repositories](../../internal/profile/filesystem_repository.go), [internal sources](sources.md#profiles-and-built-ins) | +| `internal/profile` | Owns immutable in-memory and filesystem profile repositories, local profile validation, raw-source overlays with error-preserving fallback, and inherited-profile resolution. | [Framework formats](../formats.md), [profile repositories](../../internal/profile/filesystem_repository.go), [internal sources](sources.md#profiles-and-built-ins) | | `internal/prompt` | Renders prompt messages from Go templates with artifact, variable, session, and cache-control data. | [Go-template renderer](../../internal/prompt/go_renderer.go) | | `internal/artifact` | Resolves ordinary inline and unrestricted caller-selected file references into copied artifacts with metadata and hashes. | [Internal sources and validation](sources.md) | | `internal/validate` | Validates basic, JSON, and JSON Schema output using operating-system filesystem or `fs.FS` schema sources and creates operation-local validation plans with canonical contained schema resources. | [Framework formats](../formats.md#schemas), [internal sources and validation](sources.md) | diff --git a/docs/internal/sources.md b/docs/internal/sources.md index ff09970..8e4034e 100644 --- a/docs/internal/sources.md +++ b/docs/internal/sources.md @@ -45,8 +45,10 @@ duplicate detection, and source containment: ## Profiles And Built-Ins -`internal/profile` loads, locally validates, overlays, and resolves execution -profiles from an operating-system filesystem or an `fs.FS`. A file contains +`internal/profile` owns immutable in-memory and filesystem repositories, local +validation, overlays, and resolution for execution profiles. In-memory +definitions are normalized and validated once at that repository boundary and +are defensively copied on storage and lookup. A file contains exactly one YAML document and its trimmed YAML `id` is its only selection identity; filenames do not confer authority. Each point lookup reads discovered files once for their metadata and reuses the selected file's bytes for strict diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 0efc8bf..9d2003b 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -132,6 +132,15 @@ Purpose: move a self-contained repository before changing engine assembly. 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 diff --git a/engine.go b/engine.go index 3bcf8c9..5e2ab8c 100644 --- a/engine.go +++ b/engine.go @@ -314,7 +314,11 @@ func WithFallbackProfileFS(fsys fs.FS, root string) Option { // earlier in-memory set rather than merging it. func WithProfiles(profiles ...Profile) Option { return optionFunc(func(options *engineOptions) error { - repo, err := newMemoryProfileRepository(profiles) + definitions := make([]domain.ExecutionProfile, len(profiles)) + for index, publicProfile := range profiles { + definitions[index] = toDomainProfile(publicProfile) + } + repo, err := profile.NewMemoryRepository(definitions) if err != nil { return err } diff --git a/engine_test.go b/engine_test.go index 0e9b6f8..b361c01 100644 --- a/engine_test.go +++ b/engine_test.go @@ -2546,6 +2546,38 @@ func TestInMemoryProfileExtraParamsAreCopiedAcrossPublicBoundary(t *testing.T) { } } +func TestWithProfilesCopiesDefinitionsWhenApplied(t *testing.T) { + nested := map[string]any{"value": "when option is created"} + option := promptkit.WithProfiles(promptkit.Profile{ + ID: "profile", + Endpoint: "http://copy.example/v1", + Model: "copy-model", + ExtraParams: map[string]any{"nested": nested}, + }) + nested["value"] = "when option is applied" + + engine, err := promptkit.NewEngine(promptkit.Config{PromptDir: frameworkPromptDir}, option) + if err != nil { + t.Fatalf("construct engine: %v", err) + } + nested["value"] = "after construction" + + prepared, err := engine.Prepare(context.Background(), promptkit.RunRequest{ + PromptID: frameworkMarkdownSummaryPromptID, + ProfileID: "profile", + Inputs: map[string]promptkit.ArtifactRef{ + "transcript": promptkit.Inline("Rin opens the gate."), + "glossary": promptkit.Inline("gate: A guarded passage."), + }, + }) + if err != nil { + t.Fatalf("prepare with copied profile: %v", err) + } + if got := prepared.EffectiveModelParams.ExtraParams["nested"].(map[string]any)["value"]; got != "when option is applied" { + t.Fatalf("profile copy value = %q, want value at option application", got) + } +} + func TestWithProfilesRejectsInvalidExtraParams(t *testing.T) { tests := []struct { name string diff --git a/internal/profile/memory_repository.go b/internal/profile/memory_repository.go new file mode 100644 index 0000000..67fb0de --- /dev/null +++ b/internal/profile/memory_repository.go @@ -0,0 +1,50 @@ +package profile + +import ( + "context" + "fmt" + + "gitea.maximumdirect.net/eric/promptkit/internal/domain" + "gitea.maximumdirect.net/eric/promptkit/internal/jsonvalue" +) + +// NewMemoryRepository constructs an immutable repository from profile +// definitions supplied by an internal caller. +func NewMemoryRepository(definitions []domain.ExecutionProfile) (Repository, error) { + repository := &memoryRepository{profiles: make(map[string]domain.ExecutionProfile, len(definitions))} + for _, definition := range definitions { + extraParams, err := jsonvalue.CopyMap(definition.ExtraParams) + if err != nil { + return nil, err + } + definition.ExtraParams = extraParams + if err := NormalizeAndValidateDefinition(&definition); err != nil { + return nil, err + } + if _, exists := repository.profiles[definition.ID]; exists { + return nil, fmt.Errorf("duplicate profile id %q", definition.ID) + } + repository.profiles[definition.ID] = definition + } + return repository, nil +} + +type memoryRepository struct { + profiles map[string]domain.ExecutionProfile +} + +func (r *memoryRepository) GetProfile(_ context.Context, id string) (*domain.ExecutionProfile, error) { + if r == nil { + return nil, ErrProfileNotFound + } + definition, found := r.profiles[id] + if !found { + return nil, ErrProfileNotFound + } + extraParams, err := jsonvalue.CopyMap(definition.ExtraParams) + if err != nil { + return nil, fmt.Errorf("copy memory profile %q: %w", definition.ID, err) + } + definition.ExtraParams = extraParams + return &definition, nil +} diff --git a/internal/profile/memory_repository_test.go b/internal/profile/memory_repository_test.go new file mode 100644 index 0000000..edd3c01 --- /dev/null +++ b/internal/profile/memory_repository_test.go @@ -0,0 +1,91 @@ +package profile + +import ( + "context" + "errors" + "testing" + + "gitea.maximumdirect.net/eric/promptkit/internal/domain" +) + +func TestMemoryRepositoryOwnsNormalizedDefinitions(t *testing.T) { + nested := map[string]any{"value": "original"} + definitions := []domain.ExecutionProfile{{ + ID: " profile ", + BackendID: " backend ", + Endpoint: " https://example.test/v1/ ", + Model: "model", + ExtraParams: map[string]any{"nested": nested}, + }} + + repository, err := NewMemoryRepository(definitions) + if err != nil { + t.Fatalf("construct repository: %v", err) + } + nested["value"] = "mutated input" + + first, err := repository.GetProfile(context.Background(), "profile") + if err != nil { + t.Fatalf("load profile: %v", err) + } + if first.ID != "profile" || first.BackendID != "backend" || first.Endpoint != "https://example.test/v1/" { + t.Fatalf("normalized definition = %#v", first) + } + if first.ExtraParams["nested"].(map[string]any)["value"] != "original" { + t.Fatalf("repository retained input mutation: %#v", first.ExtraParams) + } + first.ExtraParams["nested"].(map[string]any)["value"] = "mutated result" + + second, err := repository.GetProfile(context.Background(), "profile") + if err != nil { + t.Fatalf("reload profile: %v", err) + } + if second.ExtraParams["nested"].(map[string]any)["value"] != "original" { + t.Fatalf("repository retained result mutation: %#v", second.ExtraParams) + } +} + +func TestMemoryRepositoryPreservesInheritanceAndRejectsInvalidDefinitions(t *testing.T) { + derived, err := NewMemoryRepository([]domain.ExecutionProfile{{ + ID: "derived", + BaseProfileID: "base", + ReasoningEffort: "high", + }}) + if err != nil { + t.Fatalf("construct derived repository: %v", err) + } + profile, err := derived.GetProfile(context.Background(), "derived") + if err != nil { + t.Fatalf("load derived profile: %v", err) + } + if profile.BaseProfileID != "base" || profile.ReasoningEffort != "high" { + t.Fatalf("derived profile = %#v", profile) + } + if _, err := derived.GetProfile(context.Background(), "missing"); !errors.Is(err, ErrProfileNotFound) { + t.Fatalf("missing profile error = %v, want ErrProfileNotFound", err) + } + + tests := []struct { + name string + definitions []domain.ExecutionProfile + }{ + { + name: "normalized duplicate IDs", + definitions: []domain.ExecutionProfile{ + {ID: " duplicate ", Endpoint: "https://one.example/v1", Model: "one"}, + {ID: "duplicate", Endpoint: "https://two.example/v1", Model: "two"}, + }, + }, + { + name: "invalid extra parameters", + definitions: []domain.ExecutionProfile{{ID: "invalid", Endpoint: "https://example.test/v1", Model: "model", ExtraParams: map[string]any{"invalid": func() {}}}}, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if _, err := NewMemoryRepository(test.definitions); err == nil { + t.Fatal("expected repository construction error") + } + }) + } +} diff --git a/profiles.go b/profiles.go index 7ed7218..b01c826 100644 --- a/profiles.go +++ b/profiles.go @@ -1,12 +1,7 @@ package promptkit import ( - "context" - "fmt" - "gitea.maximumdirect.net/eric/promptkit/internal/domain" - "gitea.maximumdirect.net/eric/promptkit/internal/jsonvalue" - "gitea.maximumdirect.net/eric/promptkit/internal/profile" ) // OpenAICompatibleProfile returns an in-memory Profile for an OpenAI-compatible @@ -50,43 +45,8 @@ func copyShallowAnyMap(src map[string]any) map[string]any { return out } -type memoryProfileRepository struct { - profiles map[string]domain.ExecutionProfile -} - -func newMemoryProfileRepository(profiles []Profile) (*memoryProfileRepository, error) { - repo := &memoryProfileRepository{profiles: make(map[string]domain.ExecutionProfile, len(profiles))} - for _, publicProfile := range profiles { - prof, err := toDomainProfile(publicProfile) - if err != nil { - return nil, err - } - if _, exists := repo.profiles[prof.ID]; exists { - return nil, fmt.Errorf("duplicate profile id %q", prof.ID) - } - repo.profiles[prof.ID] = prof - } - return repo, nil -} - -func (r *memoryProfileRepository) GetProfile(_ context.Context, id string) (*domain.ExecutionProfile, error) { - if r == nil { - return nil, profile.ErrProfileNotFound - } - prof, ok := r.profiles[id] - if !ok { - return nil, profile.ErrProfileNotFound - } - prof.ExtraParams = copyAnyMap(prof.ExtraParams) - return &prof, nil -} - -func toDomainProfile(publicProfile Profile) (domain.ExecutionProfile, error) { - extraParams, err := jsonvalue.CopyMap(publicProfile.ExtraParams) - if err != nil { - return domain.ExecutionProfile{}, err - } - prof := domain.ExecutionProfile{ +func toDomainProfile(publicProfile Profile) domain.ExecutionProfile { + return domain.ExecutionProfile{ ID: publicProfile.ID, BaseProfileID: publicProfile.BaseProfileID, BackendID: publicProfile.BackendID, @@ -99,10 +59,6 @@ func toDomainProfile(publicProfile Profile) (domain.ExecutionProfile, error) { ServiceTier: publicProfile.ServiceTier, ReasoningEffort: publicProfile.ReasoningEffort, APIKeyRequired: publicProfile.APIKeyRequired, - ExtraParams: extraParams, + ExtraParams: publicProfile.ExtraParams, } - if err := profile.NormalizeAndValidateDefinition(&prof); err != nil { - return domain.ExecutionProfile{}, err - } - return prof, nil }