feat(parser): provider facade core#876
Open
mariusvniekerk wants to merge 2 commits into
Open
Conversation
Parser integration has grown across registry callbacks, sync-engine switches, and provider-specific source handling. This design captures a shared provider facade so future parser work has one contract for discovery, fingerprinting, parsing, capabilities, and source lookup while preserving current normalized output types and sync semantics. It also records the decision to keep source shape provider-owned, provide reusable JSONL source helpers, migrate every existing provider, and use enumer-generated capability support values for readable JSON reporting. docs(parser): clarify provider embedding pattern The provider facade design was intended to use embedded defaults, but the written spec did not show that pattern concretely enough. Add explicit provider examples so new provider implementations start from ProviderBase and embed or delegate source helper types instead of treating the facade as a loose collection of functions. docs(parser): harden provider facade contract The provider facade spec needed sharper boundaries before implementation: providers must be root-bound instances, source lookup cannot assume persisted paths are real files, and parse outcomes need to preserve retry eligibility without corrupting data-version state. This keeps ProviderBase as the single embedded no-op surface while making reusable source discovery plain composition with explicit forwarding, so new provider work has a predictable contract without a hidden hook layer. Validation: rg stale contract terms; git diff --check. docs(parser): clarify provider facade edge cases The provider facade contract still had ambiguity around mixed multi-session outcomes, fresh source lookup, changed-path filtering, and migration staging. Those gaps would let different provider migrations handle retries, diagnostics, and lookup callers differently. This follow-up makes data-version state per parsed result, defines fresh-source resolution without assuming filesystem paths, gives fingerprint performance criteria, and splits caller migration into reviewable stages. Validation: rg stale contract terms; git diff --check. Go tests not run because this is docs-only. docs(parser): require provider config snapshots Provider instances are meant to be root-bound and immutable after construction. The examples now show ProviderConfig cloning so helper roots and ProviderBase.Config cannot share a mutable caller slice. Validation: rg stale contract terms; git diff --check. Go tests not run because this is docs-only. docs(parser): define partial source failure semantics Multi-session providers need an unambiguous rule for retryable session failures. The spec now requires SourceError.SessionID for per-session errors, routes unknown-scope failures to whole-source errors, and preserves absent rows during partial parses unless the provider explicitly excludes or cleanly replaces them. Validation: rg stale contract terms; git diff --check. Go tests not run because this is docs-only. docs(parser): tighten provider retry cache semantics Provider results now have per-session retry state, but skip-cache persistence remains source-scoped. The spec now makes that aggregate rule explicit and also requires independent root-slice ownership between ProviderBase config and source helpers. Validation: rg stale contract terms; git diff --check. Go tests not run because this is docs-only. docs(parser): require complete results for clean source skips Per-result retry state does not make skip-cache entries per-result. The spec now requires providers to declare a complete result set before the engine can persist a clean source/fingerprint skip, and keeps failures as diagnostic or failure-cache state instead of clean source state. Validation: rg stale contract terms; git diff --check. Go tests not run because this is docs-only. 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. docs(parser): require dual-run provider migration Provider branches need to prove migration, not just add parallel provider implementations. Documenting the root dual-run harness, manifest opt-in, and stack-tip-only legacy removal gives each PR an obvious review surface while preserving legacy sync as the writer during the stack. Validation: git diff --check; spec self-review for placeholders and stale defaulting language; mdformat hook formatting.
Introduce the Provider interface, ProviderBase/ProviderFactory, and source-set helpers; own provider discovery and lookup at the root; and add the legacy-call shim scan that gates provider files. fix(parser): cover Aider, OMP, Reasonix in migration manifest These agents live in the registry but were absent from the provider migration manifest, so ValidateProviderMigrationModes failed once the registry began enforcing that every agent has a mode. They remain on the legacy path here; later stack commits migrate them to concrete providers and flip these entries to provider-authoritative. fix(sync): keep shadow provider discovery observational Shadow provider mode must not add provider-only work or satisfy source lookups that the legacy runtime would miss. Otherwise a migration comparison can change live sync behavior before the provider becomes authoritative.\n\nProvider-authoritative discovery now reports discovery failures as sync failures and suppresses the provider completion watermark for that run, preserving the next incremental pass. The shim scan also keeps pending exemptions honest by failing stale entries while ignoring provider-owned selector methods.\n\nValidation: go test -tags "fts5" ./internal/parser -run 'TestProviderFilesDoNotReferenceLegacyEntrypoints' -count=1; go test -tags "fts5" ./internal/sync -run 'Test(DiscoverProviderSourcesOnlyRunsAuthoritativeProviders|SyncAllProviderDiscoveryFailureSkipsFinishedWatermark|FindSourceFileFallsBackToAuthoritativeNonFileProvider|ClassifyProviderChangedPath|ProcessFileShadow|ProcessFileProviderAuthoritative|ProviderVirtualSourceBackedByEvent)' -count=1; go test -tags "fts5" ./internal/parser ./internal/sync -count=1; go vet ./...; git diff --check docs(parser): clarify provider freshness contract The facade spec still described successful parses as eligible for a clean skip-cache entry, which conflicts with the no-schema-change data-version model and can leave unchanged sessions stale after parser upgrades.\n\nDocument stored changed-path hints explicitly and keep successful unchanged-source freshness tied to DB metadata plus parser data version, reserving skipped_files for retry, failure, and explicit skip cases.\n\nValidation: go test -tags "fts5" ./internal/parser -run 'TestProviderFilesDoNotReferenceLegacyEntrypoints' -count=1; git diff --check. mdformat ran via commit hook. docs(parser): pin provider source identity semantics The facade contract needs to say exactly which provider source key is persisted because the migration intentionally avoids a schema change. Without that rule, providers could diverge between SourceRef, SourceFingerprint, and sessions.file_path identities.\n\nAlso define capability conformance by meaningful field presence so unsupported zero-value fields are treated consistently in provider tests.\n\nValidation: git diff --check. mdformat is unavailable on PATH, but the commit hook ran. style(docs): mdformat provider dual-run harness plan
Collaborator
Author
|
This change is part of the following stack:
Change managed by git-spice. |
This was referenced Jun 26, 2026
This was referenced Jun 26, 2026
mariusvniekerk
changed the title
roborev: Combined Review (
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Establishes the provider facade contract the rest of this stack builds on: the
ProviderandProviderFactoryinterfaces, the facade value types (SourceRef,ParseRequest/ParseOutcome,SourceFingerprint, capabilities), andProviderBaseas the single embedded no-op surface. No providers migrate here — this branch is the foundation plus its design docs.Where to look:
internal/parser/provider.goand the provider-facade design docs.