kaxil commented on code in PR #70498:
URL: https://github.com/apache/airflow/pull/70498#discussion_r3677876967
##########
registry/src/js/provider-filters.js:
##########
@@ -40,9 +40,11 @@
const lifecycle = item.dataset.lifecycle;
const name = item.dataset.name || '';
const categories = item.dataset.categories || '';
+ const integrations = item.dataset.integrations || '';
const matchesLifecycle = currentLifecycle === 'all' || lifecycle ===
currentLifecycle;
- const matchesSearch = name.includes(currentSearch.toLowerCase());
+ const search = currentSearch.toLowerCase();
+ const matchesSearch = name.includes(search) ||
integrations.includes(search);
Review Comment:
The build-time matcher collapses `[-_\s]+` to a single space on both sides
specifically so `'pydantic-ai'` matches "Pydantic AI", but this comparison is a
raw `includes`, so the two halves of the same feature now disagree. I simulated
`matchesSearch` against the rendered `data-integrations` for all 105 in-tree
providers: `pydantic ai`, `langchain`, `mcp` and `llamaindex` all find
`common-ai`, while `pydantic-ai`, `pydantic_ai` and `mcp-server` find nothing,
and `speech to text` misses `google` because the integration is named "Google
Cloud Speech-to-Text". Running `normalize()` on both sides here closes all of
those and picks up `common-ai` as a query for free; a browser IIFE can't
`require` the module, so a comment pointing back at
`_data/providerKeywordMatch.js` is probably the best you can do about the
duplication.
##########
registry/src/providers.njk:
##########
@@ -63,12 +63,19 @@ mainClass: providers-page
{% set lc = provider.lifecycle or "production" %}
{% set lcDisplay = "incubation" if lc == "incubation" else ("deprecated"
if lc == "deprecated" else "stable") %}
{% set cats = providerCategoryMap[provider.id] or [] %}
+ {% set integrations = [] %}
+ {% for cat in provider.categories or [] %}
+ {% if cat.name %}
+ {% set integrations = (integrations.push(cat.name | lower),
integrations) %}
Review Comment:
`push` mutates in place and Nunjucks `{% set %}` inside a `{% for %}` writes
to the enclosing frame, so the `(push(), integrations)` comma expression isn't
buying anything over a bare push. `{{ (provider.categories or []) |
selectattr('name') | join(',', 'name') | lower }}` inline on line 78 renders
byte-identical output on nunjucks 3.2.4 (checked against a normal provider, a
missing `categories` key, an empty list, an entry with no name, and the one
integration name containing `&`), and lets lines 66-71 go away.
##########
registry/src/providers.njk:
##########
@@ -63,12 +63,19 @@ mainClass: providers-page
{% set lc = provider.lifecycle or "production" %}
{% set lcDisplay = "incubation" if lc == "incubation" else ("deprecated"
if lc == "deprecated" else "stable") %}
{% set cats = providerCategoryMap[provider.id] or [] %}
+ {% set integrations = [] %}
+ {% for cat in provider.categories or [] %}
+ {% if cat.name %}
+ {% set integrations = (integrations.push(cat.name | lower),
integrations) %}
+ {% endif %}
+ {% endfor %}
<li class="provider-item"
data-lifecycle="{{ lcDisplay }}"
data-name="{{ provider.name | lower }}"
data-downloads="{{ provider.pypi_downloads.monthly }}"
data-updated="{{ provider.last_updated or '' }}"
- data-categories="{{ cats | join(',') }}">
+ data-categories="{{ cats | join(',') }}"
+ data-integrations="{{ integrations | join(',') }}">
Review Comment:
Now that the search box matches integration names, a card can match on text
it never renders: searching `bedrock` or `athena` returns the Amazon card,
whose only prose is "Amazon integration (including Amazon Web Services (AWS))",
and `llamaindex` returns Common AI with no mention of LlamaIndex, which reads
more like a filtering bug than a hit. Since `data-integrations` is already on
the `<li>`, `provider-filters.js` could surface the matched name on the card at
filter time without any new build data. Follow-up rather than a blocker, but
worth deciding.
--
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]
