Skip to content

Commit 204a879

Browse files
docs(parser): define provider contract edge cases
Provider migration depends on source references being fingerprintable, outcome IDs being comparable to persisted rows, and incremental parsing having unambiguous fallback semantics. Without those rules, a new provider could satisfy the facade shape while diverging in skip-cache, retry, or caller migration behavior. This also records provider concurrency and SourceRef lifetime requirements so helpers can stay plain data holders while engine callers can safely share one provider instance.
1 parent 36ec94f commit 204a879

1 file changed

Lines changed: 133 additions & 27 deletions

File tree

docs/superpowers/specs/2026-06-19-provider-facade-design.md

Lines changed: 133 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ type Provider interface {
9898
ParseIncremental(
9999
context.Context,
100100
IncrementalRequest,
101-
) (IncrementalOutcome, bool, error)
101+
) (IncrementalOutcome, IncrementalStatus, error)
102102
}
103103
```
104104

@@ -124,13 +124,38 @@ type ProviderBase struct {
124124
Caps Capabilities
125125
Config ProviderConfig
126126
}
127+
128+
var ErrUnsupportedProviderFeature = errors.New("unsupported provider feature")
129+
130+
type UnsupportedProviderFeatureError struct {
131+
Provider AgentType
132+
Feature string
133+
}
134+
135+
func (err UnsupportedProviderFeatureError) Error() string {
136+
return string(err.Provider) + ": unsupported provider feature " + err.Feature
137+
}
138+
139+
func (err UnsupportedProviderFeatureError) Unwrap() error {
140+
return ErrUnsupportedProviderFeature
141+
}
127142
```
128143

129144
`ProviderBase` provides `Definition`, `Capabilities`, empty discovery, empty
130145
watch plans, no changed-path classification, no source lookup, unsupported
131-
fingerprints, and `(IncrementalOutcome{}, false, nil)` for incremental parsing.
132-
That keeps the engine call surface uniform: every provider can be called through
133-
the full `Provider` interface without feature-specific nil checks.
146+
fingerprints, and `(IncrementalOutcome{}, IncrementalUnsupported, nil)` for
147+
incremental parsing. Unsupported methods that report an error use
148+
`ErrUnsupportedProviderFeature`, so callers can distinguish "feature is absent"
149+
from I/O, database, or parser failures with `errors.Is`. That keeps the engine
150+
call surface uniform: every provider can be called through the full `Provider`
151+
interface without feature-specific nil checks.
152+
153+
Unsupported defaults are still contract checks, not fallback policy. Any
154+
provider that returns a `SourceRef` from discovery, changed-path classification,
155+
lookup, or another provider source path must implement `Fingerprint` for that
156+
reference. The engine treats unsupported fingerprinting for a returned source as
157+
a provider contract failure; it must not silently use a zero fingerprint, mark
158+
the source clean, or downgrade to an unspecified full parse path.
134159

135160
Reusable source helpers must not be generic indirection interfaces or provider
136161
base classes. They are plain source-set structs such as `JSONLSourceSet`,
@@ -148,6 +173,26 @@ var _ Provider = (*CodexProvider)(nil)
148173
Embedding and delegation examples must be compile-tested as part of the provider
149174
harness, so the documented pattern cannot drift into impossible Go.
150175

176+
## Provider Lifecycle And Concurrency
177+
178+
`NewProvider` returns a config-bound provider instance for one sync engine.
179+
Provider instances are long-lived enough to serve full sync, live watch sync,
180+
source lookup, diagnostics, export, and parse-diff calls for that engine.
181+
182+
Provider methods must be safe for concurrent calls. Implementations should keep
183+
configuration and source helper fields immutable after construction. Any cache,
184+
lazy initialization, database handle, or source index stored on the provider
185+
must use normal Go synchronization or be confined to one method call. Providers
186+
must honor `context.Context` cancellation for filesystem, database, and parser
187+
work that can block.
188+
189+
The engine may pass `SourceRef` values between goroutines and queue them for
190+
later work against the same provider instance. `SourceRef.Opaque` therefore must
191+
be immutable, safe to read concurrently, and valid for the life of the provider
192+
instance. It must not contain unguarded mutable state or open handles that
193+
require engine cleanup. When a source needs a handle, the provider should store
194+
stable keys in `Opaque` and open or manage the handle inside provider methods.
195+
151196
## Embedding Pattern
152197

153198
The intended implementation pattern is: embed `ProviderBase`, keep source
@@ -316,6 +361,9 @@ Rules:
316361
- `FingerprintKey` is the DB lookup key used for skip/data-version checks.
317362
- `ProjectHint` is advisory and can be empty.
318363
- `Opaque` is internal provider state. The engine treats it as an opaque token.
364+
- `Opaque` is never persisted or logged, must be immutable for engine callers,
365+
and must remain usable when `SourceRef` is queued across goroutines for the
366+
lifetime of the provider instance.
319367

320368
Backwards compatibility:
321369

@@ -500,11 +548,16 @@ Runtime behavior:
500548
- Multi-session providers return one `ParseResultOutcome` per successfully
501549
parsed session and `SourceErrors` for per-session failures, so good sessions
502550
can still be ingested.
551+
- All session IDs in parse outcomes use the persisted normalized/full session ID
552+
namespace. That includes `ParseResultOutcome.Result.Session.ID`,
553+
`ExcludedSessionIDs`, and `SourceError.SessionID`. Raw upstream IDs may appear
554+
in provider internals, diagnostics, or lookup requests, but the engine
555+
compares outcome IDs only against persisted full session IDs.
503556
- `SourceError.SessionID` is required for per-session failures from
504557
multi-session providers. `SourceKey` and `DisplayPath` are diagnostic source
505558
identifiers, not substitutes for persisted session identity. If the provider
506-
cannot isolate a failure to a session ID, it must return a whole-source
507-
`error` instead of a `SourceError`.
559+
cannot isolate a failure to a persisted full session ID, it must return a
560+
whole-source `error` instead of a `SourceError`.
508561
- `Retryable` decides whether a failure can be cached by mtime.
509562
- `ForceReplace` is the generic signal for full parses that must rewrite
510563
existing ordinals.
@@ -589,15 +642,40 @@ type IncrementalOutcome struct {
589642
PeakContextTokens int
590643
HasTotalOutputTokens bool
591644
HasPeakContextTokens bool
592-
ForceFullParse bool
593645
ForceReplace bool
594646
}
647+
648+
type IncrementalStatus uint8
649+
650+
const (
651+
IncrementalUnsupported IncrementalStatus = iota
652+
IncrementalNoNewData
653+
IncrementalApplied
654+
IncrementalNeedsFullParse
655+
)
595656
```
596657

597-
`ProviderBase.ParseIncremental` returns `(IncrementalOutcome{}, false, nil)`.
598-
Providers that support append-only incremental parsing set the relevant source
599-
capability and implement the method. Typed full-parse fallback replaces
600-
provider-specific error checks in the engine.
658+
`ProviderBase.ParseIncremental` returns
659+
`(IncrementalOutcome{}, IncrementalUnsupported, nil)`. Providers that support
660+
append-only incremental parsing set the relevant source capability and implement
661+
the method.
662+
663+
Incremental status semantics:
664+
665+
- `IncrementalUnsupported`: this provider does not implement incremental append
666+
for the requested source; the engine should run the normal full parse path.
667+
- `IncrementalNoNewData`: the source is valid and no append was needed; the
668+
engine may update freshness metadata without writing messages.
669+
- `IncrementalApplied`: `IncrementalOutcome` contains appended messages and
670+
counters that should be written.
671+
- `IncrementalNeedsFullParse`: the provider inspected the source but cannot
672+
safely append; the engine must run the normal full parse path.
673+
674+
Errors from `ParseIncremental` are real failures for the attempted incremental
675+
operation. The engine uses `IncrementalStatus`, not provider-specific error
676+
strings or a bare boolean, to decide whether to fall back to full parsing.
677+
`IncrementalRequest.SessionID` and `IncrementalOutcome.SessionID` use the same
678+
persisted full session ID namespace as normal parse outcomes.
601679

602680
## Capabilities
603681

@@ -767,7 +845,8 @@ The generic engine flow becomes:
767845
1. Ask each provider for `SourceFingerprint`.
768846
1. Run generic skip/data-version checks using `FingerprintKey` and fingerprint
769847
fields.
770-
1. Attempt incremental parsing when the provider declares and implements it.
848+
1. Attempt incremental parsing when the provider declares and implements it, and
849+
interpret the result through `IncrementalStatus`.
771850
1. Call provider `Parse` for full parses.
772851
1. Apply existing normalization and DB write paths to each
773852
`ParseResultOutcome.Result` value.
@@ -795,6 +874,8 @@ Source lookup becomes:
795874
The engine must treat stored `file_path` as an advisory compatibility key. It
796875
must not check that path first or assume it is a filesystem path, because some
797876
providers expose virtual paths or logical sessions inside a shared source.
877+
`FindSourceRequest.RawSessionID` is lookup input only; returned parse outcomes,
878+
source errors, and exclusions still use persisted full session IDs.
798879

799880
## Registry
800881

@@ -830,24 +911,36 @@ The implementation should migrate all providers, grouped by source pattern:
830911
providers.
831912
1. Migrate simple JSONL providers with acceptance tests for discovery,
832913
fingerprint, parse output, skip-cache metadata, and data-version behavior.
914+
Run provider-vs-legacy parity assertions for this group before it becomes
915+
provider-backed by default.
833916
1. Add and migrate sibling/composite source providers with acceptance tests for
834917
watch planning, composite fingerprints, sidecar/title refreshes, and changed
835-
path classification.
918+
path classification. Run provider-vs-legacy parity assertions for this group
919+
before it becomes provider-backed by default.
836920
1. Add and migrate virtual-path and SQLite fan-out providers with acceptance
837921
tests for stored advisory paths, logical session lookup, per-session errors,
838-
and source mtime behavior.
922+
and source mtime behavior. Run provider-vs-legacy parity assertions for this
923+
group before it becomes provider-backed by default.
839924
1. Add and migrate non-file import/database providers with acceptance tests for
840-
`FindSource`, fingerprinting, and unsupported source mechanics.
841-
1. Move source-processing callers onto providers: full sync, changed-path sync,
842-
and `SyncSingleSession`.
843-
1. Move lookup/watch callers onto providers: session watch flows, export/source
844-
lookup, source mtime, and token-usage raw source probing.
845-
1. Move diagnostic and comparison callers onto providers: parse-diff and parse
846-
diagnostics.
925+
`FindSource`, fingerprinting, and unsupported source mechanics. Run
926+
provider-vs-legacy parity assertions for this group before it becomes
927+
provider-backed by default.
928+
1. Move source-processing callers onto providers behind parity assertions: full
929+
sync, changed-path sync, and `SyncSingleSession`. Each caller becomes default
930+
only after its parsed output, skip-cache, data-version, source metadata, and
931+
diagnostics parity passes for the provider groups it touches.
932+
1. Move lookup/watch callers onto providers behind parity assertions: session
933+
watch flows, export/source lookup, source mtime, and token-usage raw source
934+
probing. Each caller becomes default only after lookup freshness, virtual
935+
path, source mtime, and raw probing parity passes.
936+
1. Move diagnostic and comparison callers onto providers behind parity
937+
assertions: parse-diff and parse diagnostics. Each caller becomes default
938+
only after report shape and source-error parity passes.
847939
1. Run a transitional parity phase where old `processFile` and new provider
848-
dispatch can be compared in tests. The parity assertions must include parsed
849-
output, excluded IDs, skip-cache decisions, data-version writes, retry-needed
850-
outcomes, persisted source metadata, and diagnostics.
940+
dispatch can be compared in tests across all migrated groups. The parity
941+
assertions must include parsed output, excluded IDs, skip-cache decisions,
942+
data-version writes, retry-needed outcomes, persisted source metadata, and
943+
diagnostics.
851944
1. Replace `processFile` switch with generic provider dispatch only after the
852945
grouped parity tests pass.
853946
1. Remove or deprecate old `AgentDef` source callback fields after all callers
@@ -865,7 +958,12 @@ Required tests:
865958
- Provider factory instantiation: configured roots and machine are copied into
866959
config-bound providers and do not mutate singleton registry state.
867960
- `ProviderBase` contract tests: zero-value optional methods are callable and
868-
return the documented no-op results.
961+
return the documented no-op or typed unsupported results.
962+
- Unsupported-feature tests proving the engine distinguishes
963+
`ErrUnsupportedProviderFeature` from I/O and parse errors, and treats
964+
unsupported fingerprinting for returned sources as a contract failure.
965+
- Provider concurrency tests or race tests for shared provider instances and
966+
immutable `SourceRef.Opaque` payloads.
869967
- Compile-tested embedding examples for `ProviderBase`, named JSONL source-set
870968
fields, sibling metadata source-set fields, and explicit concrete method
871969
overrides.
@@ -882,6 +980,9 @@ Required tests:
882980
groups.
883981
- Stored advisory path tests proving `FindSourceRequest.StoredFilePath` is
884982
interpreted only by the provider.
983+
- Outcome ID namespace tests proving `ParseResult` session IDs,
984+
`ExcludedSessionIDs`, and `SourceError.SessionID` use persisted full session
985+
IDs even when lookup starts with a raw upstream ID.
885986
- SQLite fan-out source key, virtual path, and per-session error tests.
886987
- Data-version tests for current, skipped, retry-needed, and mixed per-session
887988
parse outcomes from one source.
@@ -892,10 +993,14 @@ Required tests:
892993
sources, with the pass criteria from the fingerprint section.
893994
- Provider harness tests for discovery, fingerprint, parse, source lookup, and
894995
optional incremental parsing.
996+
- Incremental parsing tests for `IncrementalUnsupported`,
997+
`IncrementalNoNewData`, `IncrementalApplied`, `IncrementalNeedsFullParse`, and
998+
real incremental errors.
895999
- Parse diagnostic tests proving stable source fields are reported and opaque
8961000
payloads are not serialized or logged.
8971001
- Migration parity tests comparing provider output to current parser/process
898-
output during the transition, including skip-cache, data-version writes,
1002+
output after each provider group and before each caller surface becomes
1003+
provider-backed by default, including skip-cache, data-version writes,
8991004
persisted source metadata, diagnostics, excluded IDs, and retry-needed
9001005
behavior.
9011006
- Sync integration tests for incremental Claude/Codex, multi-session sources,
@@ -920,7 +1025,8 @@ decisions from those structures:
9201025
affected source/session current for the parser data version;
9211026
- non-retryable per-session failure: eligible for failure-cache persistence, but
9221027
not for a clean source skip-cache entry;
923-
- full parse fallback from incremental: typed outcome flag;
1028+
- full parse fallback from incremental: `IncrementalNeedsFullParse`;
1029+
- unsupported optional provider feature: `ErrUnsupportedProviderFeature`;
9241030
- successful lower-resolution fallback: per-result `DataVersionNeedsRetry` plus
9251031
`RetryReason`;
9261032
- skipped non-session source: explicit `SkipReason`;

0 commit comments

Comments
 (0)