Skip to content

Commit e8d99f0

Browse files
maskarbclaude
andcommitted
chore: move model-discovery scripts to .github/scripts/
Move GHA-only scripts to .github/scripts/ where they belong. Update workflow reference and DEFAULT_MANIFEST path. Address PR review feedback: - Document the pagination safety limit (20 pages × 100 = 2000 max) - Use @patch for SEED_MODELS in tests instead of direct mutation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 6bb772f commit e8d99f0

4 files changed

Lines changed: 78 additions & 40 deletions

File tree

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
# ---------------------------------------------------------------------------
3939

4040
DEFAULT_MANIFEST = (
41-
Path(__file__).resolve().parent.parent
41+
Path(__file__).resolve().parent.parent.parent
4242
/ "components"
4343
/ "manifests"
4444
/ "base"
@@ -49,6 +49,10 @@
4949
# e.g. claude-opus-4-6 and claude-opus-4-5 are kept, claude-opus-4-1 is dropped.
5050
MAX_VERSIONS_PER_FAMILY = 2
5151

52+
# Model Garden list API pagination settings.
53+
LIST_PAGE_SIZE = 100
54+
MAX_LIST_PAGES = 20
55+
5256

5357
# Publisher discovery configuration.
5458
# prefixes: only models whose ID starts with one of these are included.
@@ -138,8 +142,8 @@ def list_publisher_models(publisher: str, token: str) -> list[tuple[str, str | N
138142
all_models: list[tuple[str, str | None]] = []
139143
page_token = ""
140144

141-
for _ in range(20): # page limit safety
142-
params = {"pageSize": "100"}
145+
for _ in range(MAX_LIST_PAGES):
146+
params = {"pageSize": str(LIST_PAGE_SIZE)}
143147
if page_token:
144148
params["pageToken"] = page_token
145149

@@ -161,10 +165,16 @@ def list_publisher_models(publisher: str, token: str) -> list[tuple[str, str | N
161165
data = json.loads(resp.read().decode())
162166
break
163167
except urllib.error.HTTPError as e:
164-
# Permission denied or not found — retrying won't help
165-
if e.code in (403, 404):
168+
# Auth failures are fatal — don't fall back to seeds with bad credentials
169+
if e.code in (401, 403):
170+
raise RuntimeError(
171+
f"list models for {publisher} failed (HTTP {e.code}): "
172+
f"check GCP credentials and IAM permissions"
173+
) from e
174+
# Not found — retrying won't help
175+
if e.code == 404:
166176
print(
167-
f" WARNING: list models for {publisher} failed (HTTP {e.code})",
177+
f" WARNING: list models for {publisher} returned 404",
168178
file=sys.stderr,
169179
)
170180
return []
@@ -503,22 +513,24 @@ def probe_model(
503513

504514

505515
def load_manifest(path: Path) -> dict:
506-
"""Load the model manifest JSON, or return a blank manifest if missing/empty."""
507-
blank = {"version": 1, "defaultModel": "claude-sonnet-4-5", "models": []}
516+
"""Load the model manifest JSON, or return a blank manifest if missing.
517+
518+
Raises on malformed JSON to prevent overwriting a corrupt file.
519+
Returns a blank manifest only when the file does not exist yet.
520+
"""
508521
if not path.exists():
509-
return blank
510-
try:
511-
with open(path) as f:
512-
data = json.load(f)
513-
if not isinstance(data, dict) or "models" not in data:
514-
return blank
515-
return data
516-
except (json.JSONDecodeError, ValueError) as e:
517-
print(
518-
f"WARNING: malformed manifest at {path}, starting fresh ({e})",
519-
file=sys.stderr,
522+
return {"version": 1, "defaultModel": "claude-sonnet-4-5", "models": []}
523+
524+
with open(path) as f:
525+
data = json.load(f)
526+
527+
if not isinstance(data, dict) or "models" not in data:
528+
raise ValueError(
529+
f"manifest at {path} is missing required 'models' key — "
530+
f"fix the file manually or delete it to start fresh"
520531
)
521-
return blank
532+
533+
return data
522534

523535

524536
def save_manifest(path: Path, manifest: dict) -> None:

.github/workflows/model-discovery.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ jobs:
4040
env:
4141
GCP_REGION: ${{ secrets.GCP_REGION }}
4242
GCP_PROJECT: ${{ secrets.GCP_PROJECT }}
43-
run: python scripts/model-discovery.py
43+
run: python .github/scripts/model-discovery.py
4444

4545
- name: Check for changes
4646
id: diff

.github/workflows/unit-tests.yml

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ on:
1010
- 'components/ambient-cli/**'
1111
- 'components/ambient-sdk/go-sdk/**'
1212
- 'components/frontend/**'
13+
- '.github/scripts/**'
14+
- 'tests/**'
1315
- '.github/workflows/unit-tests.yml'
1416
- '!**/*.md'
1517

@@ -21,6 +23,8 @@ on:
2123
- 'components/ambient-cli/**'
2224
- 'components/ambient-sdk/go-sdk/**'
2325
- 'components/frontend/**'
26+
- '.github/scripts/**'
27+
- 'tests/**'
2428
- '.github/workflows/unit-tests.yml'
2529
- '!**/*.md'
2630

@@ -50,6 +54,7 @@ jobs:
5054
runner: ${{ steps.filter.outputs.runner }}
5155
cli: ${{ steps.filter.outputs.cli }}
5256
frontend: ${{ steps.filter.outputs.frontend }}
57+
scripts: ${{ steps.filter.outputs.scripts }}
5358
steps:
5459
- name: Checkout code
5560
uses: actions/checkout@v6
@@ -70,6 +75,9 @@ jobs:
7075
- 'components/ambient-sdk/go-sdk/**'
7176
frontend:
7277
- 'components/frontend/**'
78+
scripts:
79+
- '.github/scripts/**'
80+
- 'tests/test_model_discovery.py'
7381
7482
backend:
7583
runs-on: ubuntu-latest
@@ -300,9 +308,26 @@ jobs:
300308
- name: Run unit tests with coverage
301309
run: npx vitest run --coverage
302310

311+
scripts:
312+
runs-on: ubuntu-latest
313+
needs: detect-changes
314+
if: needs.detect-changes.outputs.scripts == 'true' || github.event_name == 'workflow_dispatch'
315+
name: Script Tests (model-discovery)
316+
steps:
317+
- name: Checkout code
318+
uses: actions/checkout@v6
319+
320+
- name: Set up Python
321+
uses: actions/setup-python@v6
322+
with:
323+
python-version: '3.11'
324+
325+
- name: Run tests
326+
run: python -m pytest tests/test_model_discovery.py -v
327+
303328
summary:
304329
runs-on: ubuntu-latest
305-
needs: [detect-changes, backend, api-server, runner, cli, frontend]
330+
needs: [detect-changes, backend, api-server, runner, cli, frontend, scripts]
306331
if: always()
307332
steps:
308333
- name: Check overall status
@@ -313,7 +338,8 @@ jobs:
313338
"${{ needs.api-server.result }}" \
314339
"${{ needs.runner.result }}" \
315340
"${{ needs.cli.result }}" \
316-
"${{ needs.frontend.result }}"; do
341+
"${{ needs.frontend.result }}" \
342+
"${{ needs.scripts.result }}"; do
317343
if [ "$result" == "failure" ] || [ "$result" == "cancelled" ]; then
318344
failed=true
319345
fi
@@ -325,6 +351,7 @@ jobs:
325351
echo " runner: ${{ needs.runner.result }}"
326352
echo " cli: ${{ needs.cli.result }}"
327353
echo " frontend: ${{ needs.frontend.result }}"
354+
echo " scripts: ${{ needs.scripts.result }}"
328355
exit 1
329356
fi
330357
echo "All unit tests passed!"
Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88

99
# Import model-discovery.py as a module (it has a hyphen in the name)
1010
_spec = importlib.util.spec_from_file_location(
11-
"model_discovery", Path(__file__).parent / "model-discovery.py"
11+
"model_discovery",
12+
Path(__file__).resolve().parent.parent / ".github" / "scripts" / "model-discovery.py",
1213
)
1314
_mod = importlib.util.module_from_spec(_spec)
1415
sys.modules["model_discovery"] = _mod
@@ -152,24 +153,22 @@ class TestDiscoverModels(unittest.TestCase):
152153
"""Test discover_models seed fallback with version_cutoff."""
153154

154155
@patch("model_discovery.list_publisher_models", return_value=[])
156+
@patch(
157+
"model_discovery.SEED_MODELS",
158+
_mod.SEED_MODELS + [("gemini-2.0-flash", "google", "google")],
159+
)
155160
def test_seed_models_respect_version_cutoff(self, _mock_list):
156161
"""Seed models older than version_cutoff should be excluded."""
157-
# Add a gemini-2.0 model to SEED_MODELS temporarily
158-
original_seeds = _mod.SEED_MODELS[:]
159-
try:
160-
_mod.SEED_MODELS.append(("gemini-2.0-flash", "google", "google"))
161-
manifest = {
162-
"defaultModel": "claude-sonnet-4-5",
163-
"providerDefaults": {"google": "gemini-2.5-flash"},
164-
}
165-
result = discover_models("fake-token", manifest)
166-
ids = [r[0] for r in result]
167-
# gemini-2.0-flash should be excluded by version_cutoff (2, 0)
168-
self.assertNotIn("gemini-2.0-flash", ids)
169-
# gemini-2.5-flash from seeds should still be present
170-
self.assertIn("gemini-2.5-flash", ids)
171-
finally:
172-
_mod.SEED_MODELS[:] = original_seeds
162+
manifest = {
163+
"defaultModel": "claude-sonnet-4-5",
164+
"providerDefaults": {"google": "gemini-2.5-flash"},
165+
}
166+
result = discover_models("fake-token", manifest)
167+
ids = [r[0] for r in result]
168+
# gemini-2.0-flash should be excluded by version_cutoff (2, 0)
169+
self.assertNotIn("gemini-2.0-flash", ids)
170+
# gemini-2.5-flash from seeds should still be present
171+
self.assertIn("gemini-2.5-flash", ids)
173172

174173

175174
if __name__ == "__main__":

0 commit comments

Comments
 (0)