kaxil commented on code in PR #71478:
URL: https://github.com/apache/airflow/pull/71478#discussion_r3998044401
##########
registry/scripts/build-pagefind-index.mjs:
##########
@@ -44,20 +46,26 @@ async function buildPagefindIndex() {
.map((category) => category.name)
.filter(Boolean)
.join(' ');
+ const externalServicesList = collectExternalServices(provider);
+ const externalServices = externalServicesList.join(' ');
await index.addCustomRecord({
url: `/providers/${provider.id}/${provider.version}/`,
// The id is indexed on its own, not as part of the distribution name:
// the `apache-airflow-providers-` prefix is shared by every provider and
// would make 'apache' or 'airflow' match all of them.
- content: `${provider.name} ${provider.id} ${provider.description}
${integrations}`,
+ content: `${provider.name} ${provider.id} ${provider.description}
${integrations} ${externalServices}`,
Review Comment:
Putting the service names in `content` flips ranking against the providers
that actually implement the service, because pagefind's BM25 normalises by
document length. Common AI's record goes 24 -> 40 words while Amazon's stays
153 and Google's 219, each with a single occurrence of the term. A/B on the
real pagefind runtime, Providers tab: `bedrock` goes amazon 0.604 (#1) ->
common-ai 1.659 (#1, amazon 0.537); `vertex` goes google 0.433 -> common-ai
1.659; `vertex ai` goes google 1.162 -> common-ai 7.546. `google`, `aws`,
`azure` and `openai` are untouched -- the incumbents keep #1 by a wide margin.
The gain is real too: `ollama` and `groq` went from zero results to Common AI,
and `anthropic` from noise (openlineage) to the right provider.
`addCustomRecord` has no per-term weighting, so the lever is which services go
into `content`: keeping a service out of `content` when it collides with
another provider's own name or id (leaving it in `meta` and `data-services`)
would preserve bo
th effects. Either way it's worth a line in the description so the tradeoff is
signed off rather than discovered.
##########
registry/src/_data/exploreCategories.js:
##########
@@ -57,6 +57,13 @@ module.exports = [
color: 'amber',
keywords: ['openai', 'cohere', 'anthropic', 'huggingface', 'mlflow',
'pinecone', 'qdrant', 'weaviate', 'pgvector', 'langchain', 'llamaindex', 'mcp',
'pydantic ai'],
description: 'OpenAI, vector DBs, and ML platforms',
+ // Opt-in only: this category's keywords are capability claims ("this
+ // provider integrates with X"), the same axis as `external-services`
+ // ("upstream provider/service names this connection type reaches" --
+ // provider.yaml.schema.json:475), so external services may also match
+ // here. Every other category's keywords are identity claims (this
+ // distribution's own directory id) and must not opt in.
+ includeExternalServices: true,
Review Comment:
The conclusion is right, but the rule stated for it doesn't hold in either
direction, and this comment is the only written rationale for the new flag
anywhere. `dingtalk` isn't a directory id (the distribution is `dingding`; it
matches only the integration name "DingTalk", which is why #70498 added it),
and neither are `dataflow`, `couchbase` or `twilio` -- while 7 of `ai-ml`'s own
13 keywords are directory ids (openai, cohere, anthropic, pinecone, qdrant,
weaviate, pgvector). The concrete version is stronger: flipping the flag on for
every category across all 107 `provider.yaml` files changes exactly one,
`cloud`, which picks up Common AI through keyword `google` matching the
services "Google" and "Google Vertex AI". Worth saying that instead. Related:
on today's data the `ai-ml` opt-in is a 0-delta, since Common AI already
matches through langchain, llamaindex, mcp and pydantic ai, so a sentence in
the description saves a reviewer hunting for a changed Explore page.
##########
registry/src/js/search.js:
##########
@@ -110,13 +120,29 @@
const icon = type === 'provider' ? 'P' : (moduleType ?
(typeIcons[moduleType] || moduleType[0].toUpperCase()) : 'M');
const resultType = type === 'provider' ? 'provider' : moduleType;
+ // A provider often matches because of an external service its
connections
+ // reach (e.g. "anthropic" -> Common AI), not the provider's own name --
+ // badge the specific service so that's visible instead of just the
+ // provider it lives under.
+ let matchedService = '';
+ if (type === 'provider' && currentQuery) {
+ const services = (result.meta.externalServices ||
'').split(',').filter(Boolean);
+ const normalizedQuery = normalize(currentQuery);
+ matchedService = services.find((service) => {
Review Comment:
The word-start fix does what I asked and `mistral-ai`, `llm` and
`ai`-vs-OpenAI all behave now, but the badge still derives from `currentQuery`
alone and never from what pagefind matched, so it can still name a service that
had nothing to do with the hit. Searching `ai` returns Common AI on its own
name and badges it "Mistral AI"; `a` returns it #1 and badges "Anthropic".
Skipping the lookup when the provider's own identity already explains the hit
fixes it without losing any legitimate badge (openai, anthropic, bedrock,
vertex, azure, ollama, vllm, mistral-ai all still badge): `const selfText =
normalize(name + ' ' + (result.meta.providerId || ''))` and only run the `find`
when `!selfText.includes(normalizedQuery)`.
##########
registry/src/_data/providerKeywordMatch.js:
##########
@@ -39,20 +45,30 @@ function fuzzyIncludes(value, keyword) {
}
// Every string a provider is searchable by: its id/slug and its declared
-// integration names (provider.categories[].name — e.g. "LangChain",
-// "Pydantic AI").
-function collectSearchableValues(provider) {
+// integration names (provider.categories[].name) are always included.
+// Its declared `external_services` are included only when the caller opts
+// in via `{ includeExternalServices: true }` — see exploreCategories.js
+// for which category does and why.
+function collectSearchableValues(provider, { includeExternalServices = false }
= {}) {
Review Comment:
`registry/README.md` still documents this matcher as keywords "matched
against `provider.id` and the provider's declared integration names", in both
the `keywords` field bullet and the paragraph below it, and the field list
doesn't mention `includeExternalServices` at all. #70498 widened this same
matcher and updated that exact paragraph, so it seems worth the same treatment
here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
