Repository navigation
feat(skills): add Scouts tab backed by a generic skill category - #64796
Conversation
Split the /skills scene into a Skills tab (uncategorized skills) and a Scouts tab (signals-scout-* skills, canonical + custom), each on its own URL (/skills, /skills/scouts) with a tab-specific description. Decouple scout identification from the signals-scout- name prefix by adding a generic LLMSkill.category column. The list endpoint filters on it (?category=scout / ?category= for uncategorized); a migration backfills existing scouts and the Signals harness stamps category=scout on seed. The frontend tab set is config-driven so future categories (e.g. official AI-plugin skills) are a one-line addition.
|
Size Change: 0 B Total Size: 64.5 MB ℹ️ View Unchanged
|
Query snapshots: Backend query snapshots updatedChanges: 4 snapshots (4 modified, 0 added, 0 deleted) What this means:
Next steps:
|
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
products/skills/backend/api/test/test_skills.py:295-327
The three new `category`-filter tests exercise the same endpoint with slightly different parameters and make very similar assertions. Per the codebase's strong preference for parameterised tests, these should be collapsed into a single `@pytest.mark.parametrize` case so the fixture setup and assertion shape are expressed once.
```suggestion
@pytest.mark.parametrize(
"query_suffix,expected_count,expected_names",
[
("?category=scout", 2, ["signals-scout-errors", "signals-scout-web"]),
("?category=", 1, ["ordinary-skill"]),
("", 2, ["ordinary-skill", "signals-scout-errors"]),
],
)
def test_list_skills_filter_by_category(
self, mock_feature_enabled, query_suffix, expected_count, expected_names
):
self.create_skill(name="ordinary-skill", description="Plain.")
self.create_skill(name="signals-scout-errors", description="A scout.", category="scout")
self.create_skill(name="signals-scout-web", description="Another scout.", category="scout")
response = self.client.get(self._url() + query_suffix)
assert response.status_code == status.HTTP_200_OK
assert response.json()["count"] == expected_count
assert sorted(r["name"] for r in response.json()["results"]) == sorted(expected_names)
```
Reviews (1): Last reviewed commit: "test(mcp): update unit test snapshots" | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4bd303b81c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
- carry LLMSkill.category forward when publishing new versions and adding/ renaming/deleting bundled files, so editing a scout keeps it on the tab - duplicate strips the harness seed marker (keeps category) so a forked canonical scout is correctly shown as Custom, not Canonical - reserve category-tab slugs (e.g. 'scouts') as skill names, front and back, so /skills/:name still resolves for those names - collapse the category-filter tests into one parameterized case - size=small on the scene action buttons to match the Quick start height
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7baa475aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Gates denied due to Django migrations and T2-never classification (cross-cutting feat, 22 files). Additionally, there are several unresolved substantive comments on the current head from bot reviewers flagging real issues: a security concern about client-writable category enabling spoofed canonical badges, a gap where newly registered custom scouts won't appear on the Scouts tab, and a breakage risk for teams that already have a skill named scouts. These need to be addressed before auto-approval.
- category is now read-only via the API (server/producer-owned), so callers can't set arbitrary values or spoof category=scout; regenerated types - duplicating a scout starts uncategorized (new identity, not a registered scout) — keeps non-runnable rows out of the Scouts tab; still strips the harness seed marker from copied metadata - add category to skills storybook mocks
Query snapshots: Backend query snapshots updatedChanges: 2 snapshots (2 modified, 0 added, 0 deleted) What this means:
Next steps:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81b26e06ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ts show on the Scouts tab Custom scouts authored via the skills API are recognized by the signals-scout- prefix at registration but weren't getting the server-owned category, so they scheduled but stayed off the Scouts tab. ensure_scout_category stamps category=scout on the skill row from both registration paths: the coordinator reconcile (register_missing_configs) and the explicit config-create endpoint.
Migration SQL ChangesHey 👋, we've detected some migrations on this PR. Here's the SQL output for each migration, make sure they make sense:
|
🔍 Migration Risk AnalysisWe've analyzed your migrations for potential risks. Summary: 1 Safe | 1 Needs Review | 0 Blocked
|
The migration-risk analyzer blocks combining AddField with RunPython in one migration (a data migration can hold locks alongside the schema change). Split 0002 into the schema-only AddField and a separate 0003 RunPython backfill. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts: # products/signals/backend/scout_harness/config_registry.py # products/signals/backend/test/test_scout_coordinator.py
…fill-admin * origin/master: (203 commits) fix(experiments): add missing event properties to the backend events on the recalculation workflow. (#64923) refactor(capture): decouple v1 RequestContext from analytics Context (#64184) feat(data-warehouse): add direct_query_enabled toggle to data sources (#64685) feat(data-warehouse): persist duckling bucket on DuckgresServer (#64919) fix(dev): strip read-denied $HOME dirs from sandbox PATH (#64768) fix(mcp-analytics): keep muted text readable inside quill subtrees (#64898) feat(skills): add Scouts tab backed by a generic skill category (#64796) fix(ducklake): make get_instance_region a lazy import in common helpers (#64913) fix(replay-vision): drop vestigial scanner_type from VisionAction selection (#64737) feat(warehouse): add xmin read path for postgres source (#64721) feat(warehouse): add per-table xmin sync radio (flag-gated) (#64718) feat(warehouse): validate xmin sync type and surface availability (#64723) fix(data-warehouse): CDC improvements — fix dead orphan-slot sweeper (#64725) feat(data-warehouse): scaffold Razorpay source (#64484) fix(dags): read duckling backfill bucket from stored catalog (#64909) feat(inbox): add archived tab with restore (#64876) feat(signals): surface a banner when inbox filters are active (#64811) fix(lemon-dialog): isolate nested form dialogs with unique kea logic keys (#62113) feat(session-replay): show event triggers on mobile replay settings tab (#64859) fix(preflight): restore kafka connectivity probe (#58743) ...
Problem
Signals scouts are just skills (the
signals-scout-*rows in the skills store), so they show up mixed into the flat/skillslist alongside everything else. There was no clean way to see "just my scouts" — canonical and custom — in one place.This adds a Scouts tab to the Skills product so scouts live in their own surface, while keeping bare
/skillsand/skills/:nameworking exactly as before.Changes
/skillsis now two tabs, each on its own URL with a tab-specific sub-title:/skills) — uncategorized skills only./skills/scouts) — the scout skills, with a Canonical/Custom origin column./skills/<name>).signals-scout-name prefix. Added a genericLLMSkill.categorycolumn (empty = ordinary skill,"scout"= scout). The skills product treats it as an opaque string; the Signals harness owns the value and stampscategory="scout"on seed. A data migration backfills existing scouts by their historical name prefix (the prefix is used there, and only there).categoryfilter:?category=scout(scouts),?category=(uncategorized), omitted (all). The default Skills tab shows uncategorized, so each future category drops out of the generic list automatically.SKILL_CATEGORY_TABS): adding a future tab — e.g. official AI-plugin skills — is one config entry + one route + stamping thatcategory, no special-casing.category="scout"). The scene loads cheap per-categorycountprobes (limit=1) on mount and after archiving, and the active tab is always kept visible so direct navigation to/skills/scouts(or deleting your last scout while viewing it) never strands you on an unlisted tab.Scope is skills-side + UI only: Signals' prefix-based scout discovery (
register_missing_configs) is intentionally left untouched as a separate follow-up.How did you test this code?
I'm an agent (Claude Code). Automated checks I actually ran:
products/skills/backend/api/test/test_skills.py— 82 passed, including new cases for?category=scout,?category=(uncategorized only), and no-param (all categories).test_scout_harness_lazy_seed.py+test_management_sync_signals_scout_skills.pypass (one team-count assertion failed only under reused--keepdbstate; passes on a fresh DB).sqlmigrateshows a cleanADD COLUMN ... DEFAULT '' NOT NULL(no strayDROP DEFAULT, thanks todb_default);makemigrations --checkreports no drift.typescript:checkclean for the touched files; regenerated OpenAPI types, kea logic types, andproducts.tsx; ruff + oxfmt clean.I did not manually click through the running app — visual verification of the tabs is still worth a human pass.
Automatic notifications
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Built with Claude Code, directed by @AndyMaguire. Key decisions made interactively with the human:
categorycolumn when the human flagged the prefix is going away and the skills model will grow. Chose a first-class column over ametadataJSON key for queryability and to set the pattern for future skill attributes.categoryfilter and makes the design extend cleanly to a future official-plugin tab the human raised.Skills invoked while producing this PR:
/improving-drf-endpoints(serializer + viewset),/django-migrations(thecategorycolumn + backfill),/adopting-generated-api-types(wiring the regenerated client).Agent-authored; requires human review — please give the tabs a quick visual check.