refactor(cli): shared pluggable extractor - #1925
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR lays the groundwork for sharing natural-language search extraction across dirctl and (later) the server gateway by introducing a shared, pluggable extractor abstraction in the client module and refactoring CLI NL decomposition to depend on that abstraction.
Changes:
- Added
client/extractorwith a backend-agnosticExtractorinterface plus local (assets) and remote (gRPC) implementations, and resolution helpers (ResolveExtractor/ResolveConfigured). - Moved NL decomposition into
client/nlsearch, updatingdirctlsearch/routing callers to use the shared interface and options. - Updated shared config to persist an optional
RemoteAddr, and adjusted module dependencies/tests accordingly.
Reviewed changes
Copilot reviewed 23 out of 30 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/go.mod | Adds indirect dependency for generated OASF-SDK gRPC bindings used by new remote extractor tests. |
| tests/go.sum | Records checksums for the new generated gRPC bindings dependency. |
| client/go.mod | Adds direct deps needed by shared extractor (gRPC bindings, zerolog) and updates indirect deps via tidy. |
| client/go.sum | Records checksums for newly pulled transitive dependencies. |
| client/config/config.go | Persists extractor.remote_addr in the shared config schema. |
| client/config/config_test.go | Adds coverage ensuring RemoteAddr survives save/load round trips. |
| client/nlsearch/decompose.go | Refactors decomposition to consume client/extractor.Extractor + ExtractOptions. |
| client/nlsearch/decompose_test.go | Adds unit tests for filtering and option forwarding without requiring a real model. |
| client/extractor/extractor.go | Introduces the backend-agnostic Extractor interface and shared option/result types. |
| client/extractor/config.go | Extends extractor config with RemoteAddr and updates package-level docs. |
| client/extractor/config_test.go | Adds tests for default resolution and local config validation. |
| client/extractor/assets.go | Minor comment tweak in teardown guard path. |
| client/extractor/assets_test.go | Adds tests for provision detection and teardown safety guards. |
| client/extractor/local.go | Implements local backend wrapper and moves provisioning/load/smokecheck into client. |
| client/extractor/load_test.go | Adds tests ensuring load paths error clearly when unconfigured/unprovisioned. |
| client/extractor/resolve.go | Adds backend selection + configured resolver, including remote-first selection. |
| client/extractor/resolve_test.go | Adds unit tests for backend selection and resolution behavior. |
| client/extractor/remote.go | Implements remote backend and maps gRPC responses into SDK result shapes. |
| client/extractor/remote_test.go | Adds mapping tests using a fake gRPC client. |
| client/extractor/provision_integration_test.go | Adds an opt-in integration test for provisioning + smoke check. |
| cli/cmd/search/nlsearch.go | Rewires CLI NL search to resolve the shared extractor and call shared decomposition. |
| cli/cmd/routing/nlsearch.go | Rewires routing NL search to shared extractor/decomposer, keeping local-only tuning knobs. |
| cli/cmd/init/run.go | Switches init to use the shared extractor package. |
| cli/cmd/init/options.go | Switches init options wiring to shared extractor package. |
| cli/cmd/init/run_test.go | Updates init tests to reference shared extractor package. |
| cli/cmd/import/config.go | Switches import enricher to shared extractor package for local load/provisioning. |
| cli/go.mod | Adds indirect dependency for generated OASF-SDK gRPC bindings. |
| cli/go.sum | Records checksums for new gRPC bindings dependency. |
| cli/internal/extractor/provision.go | Deleted; functionality moved to client/extractor. |
| cli/internal/extractor/load.go | Deleted; functionality moved to client/extractor. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
akijakya
force-pushed
the
refactor/shared-pluggable-extractor
branch
2 times, most recently
from
July 30, 2026 08:27
2426bf4 to
6e443c3
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…ution Signed-off-by: András Jáky <ajaky@cisco.com>
… backends Signed-off-by: András Jáky <ajaky@cisco.com>
Signed-off-by: András Jáky <ajaky@cisco.com>
… interface Signed-off-by: András Jáky <ajaky@cisco.com>
…r, docs Signed-off-by: András Jáky <ajaky@cisco.com>
Signed-off-by: András Jáky <ajaky@cisco.com>
akijakya
force-pushed
the
refactor/shared-pluggable-extractor
branch
from
July 31, 2026 09:06
6e443c3 to
7dce2b4
Compare
akijakya
enabled auto-merge (squash)
July 31, 2026 09:09
markpmarton
pushed a commit
to markpmarton/dir
that referenced
this pull request
Aug 3, 2026
* feat(client/extractor): add shared extractor config and backend resolution Signed-off-by: András Jáky <ajaky@cisco.com> * feat(client/extractor): add Extractor interface with local and remote backends Signed-off-by: András Jáky <ajaky@cisco.com> * refactor(client/nlsearch): decompose against the Extractor interface Signed-off-by: András Jáky <ajaky@cisco.com> * refactor(cli): move extractor/nlsearch to client, wire callers to the interface Signed-off-by: András Jáky <ajaky@cisco.com> * refactor(client/extractor): address review — Close(), race-safe logger, docs Signed-off-by: András Jáky <ajaky@cisco.com> * fix: restore oasf-sdk grpc dep and allow-list it for licensei Signed-off-by: András Jáky <ajaky@cisco.com> --------- Signed-off-by: András Jáky <ajaky@cisco.com>
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
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.
Summary
Introduces a shared, pluggable OASF extractor in the
clientmodule and moves the natural-language search decomposer onto it, so bothdirctland (in a later step) the server gateway share one extractor abstraction over either an in-process local backend or a remote gRPC OASF-SDK server.This is the foundation slice of the AI Catalog natural-language search epic (#1903): it establishes the abstraction and refactors
dirctlonto it, with no behavior change todirctl searchon the local-assets path.Why
Today the OASF extractor is a CLI-only concern: it lives in
cli/internal/extractoras an in-process library over locally-provisioned assets (~89 MB), and the NL decomposition lives incli/internal/nlsearch. The server gateway (a different module) can't reach any of it, and there's no way to reach the extractor over the network. To bring NL search to the AI Catalog UI, the extraction has to run on a backend that owns the extractor — and a browser can't host the model — so the extractor needs to be:cliandserver(lives inclient, which both already import), andWhat changed
New
client/extractorpackageExtractorinterface +ExtractOptions, returning the SDK'sextractor.Resultso downstream consumers are backend-agnostic.localExtractor— wraps the in-processoasf-sdk/pkg/extractorover provisioned assets.Load,LoadConfigured,Provision,SmokeCheck, and the asset/config helpers move here fromcli/internal/extractor.remoteExtractor— a gRPCExtractorServiceclient that mapsExtractResponseback into the SDKResultshape.ResolveExtractor/ResolveConfigured— pick the backend: remote-first whenRemoteAddris set, else provisioned local assets, else one actionable error naming both fixes.New
client/nlsearchpackageDecompose/DecomposeWithMinScorenow take theExtractorinterface +ExtractOptionsinstead of the concrete*sdk.Extractorand variadicsdk.QueryOption.Config
client/config.Extractorgains a persistedRemoteAddrfield (empty ⇒ local assets).CLI rewire (
init,import,search,routing)client/extractor+client/nlsearch;cli/internal/extractorandcli/internal/nlsearchare deleted.search/routingresolve viaResolveConfigured; routing's semantic-only embedding weights pass through as local-backend tuning.Dependencies
buf.build/gen/go/agntcy/oasf-sdk/grpc/go(pinned to the same buf commit as the already-requiredprotocolbuffers/go).go mod tidyforclient,cli, andtests.Design notes
SearchServicestays NL-agnostic. The planner lives in clients (todaydirctl; the gateway later), not the server.clientmodule — no new module.Testing
chooseBackend/ResolveExtractor),remoteExtractorresponse→Resultmapping (fake gRPC client), andDecomposetier/threshold filtering + keyword wrapping + option forwarding (the decomposer had zero tests before). Existing extractor tests were moved so coverage is preserved.task lint:go— 0 issues across all 7 modules.clientandclimodule test suites — green.server/store/oci → TestIntegrationOCIStoreWorkflowfails ontask test:unit, but it is pre-existing and unrelated — that module is byte-identical to the base commit, and the test requires a live OCI registry atlocalhost:5555(normally provided by the e2e testenv), so it fails on any plain unit-test run.Scope / follow-ups
dirctl initdoes not yet expose a flag to setRemoteAddr; wiring the remote extractor intodirctland the daemon/gateway is [Feature]: Extractor config (RemoteAddr) + resolver wiring for dirctl and gateway #1906.POST /v1/searchendpoint ([Feature]: Backend POST /v1/search endpoint (proto + gateway handler, extract-then-filter) #1905), Helm deployment of the OASF-SDK server ([Feature]: Helm: optional OASF-SDK extractor server deployment + config #1907), UI wiring ([Feature]: AI Catalog UI: wire the search bar to /v1/search #1908), and e2e ([Test]: e2e for AI Catalog /v1/search — local (assets) + remote (OASF-SDK server) #1909) build on this.Closes #1904