From deebc89255fecadd2d1acd85067a21ca00a83a73 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 29 Aug 2026 12:34:31 +0000 Subject: [PATCH] Make command line references pipeline scoped --- docs/cli.md | 44 ++- docs/config.md | 13 +- docs/consumers/dnd-pipeline.md | 16 +- docs/consumers/subprocess.md | 37 ++- docs/internal/cli.md | 15 +- docs/internal/pipeline.md | 7 + docs/roadmap/cli-pipeline-references.md | 258 ++++++++++++++++++ internal/cli/reference_contract_test.go | 332 ++++++++++++++++------- internal/cli/run.go | 340 +++++++++++++++--------- 9 files changed, 824 insertions(+), 238 deletions(-) create mode 100644 docs/roadmap/cli-pipeline-references.md diff --git a/docs/cli.md b/docs/cli.md index a9a67ebf..9002fa1f 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -55,8 +55,8 @@ pipeline ID and **--input** are required. | **--session-id id** | Override the generated prompt session identifier with a non-empty value for LLM-backed module calls. | | **--reasoning-effort value** | Replace the selected PromptKit profile's reasoning effort for every LLM-backed call in this run. The value must be non-empty and the flag may be specified only once. | | **--clear-reasoning-effort** | Clear reasoning effort inherited from the selected PromptKit profile for every LLM-backed call in this run. | -| **--reference selector=path** | Add or replace a file reference binding. Repeatable. | -| **--without-reference selector** | Remove a configured optional reference binding. Repeatable. | +| **--reference selector=path** | Add or replace external file reference bindings at pipeline, lane, chunk, or binding scope. Repeatable. | +| **--without-reference selector** | Remove matching configured external reference bindings. Repeatable. | **--chunk_cache** accepts only **auto**, **bypass**, or **refresh**. **--debug-dir**, **--output-dir**, **--session-id**, and @@ -81,23 +81,49 @@ guidance. ### Reference selectors -Use **--reference** only for a reference slot declared by the selected -configured target. The accepted selector forms are: +Use **--reference** only for reference slots declared by the selected +configured targets. Qualification narrows the scope of an override: | Form | Target | | --- | --- | -| slot=path | The unique selected target that declares slot. | +| slot=path | Every selected target that declares slot. | | chunk.slot=path | The chunker. | -| merge.slot=path | The unique selected merger that declares slot. | -| lane.slot=path | The unique extractor, merger, or normalizer in lane that declares slot. | +| lane.slot=path | Every extractor, merger, or normalizer in lane that declares slot. | | lane.extract.slot=path | The extractor in lane. | | lane.merge.slot=path | The merger in lane. | | lane.normalize.slot=path | The normalizer in lane. | -**--without-reference** uses the same selector forms without =path. Slot -names, requiredness, and configured bindings are part of the +Pipeline- and lane-scoped selectors are expected to match multiple targets and +fail if they match none. A stage-specific selector fails when its lane is not +selected or its target does not declare the slot. There is no stage-wide +`merge.slot` shorthand; name the lane when targeting a merger. + +CLI bindings override configured external paths. For overlapping CLI +selectors, a binding-specific or chunk selector overrides a lane selector, and +a lane selector overrides a pipeline selector. The last occurrence wins at +equal scope. Binding and unbinding the same concrete target at equal scope is +an error; a narrower bind or unbind may create an intentional exception to a +broader action. + +**--without-reference** uses the same selector forms without `=path` and +removes external bindings only. Neither flag replaces or removes a generated +artifact handoff; an external/generated collision is a resolution error. +Required slots are checked after all effective changes. CLI reference paths +are resolved relative to the process working directory, so subprocess and +service callers should use absolute paths. Slot names, accepted media types, +size limits, requiredness, and configured generated bindings are part of the [configuration contract](config.md). +For example, one shared campaign reference can reach every compatible target, +with an optional lane-specific exception: + +~~~ +notarius run dnd-session \ + --input /data/transcript.json \ + --reference party=/data/references/party.txt \ + --reference npc-registry.party=/data/references/npc-party-context.txt +~~~ + ### Run output Without **--json**, standard output contains the completed pipeline ID, counts diff --git a/docs/config.md b/docs/config.md index 0ef98631..43e635b7 100644 --- a/docs/config.md +++ b/docs/config.md @@ -411,10 +411,15 @@ slot. A generated binding supplies one accepted normalized artifact; it does not name a file. A configured generated dependency remains required even when that consumer slot is otherwise optional. -Pipeline references are defaults. A matching step-local or binding-local -external path overrides a pipeline default. Required slots must be bound after -these configuration values and any CLI reference overrides are applied. -Reference paths in YAML are resolved relative to the configuration file. +Pipeline references are configuration defaults. A matching step-local or +binding-local external path overrides a pipeline default. CLI reference +bindings are then operational overrides of configured external paths; their +pipeline, lane, and binding scopes and precedence are defined by the +[CLI reference](cli.md#reference-selectors). A CLI file reference cannot +replace a configured generated artifact handoff. Required slots must be bound +after configuration and CLI reference actions are applied. Reference paths in +YAML are resolved relative to the configuration file; CLI reference paths are +resolved relative to the process working directory. ### D&D Reference Slots diff --git a/docs/consumers/dnd-pipeline.md b/docs/consumers/dnd-pipeline.md index 86beedf0..e3fa6e15 100644 --- a/docs/consumers/dnd-pipeline.md +++ b/docs/consumers/dnd-pipeline.md @@ -24,7 +24,9 @@ files. Use absolute paths for service and subprocess deployments. In particular, observe these different resolution rules: - reference paths in YAML are resolved relative to the Notarius configuration - file; and + file; +- reference paths passed with `--reference` are resolved relative to the + Notarius process working directory; and - `promptkit.profile_file` is resolved relative to the Notarius process working directory. @@ -73,9 +75,21 @@ notarius run dnd-session \ --config /absolute/path/to/notarius.yml \ --input /absolute/path/to/transcripts/final.trimmed.json \ --output-dir /absolute/path/to/notarius-output \ + --reference party=/absolute/path/to/references/party.txt \ + --reference players=/absolute/path/to/references/players.txt \ + --reference glossary=/absolute/path/to/references/glossary.txt \ + --reference spell_catalog=/absolute/path/to/references/spells.json \ --json ``` +Each unqualified reference is pipeline-scoped: Notarius supplies it to every +selected D&D target that declares the slot. A deployment may omit an optional +reference it does not maintain, and may use the lane- or binding-qualified +forms from the [CLI reference](../cli.md#reference-selectors) for an exceptional +override. The registry, scene-description, combat-turn, and NPC-occurrence +references declared between ordered steps in the complete configuration are +generated artifacts. Do not pass those handoffs on the CLI. + The caller should: - capture stdout and stderr separately; diff --git a/docs/consumers/subprocess.md b/docs/consumers/subprocess.md index 70353ec1..6554722f 100644 --- a/docs/consumers/subprocess.md +++ b/docs/consumers/subprocess.md @@ -31,13 +31,36 @@ notarius run pipeline-id \ ``` Use absolute paths for supplied input, configuration, output-root, and -reference files. Notarius generates a stable prompt session for the resolved -input module and exact input bytes. Pass **--session-id** only when intentionally -grouping different invocations under a different session. Supply credentials -through Notarius's documented configuration and environment mechanisms, never -as command-line arguments or generated secret-bearing configuration. In -particular, a session identifier is provider-visible and is not a credential -mechanism. +reference files. Pass each external reference as its own argument-vector pair; +do not construct and invoke a shell command. An unqualified reference selector +supplies that file to every compatible selected target. Lane and stage +qualification are available for exceptional overrides, while generated +same-run references remain part of configured pipeline composition. The +[CLI reference](../cli.md#reference-selectors) owns the exact selector and +precedence contract. + +The maintained D&D subprocess workflow uses this facility for campaign context: + +```sh +notarius run dnd-session \ + --config /absolute/path/to/notarius.yml \ + --input /absolute/path/to/transcripts/final.trimmed.json \ + --output-dir /absolute/path/to/notarius-output \ + --reference party=/absolute/path/to/references/party.txt \ + --reference players=/absolute/path/to/references/players.txt \ + --reference glossary=/absolute/path/to/references/glossary.txt \ + --reference spell_catalog=/absolute/path/to/references/spells.json \ + --json +``` + +Only pass the external references available to and desired by the deployment. +Notarius generates a stable prompt session for the resolved input module and +exact input bytes; reference changes do not change it. Pass **--session-id** +only when intentionally grouping different invocations under a different +session. Supply credentials through Notarius's documented configuration and +environment mechanisms, never as command-line arguments or generated +secret-bearing configuration. In particular, a session identifier is +provider-visible and is not a credential mechanism. Wait for the process before interpreting standard output. Only an exit status of 0 permits decoding the receipt. On a nonzero exit, retain standard error for diff --git a/docs/internal/cli.md b/docs/internal/cli.md index fc79b65b..64196240 100644 --- a/docs/internal/cli.md +++ b/docs/internal/cli.md @@ -115,6 +115,16 @@ to checkpoint identity and `pipeline.RunInput`. The public flag and stability contract are defined by the [CLI reference](../cli.md#run); framework and LLM packages only transport the supplied value. +The CLI also owns the scope grammar for reference flags. It enumerates the +selected chunk and lane targets from registered module specifications, expands +pipeline- and lane-scoped actions into exact stage-and-lane bindings, and +resolves overlapping bind and unbind actions by specificity before calling +configuration resolution. The generic pipeline therefore receives only exact +`ReferenceBinding` and `ReferenceUnbind` values and has no knowledge of CLI +selector syntax. Configuration resolution retains ownership of configured +external/generated conflicts, required slots, and module compatibility; file +materialization still occurs afterward. + For `run --json`, the CLI constructs and encodes its private run-result receipt after a successful runner result is available, before it publishes logical output files. It writes the prepared receipt to standard output only after @@ -166,8 +176,9 @@ is discoverable. - **internal/cli/production_contract_test.go** covers registrar composition, production catalog contents, assets, and representative configuration validation. -- **internal/cli/reference_contract_test.go** covers CLI reference overrides, - origin separation, and materialization boundaries. +- **internal/cli/reference_contract_test.go** covers scoped CLI reference + expansion, specificity, bind/unbind conflicts, generated-reference + protection, origin separation, and materialization boundaries. - **internal/cli/state_hardening_test.go** covers safe run identity, state roots, and failure ordering. diff --git a/docs/internal/pipeline.md b/docs/internal/pipeline.md index 6b53a00b..40f91c57 100644 --- a/docs/internal/pipeline.md +++ b/docs/internal/pipeline.md @@ -97,6 +97,13 @@ codec, checks its complete schema and media identity, and records a content digest plus bounded producer provenance. A missing, ambiguous, invalid, or incompatible producer prevents the consumer step from starting. +Resolution receives only exact stage-and-lane operational reference overrides. +The CLI may offer broader pipeline- or lane-scoped selectors, but expands and +arbitrates those before entering the framework. External overrides are applied +after configured external defaults and local bindings. They cannot coexist +with a generated binding for the same target and slot, and external unbinds do +not remove generated handoffs. + ## Execution And Ordering The runner validates its input, installs no-op state collaborators when none diff --git a/docs/roadmap/cli-pipeline-references.md b/docs/roadmap/cli-pipeline-references.md new file mode 100644 index 00000000..1510fda1 --- /dev/null +++ b/docs/roadmap/cli-pipeline-references.md @@ -0,0 +1,258 @@ +# Pipeline-Scoped CLI References + +## Purpose + +Make command-line reference binding match the pipeline-level mental model used +by configuration and by subprocess callers. A caller should be able to supply +each shared external reference once, while retaining explicit syntax for the +less common case in which one lane or one stage binding needs a different +source. + +## Motivation And Current Problem + +Notarius already accepts repeatable `--reference selector=path` and +`--without-reference selector` flags. The current unqualified form, +`--reference slot=path`, succeeds only when exactly one selected chunk, extract, +merge, or normalize target declares that slot. If several targets consume a +shared reference such as `party`, `glossary`, or `spell_catalog`, the command is +rejected as ambiguous and the caller must repeat stage-qualified bindings. + +That behavior is internally precise but does not match the public configuration +model. A reference declared at pipeline scope is shared with every compatible +target, while target-local configuration provides the exceptional override. +It is therefore surprising for the least-qualified CLI syntax to mean "find one +unique target" rather than "supply this reference to the pipeline." + +The mismatch is especially costly for subprocess use. The primary complete D&D +workflow expects an orchestrator to provide one transcript, one output root, +and a small collection of campaign reference files. Requiring the orchestrator +to know and enumerate every internal consumer of those shared files couples it +to lane composition, makes commands needlessly long, and creates maintenance +work whenever another compatible D&D module is added. + +## Target CLI Contract + +`--reference` remains repeatable, but selector qualification expresses scope: + +| Form | Target scope | +| --- | --- | +| `slot=path` | Every selected pipeline target that declares `slot`. | +| `chunk.slot=path` | The selected chunk binding. | +| `lane.slot=path` | Every extract, merge, or normalize binding in `lane` that declares `slot`. | +| `lane.extract.slot=path` | The extract binding in `lane`. | +| `lane.merge.slot=path` | The merge binding in `lane`. | +| `lane.normalize.slot=path` | The normalize binding in `lane`. | + +The current stage-wide `merge.slot=path` shorthand is removed. A merge-specific +override must name its lane as `lane.merge.slot=path`; this keeps the grammar +hierarchical and avoids another uniqueness-dependent selector. + +`--without-reference` uses the same selector forms without `=path`. It removes +matching external bindings only. It never removes a generated artifact handoff, +and resolution continues to reject a missing required reference. + +Broad selectors are expected to match multiple compatible targets. They fail +when they match no selected target, when the named lane is not selected, or +when a stage-qualified target does not declare the slot. Errors should identify +the selector and relevant scope without requiring callers to understand private +resolver structures. + +## Precedence And Conflict Policy + +Command-line references are operational overrides and take precedence over +external file references supplied at pipeline, step, lane, or binding scope in +configuration. Among CLI selectors that affect the same concrete target: + +1. an exact `lane.stage.slot` or `chunk.slot` selector wins over a lane-scoped + selector; +2. a lane-scoped selector wins over a pipeline-scoped selector; and +3. the last occurrence wins among selectors with equal scope and action. + +This ordering lets an orchestrator provide shared defaults once and express +only genuine exceptions: + +```sh +--reference party=/refs/party.txt \ +--reference npc-registry.party=/refs/npc-party-context.txt +``` + +Binding and unbinding the same concrete target at the same specificity is a +configuration error rather than an argument-order-dependent result. A +more-specific unbind may carve an exception out of a broader binding, and a +more-specific binding may restore an exception to a broader unbind. + +Generated artifact references remain a distinct source form. A CLI file +reference must not silently replace, remove, or coexist with a generated +handoff for the same concrete target and slot. Resolution fails with a +target-specific conflict and directs the caller to narrow or remove the CLI +selector. Same-run generated registry and eligibility handoffs in the complete +D&D pipeline therefore remain controlled by pipeline composition. + +CLI reference paths continue to resolve relative to the process working +directory and retain CLI provenance. Subprocess guidance must recommend +absolute paths. The reference source is auxiliary context and does not affect +the generated prompt session identifier. + +## Subprocess Target State + +Passing shared references is a first-class part of the documented subprocess +workflow. The complete D&D invocation should have this shape: + +```sh +notarius run dnd-session \ + --config /absolute/path/to/notarius.yml \ + --input /absolute/path/to/transcripts/final.trimmed.json \ + --output-dir /absolute/path/to/notarius-output \ + --reference party=/absolute/path/to/references/party.txt \ + --reference players=/absolute/path/to/references/players.txt \ + --reference glossary=/absolute/path/to/references/glossary.txt \ + --reference spell_catalog=/absolute/path/to/references/spells.json \ + --json +``` + +Only references actually available to and desired by the deployment need to be +passed. The selected modules continue to determine accepted slot names, media +types, requiredness, and size limits. The orchestrator passes references as +separate argument-vector elements rather than building a shell command, captures +standard output and standard error separately, checks exit status, and uses the +JSON receipt's `output_directory` to locate the generated run bundle. + +The documentation must clearly distinguish: + +- the input file supplied by `--input`; +- the output *root* supplied by `--output-dir` and the run-specific output path + returned in the receipt; +- external file references supplied by `--reference`; and +- generated references produced and consumed within the configured ordered + pipeline. + +## Required Implementation Changes + +### CLI selector model and parsing + +- Replace the unique-target interpretation of unqualified and lane-qualified + selectors with explicit pipeline, lane, chunk, and exact-binding scopes. +- Remove parsing and help text for the stage-wide `merge.slot` form. +- Preserve repeatable flag handling, non-empty selector/path checks, supported + stage names, and syntax-error exit classification. +- Represent selector scope explicitly enough that precedence and diagnostics do + not depend on inferring intent from empty fields. + +### Target expansion and precedence + +- Refactor CLI reference resolution to return every compatible selected target + for a broad selector rather than requiring uniqueness. +- Resolve overlapping bind and unbind requests into one deterministic action + per concrete target and slot using the precedence policy above. +- Produce exact `pipeline.ReferenceBinding` and `pipeline.ReferenceUnbind` + values after CLI scoping is resolved. Keep broad-selector policy at the CLI + boundary rather than adding CLI grammar or D&D knowledge to the generic + pipeline framework. +- Preserve CLI provenance and the existing rule that command-line paths are + materialized relative to the working directory. +- Detect generated-reference conflicts before file access or stage execution + and retain required-slot validation after effective bindings are known. +- Ensure lane selection is honored for compact pipelines. Explicit ordered + pipelines continue to use their complete configured lane set because they do + not support `--only`. + +### Public and internal documentation + +- Rewrite the reference-selector section of `docs/cli.md`, which is the + canonical owner of flag syntax, selector semantics, precedence, and command + errors. +- Update `docs/consumers/subprocess.md` so reference-bearing invocation is part + of the primary subprocess workflow rather than an unillustrated aside. +- Update `docs/consumers/dnd-pipeline.md` with the complete D&D subprocess + invocation and explain which references are external versus generated. +- Lightly update `docs/config.md` to link the configuration hierarchy to the + CLI override contract without duplicating CLI syntax. +- Update `docs/internal/cli.md` and, only where necessary, the reference + resolution discussion in `docs/internal/pipeline.md` to describe the + implemented expansion and precedence boundary. +- Do not expand the README quickstart or duplicate complete configuration + files in prose. The maintained example configuration remains the canonical + copyable pipeline definition. + +No ADR is required. This is a deliberate public CLI usability correction that +fits the existing architectural decisions: configuration remains centralized, +operational overrides remain explicit, generated handoffs remain part of +pipeline composition, and broad CLI syntax is translated into existing exact +framework bindings at the application boundary. The CLI and configuration +references are the durable owners of the resulting current behavior once it is +implemented. + +## Testing And Validation + +Add lean offline behavioral coverage at the CLI contract boundary for: + +- one pipeline-scoped reference reaching multiple compatible chunk and lane + targets; +- one lane-scoped reference reaching both extract and normalize bindings; +- exact-binding and lane-scoped exceptions overriding broader values; +- final-occurrence behavior at equal specificity; +- broad selectors with no matches and selectors naming unselected lanes or + undeclared slots; +- bind/unbind specificity and same-specificity conflicts; +- rejection of external/generated conflicts and protection of required + generated handoffs; +- working-directory resolution and CLI provenance for expanded bindings; and +- removal of the `merge.slot` shorthand. + +Prefer package-level resolution and representative `RunWithOptions` tests over +duplicating the full selector matrix through end-to-end fixtures. Existing +tests that encode unique-target ambiguity should be rewritten or removed rather +than retained as historical change detectors. Do not assert complete error +strings when stable classification and a concise semantic fragment provide +sufficient confidence. + +Validation for the completed change should include: + +```sh +go test ./internal/cli ./internal/core/config ./internal/framework/pipeline +go test ./... +go vet ./... +go build ./cmd/notarius +``` + +Also validate both maintained example configurations against their selected +pipelines, verify every changed documentation link, and manually review the +D&D subprocess command against the actual flag parser and complete example +configuration. + +## Non-Goals + +- Passing reference contents directly in command-line arguments or environment + variables. +- Adding a reference-manifest file, glob syntax, wildcard selectors, or a + second pipeline-reference flag. +- Allowing the CLI to define undeclared reference slots or bypass module media + type, size, UTF-8, requiredness, or materialization validation. +- Replacing generated same-run artifacts with external files or changing + ordered pipeline dependencies. +- Changing configuration-file reference path resolution, prompt-session + derivation, output directory allocation, or the run-result receipt. +- Adding D&D-specific selector behavior to generic CLI or pipeline packages. +- Preserving uniqueness-dependent `slot` or `lane.slot` behavior as an alias. + +## Acceptance Criteria + +- A subprocess caller can supply `party`, `players`, `glossary`, and + `spell_catalog` once each and have every compatible selected D&D target + receive the corresponding external file. +- `slot=path` has pipeline scope and never fails merely because several selected + targets declare the slot. +- Lane and exact-binding selectors provide deterministic, documented + exceptions with specificity-based precedence. +- A broad selector that matches nothing is rejected before source parsing or + LLM work. +- CLI external references override configured external paths but never silently + replace or remove generated artifact references. +- All effective CLI bindings retain correct provenance and absolute-path + materialization behavior. +- `docs/cli.md` owns the exact public syntax, and both subprocess guides present + shared CLI references as the primary orchestration workflow. +- Current behavior outside `docs/roadmap/` is not documented until the code + lands, and no complete example is duplicated outside `examples/`. +- Focused tests, repository-wide tests, vetting, build, maintained configuration + validation, and documentation-link review pass. diff --git a/internal/cli/reference_contract_test.go b/internal/cli/reference_contract_test.go index 9d41e405..b7373f7e 100644 --- a/internal/cli/reference_contract_test.go +++ b/internal/cli/reference_contract_test.go @@ -4,6 +4,7 @@ import ( "bytes" "os" "path/filepath" + "slices" "strings" "testing" @@ -14,20 +15,16 @@ import ( func TestReferenceSelectorsParseAndApplyAllDocumentedForms(t *testing.T) { tests := []struct { - name string - selector string - only []string - wantStage pipeline.ModuleStage - wantLane string - wantSlot string + name string + selector string + want []string }{ - {name: "flat", selector: "alpha-slot", wantStage: pipeline.StageExtract, wantLane: "alpha", wantSlot: "alpha-slot"}, - {name: "chunk", selector: "chunk.chunk-slot", wantStage: pipeline.StageChunk, wantSlot: "chunk-slot"}, - {name: "merge", selector: "merge.alpha-merge", only: []string{"alpha"}, wantStage: pipeline.StageMerge, wantLane: "alpha", wantSlot: "alpha-merge"}, - {name: "lane", selector: "alpha.alpha-slot", wantStage: pipeline.StageExtract, wantLane: "alpha", wantSlot: "alpha-slot"}, - {name: "lane extract", selector: "alpha.extract.alpha-slot", wantStage: pipeline.StageExtract, wantLane: "alpha", wantSlot: "alpha-slot"}, - {name: "lane merge", selector: "alpha.merge.alpha-merge", wantStage: pipeline.StageMerge, wantLane: "alpha", wantSlot: "alpha-merge"}, - {name: "lane normalize", selector: "alpha.normalize.alpha-normalize", wantStage: pipeline.StageNormalize, wantLane: "alpha", wantSlot: "alpha-normalize"}, + {name: "pipeline", selector: "shared", want: []string{"alpha.extract.shared", "alpha.merge.shared", "alpha.normalize.shared", "beta.extract.shared", "beta.merge.shared", "beta.normalize.shared"}}, + {name: "chunk", selector: "chunk.chunk-slot", want: []string{"chunk.chunk-slot"}}, + {name: "lane", selector: "alpha.shared", want: []string{"alpha.extract.shared", "alpha.merge.shared", "alpha.normalize.shared"}}, + {name: "lane extract", selector: "alpha.extract.alpha-slot", want: []string{"alpha.extract.alpha-slot"}}, + {name: "lane merge", selector: "alpha.merge.alpha-merge", want: []string{"alpha.merge.alpha-merge"}}, + {name: "lane normalize", selector: "alpha.normalize.alpha-normalize", want: []string{"alpha.normalize.alpha-normalize"}}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -37,70 +34,135 @@ func TestReferenceSelectorsParseAndApplyAllDocumentedForms(t *testing.T) { if err != nil { t.Fatal(err) } - overrides, _, err := resolveCLIReferenceRequests(cfg, "demo", tt.only, catalog, []cliReferenceRequest{{Selector: selector, Source: "reference.txt"}}, nil) + overrides, _, err := resolveCLIReferenceRequests(cfg, "demo", nil, catalog, []cliReferenceRequest{{Selector: selector, Source: "reference.txt"}}, nil) if err != nil { t.Fatalf("resolve selector: %v", err) } - if len(overrides) != 1 { - t.Fatalf("overrides = %#v, want one binding", overrides) + if got := referenceContractBindingLabels(overrides); !slices.Equal(got, tt.want) { + t.Fatalf("binding targets = %#v, want %#v", got, tt.want) } - got := overrides[0] - if got.Stage != tt.wantStage || got.LaneID != tt.wantLane || got.SlotName != tt.wantSlot || got.BindingSource != contracts.ReferenceBindingSourceCLI { - t.Fatalf("binding = %#v, want %s/%s/%s from CLI", got, tt.wantStage, tt.wantLane, tt.wantSlot) - } - }) - } -} - -func TestReferenceSelectorsRejectAmbiguityWithSpecificSuggestions(t *testing.T) { - cfg := referenceContractConfig() - catalog := referenceContractCatalog(t, true, true) - for _, tt := range []struct { - name string - selector string - want []string - }{ - {name: "flat shared slot", selector: "shared", want: []string{"alpha.extract.shared", "beta.extract.shared"}}, - {name: "lane shared slot", selector: "alpha.shared", want: []string{"alpha.extract.shared", "alpha.merge.shared", "alpha.normalize.shared"}}, - {name: "all mergers", selector: "merge.shared", want: []string{"alpha.merge.shared", "beta.merge.shared"}}, - } { - t.Run(tt.name, func(t *testing.T) { - selector, err := parseReferenceSelector(tt.selector, "--reference") - if err != nil { - t.Fatal(err) - } - _, _, err = resolveCLIReferenceRequests(cfg, "demo", nil, catalog, []cliReferenceRequest{{Selector: selector, Source: "reference.txt"}}, nil) - if err == nil { - t.Fatal("resolve selector succeeded, want ambiguity error") - } - for _, fragment := range tt.want { - if !strings.Contains(err.Error(), fragment) { - t.Fatalf("error = %q, want suggestion %q", err, fragment) + for _, binding := range overrides { + if binding.Source != "reference.txt" || binding.BindingSource != contracts.ReferenceBindingSourceCLI { + t.Fatalf("binding = %#v, want CLI source", binding) } } }) } } -func TestReferenceSelectorsRespectSelectedLanesBeforeMaterialization(t *testing.T) { +func TestReferenceSelectorSpecificityAndFinalOccurrenceChooseConcreteBindings(t *testing.T) { + cfg := referenceContractConfig() + catalog := referenceContractCatalog(t, true, true) + requests := []cliReferenceRequest{ + {Selector: mustParseReferenceSelector(t, "shared", "--reference"), Source: "pipeline-first.txt"}, + {Selector: mustParseReferenceSelector(t, "shared", "--reference"), Source: "pipeline-final.txt"}, + {Selector: mustParseReferenceSelector(t, "alpha.shared", "--reference"), Source: "lane.txt"}, + {Selector: mustParseReferenceSelector(t, "alpha.extract.shared", "--reference"), Source: "binding.txt"}, + } + overrides, unbinds, err := resolveCLIReferenceRequests(cfg, "demo", nil, catalog, requests, nil) + if err != nil { + t.Fatal(err) + } + if len(unbinds) != 0 { + t.Fatalf("unbinds = %#v, want none", unbinds) + } + want := map[string]string{ + "alpha.extract.shared": "binding.txt", + "alpha.merge.shared": "lane.txt", + "alpha.normalize.shared": "lane.txt", + "beta.extract.shared": "pipeline-final.txt", + "beta.merge.shared": "pipeline-final.txt", + "beta.normalize.shared": "pipeline-final.txt", + } + for _, binding := range overrides { + label := referenceContractBindingLabel(binding) + if binding.Source != want[label] { + t.Fatalf("binding %s source = %q, want %q", label, binding.Source, want[label]) + } + delete(want, label) + } + if len(want) != 0 { + t.Fatalf("missing bindings: %#v", want) + } +} + +func TestCompleteDNDSharedCLIReferencesExpandAcrossCompatibleTargets(t *testing.T) { + cfg := loadMaintainedExample(t, repositoryPath("examples", "dnd-complete.config.yml")) + catalog := catalogFromRegistries(productionTestComponents(t).registries) + sources := map[string]string{ + "party": "/references/party.txt", + "players": "/references/players.txt", + "glossary": "/references/glossary.txt", + "spell_catalog": "/references/spells.json", + } + requests := make([]cliReferenceRequest, 0, len(sources)) + for _, slot := range []string{"party", "players", "glossary", "spell_catalog"} { + requests = append(requests, cliReferenceRequest{ + Selector: mustParseReferenceSelector(t, slot, "--reference"), + Source: sources[slot], + }) + } + overrides, unbinds, err := resolveCLIReferenceRequests(cfg, "dnd-session", nil, catalog, requests, nil) + if err != nil { + t.Fatalf("expand complete D&D references: %v", err) + } + if len(unbinds) != 0 { + t.Fatalf("unbinds = %#v, want none", unbinds) + } + + actual := make(map[string]pipeline.ReferenceBinding, len(overrides)) + for _, binding := range overrides { + actual[referenceContractBindingLabel(binding)] = binding + } + targets, err := selectedReferenceTargets(cfg, "dnd-session", nil, catalog) + if err != nil { + t.Fatal(err) + } + matched := make(map[string]int, len(sources)) + for _, target := range targets { + for slot, sourcePath := range sources { + if _, ok := target.slots[slot]; !ok { + continue + } + matched[slot]++ + label := targetLabel(target) + "." + slot + binding, ok := actual[label] + if !ok || binding.Source != sourcePath || binding.BindingSource != contracts.ReferenceBindingSourceCLI { + t.Fatalf("binding %q = %#v, want CLI source %q", label, binding, sourcePath) + } + } + } + for slot := range sources { + if matched[slot] < 2 { + t.Fatalf("reference %q matched %d target(s), want a shared D&D reference", slot, matched[slot]) + } + } + if _, err := cfg.Resolve(config.ResolveInput{PipelineID: "dnd-session", Catalog: catalog, ReferenceOverrides: overrides}); err != nil { + t.Fatalf("resolve complete D&D CLI references: %v", err) + } +} + +func TestReferenceSelectorsRejectInvalidOrUnselectedScopesBeforeMaterialization(t *testing.T) { cfg := referenceContractConfig() catalog := referenceContractCatalog(t, true, true) for _, tt := range []struct { name string selector string + only []string want string }{ - {name: "unselected lane", selector: "beta.extract.beta-slot", want: `reference lane "beta" is not selected`}, + {name: "pipeline slot", selector: "missing", want: `reference slot "missing" is not declared by any selected target`}, + {name: "lane slot", selector: "alpha.missing", want: `reference slot "missing" is not declared by selected lane "alpha"`}, + {name: "binding slot", selector: "alpha.extract.missing", want: `reference slot "missing" is not declared`}, + {name: "former merge shorthand", selector: "merge.shared", want: `reference lane "merge" is not selected`}, + {name: "unselected lane", selector: "beta.extract.beta-slot", only: []string{"alpha"}, want: `reference lane "beta" is not selected`}, {name: "unknown lane", selector: "missing.extract.beta-slot", want: `reference lane "missing" is not selected`}, } { t.Run(tt.name, func(t *testing.T) { - selector, err := parseReferenceSelector(tt.selector, "--reference") - if err != nil { - t.Fatal(err) - } - _, _, err = resolveCLIReferenceRequests(cfg, "demo", []string{"alpha"}, catalog, []cliReferenceRequest{{Selector: selector, Source: filepath.Join(t.TempDir(), "missing.txt")}}, nil) + selector := mustParseReferenceSelector(t, tt.selector, "--reference") + _, _, err := resolveCLIReferenceRequests(cfg, "demo", tt.only, catalog, []cliReferenceRequest{{Selector: selector, Source: filepath.Join(t.TempDir(), "missing.txt")}}, nil) if err == nil || !strings.Contains(err.Error(), tt.want) || strings.Contains(err.Error(), "missing.txt") { - t.Fatalf("error = %v, want selection failure before file access", err) + t.Fatalf("error = %v, want selection failure containing %q before file access", err, tt.want) } }) } @@ -131,40 +193,50 @@ func TestReferenceSyntaxErrorsReturnTwo(t *testing.T) { } } -func TestReferenceOverridesUseFinalExactTargetBinding(t *testing.T) { +func TestReferenceBindAndUnbindSpecificity(t *testing.T) { cfg := referenceContractConfig() catalog := referenceContractCatalog(t, true, true) - alphaShared, err := parseReferenceSelector("alpha.extract.shared", "--reference") - if err != nil { - t.Fatal(err) - } - betaShared, err := parseReferenceSelector("beta.extract.shared", "--reference") - if err != nil { - t.Fatal(err) - } - overrides, unbinds, err := resolveCLIReferenceRequests(cfg, "demo", nil, catalog, []cliReferenceRequest{ - {Selector: alphaShared, Source: "alpha-first.txt"}, - {Selector: alphaShared, Source: "alpha-final.txt"}, - {Selector: betaShared, Source: "beta-only.txt"}, - }, nil) - if err != nil { - t.Fatal(err) - } - if len(unbinds) != 0 { - t.Fatalf("unbinds = %#v, want none", unbinds) - } - effective, err := cfg.Resolve(config.ResolveInput{PipelineID: "demo", Catalog: catalog, ReferenceOverrides: overrides}) - if err != nil { - t.Fatalf("resolve pipeline: %v", err) - } - alpha := referenceContractLane(t, effective.ResolvedPipeline, "alpha") - beta := referenceContractLane(t, effective.ResolvedPipeline, "beta") - if source := referenceContractBindingSource(alpha.ExtractReferences.Bindings, "shared"); source != "alpha-final.txt" { - t.Fatalf("alpha shared source = %q, want final exact-target override", source) - } - if source := referenceContractBindingSource(beta.ExtractReferences.Bindings, "shared"); source != "beta-only.txt" { - t.Fatalf("beta shared source = %q, want target-specific override", source) - } + t.Run("specific unbind carves out broad binding", func(t *testing.T) { + overrides, unbinds, err := resolveCLIReferenceRequests(cfg, "demo", nil, catalog, + []cliReferenceRequest{{Selector: mustParseReferenceSelector(t, "shared", "--reference"), Source: "shared.txt"}}, + []cliReferenceUnbindRequest{{Selector: mustParseReferenceSelector(t, "alpha.extract.shared", "--without-reference")}}, + ) + if err != nil { + t.Fatal(err) + } + if got := referenceContractBindingLabels(overrides); slices.Contains(got, "alpha.extract.shared") || len(got) != 5 { + t.Fatalf("overrides = %#v, want all shared targets except alpha extract", got) + } + if got := referenceContractUnbindLabels(unbinds); !slices.Equal(got, []string{"alpha.extract.shared"}) { + t.Fatalf("unbinds = %#v, want alpha extract", got) + } + }) + + t.Run("specific binding restores broad unbind", func(t *testing.T) { + overrides, unbinds, err := resolveCLIReferenceRequests(cfg, "demo", nil, catalog, + []cliReferenceRequest{{Selector: mustParseReferenceSelector(t, "alpha.extract.shared", "--reference"), Source: "alpha.txt"}}, + []cliReferenceUnbindRequest{{Selector: mustParseReferenceSelector(t, "shared", "--without-reference")}}, + ) + if err != nil { + t.Fatal(err) + } + if got := referenceContractBindingLabels(overrides); !slices.Equal(got, []string{"alpha.extract.shared"}) { + t.Fatalf("overrides = %#v, want alpha extract", got) + } + if got := referenceContractUnbindLabels(unbinds); slices.Contains(got, "alpha.extract.shared") || len(got) != 5 { + t.Fatalf("unbinds = %#v, want all shared targets except alpha extract", got) + } + }) + + t.Run("same specificity conflicts", func(t *testing.T) { + _, _, err := resolveCLIReferenceRequests(cfg, "demo", nil, catalog, + []cliReferenceRequest{{Selector: mustParseReferenceSelector(t, "alpha.shared", "--reference"), Source: "alpha.txt"}}, + []cliReferenceUnbindRequest{{Selector: mustParseReferenceSelector(t, "alpha.shared", "--without-reference")}}, + ) + if err == nil || !strings.Contains(err.Error(), "same specificity") { + t.Fatalf("error = %v, want same-specificity conflict", err) + } + }) } func TestReferenceUnbindsRemoveOptionalAndProtectRequiredSlots(t *testing.T) { @@ -257,6 +329,52 @@ func TestReferenceMaterializationSeparatesCLIAndConfigPathOrigins(t *testing.T) } } +func TestPipelineScopedCLIReferenceProtectsGeneratedHandoff(t *testing.T) { + cfg := referenceContractConfig() + profile := cfg.Pipelines["demo"] + alpha := profile.Artifacts["alpha"] + beta := profile.Artifacts["beta"] + alpha.Extract.References["shared"] = pipeline.GeneratedReference("produce", "beta") + profile.Artifacts = nil + profile.Steps = []pipeline.PipelineStepProfile{ + {ID: "produce", Artifacts: map[string]pipeline.ArtifactLaneProfile{"beta": beta}}, + {ID: "consume", Artifacts: map[string]pipeline.ArtifactLaneProfile{"alpha": alpha}}, + } + cfg.Pipelines["demo"] = profile + catalog := referenceContractCatalog(t, true, true) + + t.Run("binding conflicts before file access", func(t *testing.T) { + overrides, _, err := resolveCLIReferenceRequests(cfg, "demo", nil, catalog, []cliReferenceRequest{{ + Selector: mustParseReferenceSelector(t, "shared", "--reference"), + Source: filepath.Join(t.TempDir(), "never-read.json"), + }}, nil) + if err != nil { + t.Fatalf("expand CLI reference: %v", err) + } + _, err = cfg.Resolve(config.ResolveInput{PipelineID: "demo", Catalog: catalog, ReferenceOverrides: overrides}) + if err == nil || !strings.Contains(err.Error(), "conflicting generated and external bindings") || strings.Contains(err.Error(), "never-read.json") { + t.Fatalf("resolve error = %v, want generated/external conflict before file access", err) + } + }) + + t.Run("unbind leaves generated source intact", func(t *testing.T) { + _, unbinds, err := resolveCLIReferenceRequests(cfg, "demo", nil, catalog, nil, []cliReferenceUnbindRequest{{ + Selector: mustParseReferenceSelector(t, "shared", "--without-reference"), + }}) + if err != nil { + t.Fatalf("expand CLI unbind: %v", err) + } + effective, err := cfg.Resolve(config.ResolveInput{PipelineID: "demo", Catalog: catalog, ReferenceUnbinds: unbinds}) + if err != nil { + t.Fatalf("resolve generated reference with CLI unbind: %v", err) + } + binding := referenceContractFindBinding(referenceContractLane(t, effective.ResolvedPipeline, "alpha").ExtractReferences.Bindings, "shared") + if binding == nil || binding.Artifact == nil || binding.Artifact.Step != "produce" || binding.Artifact.Lane != "beta" { + t.Fatalf("generated binding = %#v, want preserved produce/beta handoff", binding) + } + }) +} + func TestReferenceTargetLookupUsesArtifactVariantsAndReportsMissingContext(t *testing.T) { cfg := referenceContractConfig() full := referenceContractCatalog(t, true, true) @@ -368,7 +486,7 @@ func referenceContractCatalog(t *testing.T, includeBetaMerger, includeBetaNormal register(registries.Chunkers.RegisterWithSpec(pipeline.ModuleSpec{Key: "reference/chunk", Stage: pipeline.StageChunk, ExecutionClass: contracts.ExecutionClassDeterministic, Requires: []string{"source"}, Provides: []string{"chunks"}, ReferenceSlots: []contracts.ReferenceSlot{{Name: "chunk-slot"}, {Name: "required-chunk", Required: true}}}, func() (contracts.Chunker, error) { return stateTestChunker{}, nil })) register(pipeline.RegisterArtifactCodec(registries.ArtifactCodecs, referenceContractCodecA{})) register(pipeline.RegisterArtifactCodec(registries.ArtifactCodecs, referenceContractCodecB{})) - register(pipeline.RegisterExtractor(registries.Extractors, pipeline.ModuleSpec{Key: "reference/extract-alpha", Stage: pipeline.StageExtract, ExecutionClass: contracts.ExecutionClassDeterministic, Requires: []string{"chunks"}, Provides: []string{"artifact"}, ArtifactKind: referenceContractKindAlpha, ReferenceSlots: []contracts.ReferenceSlot{{Name: "shared"}, {Name: "alpha-slot"}, {Name: "required-extract", Required: true}}}, func() (contracts.Extractor[stateTestArtifact], error) { return stateTestExtractor{}, nil })) + register(pipeline.RegisterExtractor(registries.Extractors, pipeline.ModuleSpec{Key: "reference/extract-alpha", Stage: pipeline.StageExtract, ExecutionClass: contracts.ExecutionClassDeterministic, Requires: []string{"chunks"}, Provides: []string{"artifact"}, ArtifactKind: referenceContractKindAlpha, ReferenceSlots: []contracts.ReferenceSlot{{Name: "shared", AcceptedArtifactKinds: []contracts.ArtifactKind{referenceContractKindBeta}, AcceptedMediaTypes: []string{"application/json"}}, {Name: "alpha-slot"}, {Name: "required-extract", Required: true}}}, func() (contracts.Extractor[stateTestArtifact], error) { return stateTestExtractor{}, nil })) register(pipeline.RegisterExtractor(registries.Extractors, pipeline.ModuleSpec{Key: "reference/extract-beta", Stage: pipeline.StageExtract, ExecutionClass: contracts.ExecutionClassDeterministic, Requires: []string{"chunks"}, Provides: []string{"artifact"}, ArtifactKind: referenceContractKindBeta, ReferenceSlots: []contracts.ReferenceSlot{{Name: "shared"}, {Name: "beta-slot"}, {Name: "required-extract", Required: true}}}, func() (contracts.Extractor[stateTestArtifact], error) { return stateTestExtractor{}, nil })) register(pipeline.RegisterMerger(registries.Mergers, pipeline.ModuleSpec{Key: "reference/shared-merge", Stage: pipeline.StageMerge, ExecutionClass: contracts.ExecutionClassDeterministic, Requires: []string{"artifact"}, Provides: []string{"merged"}, ArtifactKind: referenceContractKindAlpha, ReferenceSlots: []contracts.ReferenceSlot{{Name: "shared"}, {Name: "alpha-merge"}, {Name: "required-merge", Required: true}}}, func() (contracts.Merger[stateTestArtifact], error) { return stateTestMerger{}, nil })) if includeBetaMerger { @@ -444,6 +562,42 @@ func referenceContractBindingSource(bindings []pipeline.ReferenceBinding, slot s return "" } +func mustParseReferenceSelector(t *testing.T, value, flagName string) cliReferenceSelector { + t.Helper() + selector, err := parseReferenceSelector(value, flagName) + if err != nil { + t.Fatal(err) + } + return selector +} + +func referenceContractBindingLabels(bindings []pipeline.ReferenceBinding) []string { + labels := make([]string, 0, len(bindings)) + for _, binding := range bindings { + labels = append(labels, referenceContractBindingLabel(binding)) + } + return labels +} + +func referenceContractBindingLabel(binding pipeline.ReferenceBinding) string { + if binding.Stage == pipeline.StageChunk { + return "chunk." + binding.SlotName + } + return binding.LaneID + "." + string(binding.Stage) + "." + binding.SlotName +} + +func referenceContractUnbindLabels(unbinds []pipeline.ReferenceUnbind) []string { + labels := make([]string, 0, len(unbinds)) + for _, unbind := range unbinds { + labels = append(labels, referenceContractBindingLabel(pipeline.ReferenceBinding{ + Stage: unbind.Stage, + LaneID: unbind.LaneID, + SlotName: unbind.SlotName, + })) + } + return labels +} + func referenceContractFindBinding(bindings []pipeline.ReferenceBinding, slot string) *pipeline.ReferenceBinding { for i := range bindings { if bindings[i].SlotName == slot { diff --git a/internal/cli/run.go b/internal/cli/run.go index 8b87889e..e3bde36a 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -177,7 +177,7 @@ func runPipelineCommand(args []string, stdout, stderr io.Writer, opts Options) i fs.Var(&llmProfile, "llm-profile", "LLM profile override") fs.Var(&reasoningEffort, "reasoning-effort", "reasoning effort override") fs.Var(&chunkCache, "chunk_cache", "chunk plan cache mode: auto, bypass, or refresh") - fs.Var(&referenceFlags, "reference", "reference binding, as slot=path, chunk.slot=path, merge.slot=path, lane.slot=path, lane.extract.slot=path, lane.merge.slot=path, or lane.normalize.slot=path") + fs.Var(&referenceFlags, "reference", "reference binding, as slot=path, chunk.slot=path, lane.slot=path, lane.extract.slot=path, lane.merge.slot=path, or lane.normalize.slot=path") fs.Var(&withoutReferenceFlags, "without-reference", "unbind a reference, using the same selector forms as --reference") fs.Var(&recomputeStep, "recompute-step", "recompute one ordered pipeline step and dependent lanes") if err := validateRunFlagValues(args); err != nil { @@ -1287,11 +1287,21 @@ type cliReferenceUnbindRequest struct { } type cliReferenceSelector struct { + Scope cliReferenceSelectorScope LaneID string Stage pipeline.ModuleStage SlotName string } +type cliReferenceSelectorScope uint8 + +const ( + cliReferenceScopePipeline cliReferenceSelectorScope = iota + cliReferenceScopeLane + cliReferenceScopeChunk + cliReferenceScopeBinding +) + func parseReferenceFlags(values []string) ([]cliReferenceRequest, error) { if len(values) == 0 { return nil, nil @@ -1300,7 +1310,7 @@ func parseReferenceFlags(values []string) ([]cliReferenceRequest, error) { for _, raw := range values { name, source, ok := strings.Cut(raw, "=") if !ok { - return nil, fmt.Errorf("--reference must use slot=path or lane.slot=path") + return nil, fmt.Errorf("--reference must use slot=path, lane.slot=path, or lane.stage.slot=path") } if strings.TrimSpace(source) == "" { return nil, fmt.Errorf("--reference path must not be empty; use --without-reference to unbind") @@ -1350,17 +1360,14 @@ func parseReferenceSelector(raw string, flagName string) (cliReferenceSelector, } switch len(parts) { case 1: - return cliReferenceSelector{SlotName: strings.TrimSpace(parts[0])}, nil + return cliReferenceSelector{Scope: cliReferenceScopePipeline, SlotName: strings.TrimSpace(parts[0])}, nil case 2: first := strings.TrimSpace(parts[0]) slotName := strings.TrimSpace(parts[1]) if first == string(pipeline.StageChunk) { - return cliReferenceSelector{Stage: pipeline.StageChunk, SlotName: slotName}, nil + return cliReferenceSelector{Scope: cliReferenceScopeChunk, Stage: pipeline.StageChunk, SlotName: slotName}, nil } - if first == string(pipeline.StageMerge) { - return cliReferenceSelector{Stage: pipeline.StageMerge, SlotName: slotName}, nil - } - return cliReferenceSelector{LaneID: first, SlotName: slotName}, nil + return cliReferenceSelector{Scope: cliReferenceScopeLane, LaneID: first, SlotName: slotName}, nil case 3: laneID := strings.TrimSpace(parts[0]) stage := pipeline.ModuleStage(strings.TrimSpace(parts[1])) @@ -1368,9 +1375,9 @@ func parseReferenceSelector(raw string, flagName string) (cliReferenceSelector, if stage != pipeline.StageExtract && stage != pipeline.StageMerge && stage != pipeline.StageNormalize { return cliReferenceSelector{}, fmt.Errorf("%s lane-qualified selector must use lane.extract.slot, lane.merge.slot, or lane.normalize.slot", flagName) } - return cliReferenceSelector{LaneID: laneID, Stage: stage, SlotName: slotName}, nil + return cliReferenceSelector{Scope: cliReferenceScopeBinding, LaneID: laneID, Stage: stage, SlotName: slotName}, nil default: - return cliReferenceSelector{}, fmt.Errorf("%s must use slot, chunk.slot, merge.slot, lane.slot, lane.extract.slot, lane.merge.slot, or lane.normalize.slot", flagName) + return cliReferenceSelector{}, fmt.Errorf("%s must use slot, chunk.slot, lane.slot, lane.extract.slot, lane.merge.slot, or lane.normalize.slot", flagName) } } @@ -1391,37 +1398,153 @@ func resolveCLIReferenceRequests( return nil, nil, err } - overrides := make([]pipeline.ReferenceBinding, 0, len(referenceRequests)) + // Broad CLI selectors are only presentation syntax. Collapse them into one + // highest-specificity action per concrete framework target before pipeline + // resolution so the generic reference contract stays stage-and-lane exact. + actions := make(map[cliReferenceTargetKey]resolvedCLIReferenceAction) for _, request := range referenceRequests { - target, err := resolveCLIReferenceTarget(targets, request.Selector) + matches, err := resolveCLIReferenceTargets(targets, request.Selector) if err != nil { return nil, nil, err } - overrides = append(overrides, pipeline.ReferenceBinding{ - Stage: target.stage, - LaneID: target.laneID, - SlotName: request.Selector.SlotName, - Source: request.Source, - BindingSource: contracts.ReferenceBindingSourceCLI, - }) + for _, target := range matches { + candidate := resolvedCLIReferenceAction{ + kind: cliReferenceActionBind, + selector: request.Selector, + target: target, + slotName: request.Selector.SlotName, + source: request.Source, + specificity: request.Selector.specificity(), + } + if err := mergeCLIReferenceAction(actions, candidate); err != nil { + return nil, nil, err + } + } } - - unbinds := make([]pipeline.ReferenceUnbind, 0, len(unbindRequests)) for _, request := range unbindRequests { - target, err := resolveCLIReferenceTarget(targets, request.Selector) + matches, err := resolveCLIReferenceTargets(targets, request.Selector) if err != nil { return nil, nil, err } - unbinds = append(unbinds, pipeline.ReferenceUnbind{ - Stage: target.stage, - LaneID: target.laneID, - SlotName: request.Selector.SlotName, - }) + for _, target := range matches { + candidate := resolvedCLIReferenceAction{ + kind: cliReferenceActionUnbind, + selector: request.Selector, + target: target, + slotName: request.Selector.SlotName, + specificity: request.Selector.specificity(), + } + if err := mergeCLIReferenceAction(actions, candidate); err != nil { + return nil, nil, err + } + } } + resolved := make([]resolvedCLIReferenceAction, 0, len(actions)) + for _, action := range actions { + resolved = append(resolved, action) + } + sort.Slice(resolved, func(i, j int) bool { + left, right := resolved[i], resolved[j] + if left.target.laneID != right.target.laneID { + return left.target.laneID < right.target.laneID + } + if left.target.stage != right.target.stage { + return referenceStageOrder(left.target.stage) < referenceStageOrder(right.target.stage) + } + return left.slotName < right.slotName + }) + + overrides := make([]pipeline.ReferenceBinding, 0, len(resolved)) + unbinds := make([]pipeline.ReferenceUnbind, 0, len(resolved)) + for _, action := range resolved { + switch action.kind { + case cliReferenceActionBind: + overrides = append(overrides, pipeline.ReferenceBinding{ + Stage: action.target.stage, + LaneID: action.target.laneID, + SlotName: action.slotName, + Source: action.source, + BindingSource: contracts.ReferenceBindingSourceCLI, + }) + case cliReferenceActionUnbind: + unbinds = append(unbinds, pipeline.ReferenceUnbind{ + Stage: action.target.stage, + LaneID: action.target.laneID, + SlotName: action.slotName, + }) + } + } return overrides, unbinds, nil } +type cliReferenceActionKind uint8 + +const ( + cliReferenceActionBind cliReferenceActionKind = iota + cliReferenceActionUnbind +) + +type cliReferenceTargetKey struct { + stage pipeline.ModuleStage + laneID string + slotName string +} + +type resolvedCLIReferenceAction struct { + kind cliReferenceActionKind + selector cliReferenceSelector + target selectedReferenceTarget + slotName string + source string + specificity int +} + +func mergeCLIReferenceAction(actions map[cliReferenceTargetKey]resolvedCLIReferenceAction, candidate resolvedCLIReferenceAction) error { + key := cliReferenceTargetKey{stage: candidate.target.stage, laneID: candidate.target.laneID, slotName: candidate.slotName} + current, ok := actions[key] + if !ok || candidate.specificity > current.specificity { + actions[key] = candidate + return nil + } + if candidate.specificity < current.specificity { + return nil + } + if candidate.kind != current.kind { + return fmt.Errorf("reference target %q slot %q is both bound by %q and unbound by %q at the same specificity", targetLabel(candidate.target), candidate.slotName, current.selector.String(), candidate.selector.String()) + } + actions[key] = candidate + return nil +} + +func (selector cliReferenceSelector) specificity() int { + switch selector.Scope { + case cliReferenceScopePipeline: + return 0 + case cliReferenceScopeLane: + return 1 + case cliReferenceScopeChunk, cliReferenceScopeBinding: + return 2 + default: + return -1 + } +} + +func (selector cliReferenceSelector) String() string { + switch selector.Scope { + case cliReferenceScopePipeline: + return selector.SlotName + case cliReferenceScopeLane: + return selector.LaneID + "." + selector.SlotName + case cliReferenceScopeChunk: + return "chunk." + selector.SlotName + case cliReferenceScopeBinding: + return selector.LaneID + "." + string(selector.Stage) + "." + selector.SlotName + default: + return selector.SlotName + } +} + type selectedReferenceTarget struct { laneID string stage pipeline.ModuleStage @@ -1638,114 +1761,77 @@ func referenceSlotSet(slots []contracts.ReferenceSlot) map[string]struct{} { return slotSet } -func resolveCLIReferenceTarget(targets []selectedReferenceTarget, selector cliReferenceSelector) (selectedReferenceTarget, error) { +func resolveCLIReferenceTargets(targets []selectedReferenceTarget, selector cliReferenceSelector) ([]selectedReferenceTarget, error) { slotName := strings.TrimSpace(selector.SlotName) if slotName == "" { - return selectedReferenceTarget{}, fmt.Errorf("reference slot must not be empty") + return nil, fmt.Errorf("reference slot must not be empty") } - if selector.Stage == pipeline.StageChunk { + switch selector.Scope { + case cliReferenceScopePipeline: + matches := make([]selectedReferenceTarget, 0, len(targets)) + for _, target := range targets { + if _, ok := target.slots[slotName]; ok { + matches = append(matches, target) + } + } + if len(matches) == 0 { + return nil, fmt.Errorf("reference slot %q is not declared by any selected target", slotName) + } + return matches, nil + case cliReferenceScopeChunk: for _, target := range targets { if target.stage != pipeline.StageChunk { continue } if _, ok := target.slots[slotName]; !ok { - return selectedReferenceTarget{}, fmt.Errorf("reference slot %q is not declared by chunk module %q", slotName, target.module) + return nil, fmt.Errorf("reference slot %q is not declared by chunk module %q", slotName, target.module) } - return target, nil - } - return selectedReferenceTarget{}, fmt.Errorf("reference chunk target is not selected") - } - if selector.Stage == pipeline.StageExtract || selector.Stage == pipeline.StageMerge || selector.Stage == pipeline.StageNormalize { - if selector.LaneID == "" && selector.Stage == pipeline.StageMerge { - return resolveCLIReferenceStageTarget(targets, selector.Stage, slotName) + return []selectedReferenceTarget{target}, nil } + return nil, fmt.Errorf("reference chunk target is not selected") + case cliReferenceScopeLane: + laneSelected := false + matches := make([]selectedReferenceTarget, 0, 3) for _, target := range targets { - if target.laneID == selector.LaneID && target.stage == selector.Stage { - if _, ok := target.slots[slotName]; !ok { - return selectedReferenceTarget{}, fmt.Errorf("reference slot %q is not declared by selected %s target %q", slotName, selector.Stage, targetLabel(target)) - } - return target, nil + if target.laneID != selector.LaneID { + continue + } + laneSelected = true + if _, ok := target.slots[slotName]; ok { + matches = append(matches, target) } } - return selectedReferenceTarget{}, fmt.Errorf("reference lane %q is not selected", selector.LaneID) - } - if strings.TrimSpace(selector.LaneID) != "" { - return resolveCLIReferenceLaneTarget(targets, strings.TrimSpace(selector.LaneID), slotName) - } - return resolveCLIReferenceFlatTarget(targets, slotName) -} - -func resolveCLIReferenceStageTarget(targets []selectedReferenceTarget, stage pipeline.ModuleStage, slotName string) (selectedReferenceTarget, error) { - matches := make([]selectedReferenceTarget, 0, 2) - for _, target := range targets { - if target.stage != stage { - continue + if !laneSelected { + return nil, fmt.Errorf("reference lane %q is not selected", selector.LaneID) } - if _, ok := target.slots[slotName]; ok { - matches = append(matches, target) + if len(matches) == 0 { + return nil, fmt.Errorf("reference slot %q is not declared by selected lane %q", slotName, selector.LaneID) } - } - switch len(matches) { - case 0: - return selectedReferenceTarget{}, fmt.Errorf("reference slot %q is not declared by any selected %s target", slotName, stage) - case 1: - return matches[0], nil + return matches, nil + case cliReferenceScopeBinding: + laneSelected := false + for _, target := range targets { + if target.laneID != selector.LaneID { + continue + } + laneSelected = true + if target.stage != selector.Stage { + continue + } + if _, ok := target.slots[slotName]; !ok { + return nil, fmt.Errorf("reference slot %q is not declared by selected %s target %q", slotName, selector.Stage, targetLabel(target)) + } + return []selectedReferenceTarget{target}, nil + } + if !laneSelected { + return nil, fmt.Errorf("reference lane %q is not selected", selector.LaneID) + } + return nil, fmt.Errorf("reference %s target is not selected for lane %q", selector.Stage, selector.LaneID) default: - return selectedReferenceTarget{}, fmt.Errorf("reference slot %q is declared by multiple selected %s targets (%s); use a more specific selector such as %s", slotName, stage, targetList(matches), selectorSuggestions(matches, slotName)) + return nil, fmt.Errorf("reference selector has unknown scope") } } -func resolveCLIReferenceLaneTarget(targets []selectedReferenceTarget, laneID string, slotName string) (selectedReferenceTarget, error) { - laneSelected := false - matches := make([]selectedReferenceTarget, 0, 2) - for _, target := range targets { - if target.laneID != laneID { - continue - } - laneSelected = true - if _, ok := target.slots[slotName]; ok { - matches = append(matches, target) - } - } - if !laneSelected { - return selectedReferenceTarget{}, fmt.Errorf("reference lane %q is not selected", laneID) - } - switch len(matches) { - case 0: - return selectedReferenceTarget{}, fmt.Errorf("reference slot %q is not declared by selected lane %q", slotName, laneID) - case 1: - return matches[0], nil - default: - return selectedReferenceTarget{}, fmt.Errorf("reference slot %q is declared by multiple selected targets in lane %q (%s); use a more specific selector such as %s", slotName, laneID, targetList(matches), selectorSuggestions(matches, slotName)) - } -} - -func resolveCLIReferenceFlatTarget(targets []selectedReferenceTarget, slotName string) (selectedReferenceTarget, error) { - matches := make([]selectedReferenceTarget, 0, 2) - for _, target := range targets { - if _, ok := target.slots[slotName]; ok { - matches = append(matches, target) - } - } - switch len(matches) { - case 0: - return selectedReferenceTarget{}, fmt.Errorf("reference slot %q is not declared by any selected reference target", slotName) - case 1: - return matches[0], nil - default: - return selectedReferenceTarget{}, fmt.Errorf("reference slot %q is declared by multiple selected targets (%s); use a more specific selector such as %s", slotName, targetList(matches), selectorSuggestions(matches, slotName)) - } -} - -func targetList(targets []selectedReferenceTarget) string { - labels := make([]string, 0, len(targets)) - for _, target := range targets { - labels = append(labels, targetLabel(target)) - } - sort.Strings(labels) - return strings.Join(labels, ", ") -} - func targetLabel(target selectedReferenceTarget) string { if target.stage == pipeline.StageChunk { return "chunk" @@ -1753,17 +1839,19 @@ func targetLabel(target selectedReferenceTarget) string { return target.laneID + "." + string(target.stage) } -func selectorSuggestions(targets []selectedReferenceTarget, slotName string) string { - suggestions := make([]string, 0, len(targets)) - for _, target := range targets { - if target.stage == pipeline.StageChunk { - suggestions = append(suggestions, "chunk."+slotName) - continue - } - suggestions = append(suggestions, target.laneID+"."+string(target.stage)+"."+slotName) +func referenceStageOrder(stage pipeline.ModuleStage) int { + switch stage { + case pipeline.StageChunk: + return 0 + case pipeline.StageExtract: + return 1 + case pipeline.StageMerge: + return 2 + case pipeline.StageNormalize: + return 3 + default: + return 4 } - sort.Strings(suggestions) - return strings.Join(suggestions, " or ") } func sortedPipelineIDs(cfg config.Config) []string {