feat(skald): sidecar manifest parser + available_models protocol#1
feat(skald): sidecar manifest parser + available_models protocol#1hognek wants to merge 2 commits into
Conversation
…ls protocol - tests/test_skald_manifest.py: 15 tests for load_manifest(), SOFTWARE_TO_BACKEND_TYPE mapping, env var override, missing file, invalid JSON, and edge cases - tests/test_cluster_worker_protocol.py: 9 tests for WorkerInfo available_models field defaults/serialisation/roundtrip, plus 4 tests for ClusterManager.heartbeat() available_models derivation from backends (dedup, additive, empty backends) All 36 new tests pass, 0 regressions in 16 existing cluster tests. Refs: t_634725d4
- tinyagentos/worker/skald_manifest.py: load_manifest() parses the sidecar Taos.md sidecar into a Backend dict; SOFTWARE_TO_BACKEND_TYPE map - tinyagentos/cluster/worker_protocol.py: WorkerInfo gains available_models field (list[str]) - tinyagentos/cluster/manager.py: heartbeat() derives available_models from worker backends (dedup, additive, empty-backends safe) - tinyagentos/worker/agent.py: SkaldProactiveAgent loads manifest on init, exposes available_models for ClusterManager heartbeat Refs: t_634725d4
📝 WalkthroughWalkthroughAdds a new ChangesSkald manifest integration for worker available models
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant WorkerAgent
participant SkaldManifest
participant ClusterManager
participant WorkerInfo
WorkerAgent->>WorkerAgent: probe backends and models
WorkerAgent->>SkaldManifest: load_manifest()
SkaldManifest-->>WorkerAgent: manifest models
WorkerAgent->>WorkerAgent: mark models loaded/available, attach available_models
WorkerAgent->>ClusterManager: heartbeat(backends)
ClusterManager->>ClusterManager: flatten & dedupe available_models by model_id
ClusterManager->>WorkerInfo: update available_models
Suggested labels: enhancement, worker, cluster Suggested reviewers: hognek 🐰 A manifest whispers which models can run, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7d58e6e. Configure here.
| "status": status, | ||
| }) | ||
| if available: | ||
| backend["available_models"] = available |
There was a problem hiding this comment.
Manifest ignores backend port
Medium Severity
Sidecar entries are attached to every probed backend whose type matches SOFTWARE_TO_BACKEND_TYPE, without comparing manifest port/health_url to the backend URL. Multiple llama-cpp instances or Skald services on non-default ports can get the wrong models, ports, and health URLs on the cluster view.
Reviewed by Cursor Bugbot for commit 7d58e6e. Configure here.
| for m in manifest["models"]: | ||
| if SOFTWARE_TO_BACKEND_TYPE.get(m.get("software", "")) != backend_type: | ||
| continue | ||
| model_id = m["model_id"] |
There was a problem hiding this comment.
Missing model_id crashes probe
Medium Severity
Manifest enrichment uses m["model_id"] without a fallback. A valid JSON manifest with a model object missing model_id raises KeyError inside detect_backends, breaking registration and heartbeats until the file is fixed.
Reviewed by Cursor Bugbot for commit 7d58e6e. Configure here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tinyagentos/worker/agent.py (1)
26-31: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
kokoro/whispermanifest entries are currently unreachableSOFTWARE_TO_BACKEND_TYPEmaps them intinyagentos/worker/skald_manifest.py, butWorkerAgent.detect_backends()never probes those backend types, and they also aren’t accepted as valid backend types elsewhere. Either add the corresponding backend support or drop these manifest mappings until they can attach to a real backend.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tinyagentos/worker/agent.py` around lines 26 - 31, The `kokoro` and `whisper` manifest mappings are unreachable because `WorkerAgent.detect_backends()` does not probe those backend types and the backend validation path still rejects them. Update the backend detection/acceptance flow in `WorkerAgent.detect_backends()` and the related backend-type validation so these symbols are recognized end-to-end, or remove the `SOFTWARE_TO_BACKEND_TYPE` entries in `skald_manifest.py` until real support exists.
🧹 Nitpick comments (2)
tinyagentos/worker/agent.py (2)
138-138: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueManifest is re-read from disk and re-parsed on every heartbeat tick.
detect_backends()runs every ~5s viaheartbeat()and callsload_manifest()each time, doing a disk stat + read + JSON parse even though the sidecar manifest rarely changes. Consider caching the parsed result keyed on the file's mtime to avoid unnecessary I/O on this hot path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tinyagentos/worker/agent.py` at line 138, The heartbeat path in detect_backends() is reloading the manifest from disk on every tick, which is unnecessary overhead. Update load_manifest() in agent.py to cache the parsed manifest using the file’s mtime and reuse it when unchanged, and have detect_backends() continue calling that cached loader so heartbeat() avoids repeated stat/read/JSON parse work.
133-162: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test coverage for the manifest-enrichment branch of
detect_backends().Manifest parsing (
skald_manifest.py) and cluster propagation (manager.py/worker_protocol.py) both have dedicated tests, but the mapping/loaded-vs-available logic added here (lines 133-162) isn't covered by any provided test file. Given the branching (backend-type matching, loaded-vs-available status derivation, empty-manifest handling), this seems worth a dedicated unit test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tinyagentos/worker/agent.py` around lines 133 - 162, The manifest-enrichment branch in detect_backends() is missing unit coverage, especially the backend-type filtering and loaded-vs-available status logic. Add a focused test for tinyagentos.worker.agent.detect_backends that mocks load_manifest() and backends to verify matching SOFTWARE_TO_BACKEND_TYPE entries populate available_models with the correct fields and status values. Include cases for a loaded model, an available-but-not-loaded model, and an empty or missing manifest.models path to confirm no enrichment happens when appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tinyagentos/worker/agent.py`:
- Around line 133-162: The manifest enrichment in detect_backends() can throw
and break register() when sidecar data is malformed or mid-write. Add local
exception handling around load_manifest() and the per-model access in this
block, and make missing or invalid manifest entries fail soft by skipping
enrichment instead of propagating. Keep the existing backend loop and
available_models shaping intact, but ensure register() and the run()
registration path stay resilient even if manifest parsing or model_id lookup
fails.
---
Outside diff comments:
In `@tinyagentos/worker/agent.py`:
- Around line 26-31: The `kokoro` and `whisper` manifest mappings are
unreachable because `WorkerAgent.detect_backends()` does not probe those backend
types and the backend validation path still rejects them. Update the backend
detection/acceptance flow in `WorkerAgent.detect_backends()` and the related
backend-type validation so these symbols are recognized end-to-end, or remove
the `SOFTWARE_TO_BACKEND_TYPE` entries in `skald_manifest.py` until real support
exists.
---
Nitpick comments:
In `@tinyagentos/worker/agent.py`:
- Line 138: The heartbeat path in detect_backends() is reloading the manifest
from disk on every tick, which is unnecessary overhead. Update load_manifest()
in agent.py to cache the parsed manifest using the file’s mtime and reuse it
when unchanged, and have detect_backends() continue calling that cached loader
so heartbeat() avoids repeated stat/read/JSON parse work.
- Around line 133-162: The manifest-enrichment branch in detect_backends() is
missing unit coverage, especially the backend-type filtering and
loaded-vs-available status logic. Add a focused test for
tinyagentos.worker.agent.detect_backends that mocks load_manifest() and backends
to verify matching SOFTWARE_TO_BACKEND_TYPE entries populate available_models
with the correct fields and status values. Include cases for a loaded model, an
available-but-not-loaded model, and an empty or missing manifest.models path to
confirm no enrichment happens when appropriate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d426cc5d-bdff-43d1-a62a-670d0073d45c
📒 Files selected for processing (6)
tests/test_cluster_worker_protocol.pytests/test_skald_manifest.pytinyagentos/cluster/manager.pytinyagentos/cluster/worker_protocol.pytinyagentos/worker/agent.pytinyagentos/worker/skald_manifest.py
| from tinyagentos.worker.skald_manifest import ( | ||
| load_manifest, | ||
| SOFTWARE_TO_BACKEND_TYPE, | ||
| ) | ||
|
|
||
| manifest = load_manifest() | ||
| if manifest.get("models"): | ||
| for backend in backends: | ||
| backend_type = backend["type"] | ||
| probed_names = { | ||
| m.get("name", "") for m in backend.get("models", []) | ||
| } | ||
| available = [] | ||
| for m in manifest["models"]: | ||
| if SOFTWARE_TO_BACKEND_TYPE.get(m.get("software", "")) != backend_type: | ||
| continue | ||
| model_id = m["model_id"] | ||
| status = "loaded" if model_id in probed_names else "available" | ||
| available.append({ | ||
| "model_id": model_id, | ||
| "capability": m.get("capability", ""), | ||
| "software": m.get("software", ""), | ||
| "port": m.get("port", 0), | ||
| "vram_required_gb": m.get("vram_required_gb", 0.0), | ||
| "health_url": m.get("health_url", ""), | ||
| "status": status, | ||
| }) | ||
| if available: | ||
| backend["available_models"] = available | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Manifest enrichment can crash register() on malformed/racy sidecar data.
This block has no exception handling, and both indexing (m["model_id"]) and file parsing failures propagate uncaught. heartbeat() (line 434) wraps its entire backend probe in try/except and safely returns 0 on failure, but register() (line 378) calls detect_backends() directly with no protection, and run()'s registration branch (lines 474-479) doesn't guard it either. A malformed or transiently-rewritten manifest file (e.g. JSONDecodeError, a missing model_id key, or a non-dict JSON root) would propagate an unhandled exception out of register(), potentially breaking the worker's registration loop entirely.
🛡️ Proposed fix: contain manifest failures locally
- from tinyagentos.worker.skald_manifest import (
- load_manifest,
- SOFTWARE_TO_BACKEND_TYPE,
- )
-
- manifest = load_manifest()
- if manifest.get("models"):
+ from tinyagentos.worker.skald_manifest import (
+ load_manifest,
+ SOFTWARE_TO_BACKEND_TYPE,
+ )
+
+ try:
+ manifest = load_manifest()
+ except Exception:
+ manifest = {}
+
+ if isinstance(manifest, dict) and manifest.get("models"):
for backend in backends:
backend_type = backend["type"]
probed_names = {
m.get("name", "") for m in backend.get("models", [])
}
available = []
for m in manifest["models"]:
if SOFTWARE_TO_BACKEND_TYPE.get(m.get("software", "")) != backend_type:
continue
- model_id = m["model_id"]
+ model_id = m.get("model_id")
+ if not model_id:
+ continue
status = "loaded" if model_id in probed_names else "available"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from tinyagentos.worker.skald_manifest import ( | |
| load_manifest, | |
| SOFTWARE_TO_BACKEND_TYPE, | |
| ) | |
| manifest = load_manifest() | |
| if manifest.get("models"): | |
| for backend in backends: | |
| backend_type = backend["type"] | |
| probed_names = { | |
| m.get("name", "") for m in backend.get("models", []) | |
| } | |
| available = [] | |
| for m in manifest["models"]: | |
| if SOFTWARE_TO_BACKEND_TYPE.get(m.get("software", "")) != backend_type: | |
| continue | |
| model_id = m["model_id"] | |
| status = "loaded" if model_id in probed_names else "available" | |
| available.append({ | |
| "model_id": model_id, | |
| "capability": m.get("capability", ""), | |
| "software": m.get("software", ""), | |
| "port": m.get("port", 0), | |
| "vram_required_gb": m.get("vram_required_gb", 0.0), | |
| "health_url": m.get("health_url", ""), | |
| "status": status, | |
| }) | |
| if available: | |
| backend["available_models"] = available | |
| from tinyagentos.worker.skald_manifest import ( | |
| load_manifest, | |
| SOFTWARE_TO_BACKEND_TYPE, | |
| ) | |
| try: | |
| manifest = load_manifest() | |
| except Exception: | |
| manifest = {} | |
| if isinstance(manifest, dict) and manifest.get("models"): | |
| for backend in backends: | |
| backend_type = backend["type"] | |
| probed_names = { | |
| m.get("name", "") for m in backend.get("models", []) | |
| } | |
| available = [] | |
| for m in manifest["models"]: | |
| if SOFTWARE_TO_BACKEND_TYPE.get(m.get("software", "")) != backend_type: | |
| continue | |
| model_id = m.get("model_id") | |
| if not model_id: | |
| continue | |
| status = "loaded" if model_id in probed_names else "available" | |
| available.append({ | |
| "model_id": model_id, | |
| "capability": m.get("capability", ""), | |
| "software": m.get("software", ""), | |
| "port": m.get("port", 0), | |
| "vram_required_gb": m.get("vram_required_gb", 0.0), | |
| "health_url": m.get("health_url", ""), | |
| "status": status, | |
| }) | |
| if available: | |
| backend["available_models"] = available |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tinyagentos/worker/agent.py` around lines 133 - 162, The manifest enrichment
in detect_backends() can throw and break register() when sidecar data is
malformed or mid-write. Add local exception handling around load_manifest() and
the per-model access in this block, and make missing or invalid manifest entries
fail soft by skipping enrichment instead of propagating. Keep the existing
backend loop and available_models shaping intact, but ensure register() and the
run() registration path stay resilient even if manifest parsing or model_id
lookup fails.
1 participant
Task: t_634725d4 / t_974e39db. Adds Skald sidecar manifest (Taos.md) parser and wires available_models through the cluster protocol.
Changes
Refs
Closes jaylfc#361 (load-on-demand infrastructure — inert until available_models is populated).
Note
Low Risk
Additive cluster/worker metadata and optional file read on backend detection; no auth or load-on-demand behavior yet.
Overview
Workers can now advertise catalog models they can run (not only what is probed as loaded) by reading Skald’s JSON sidecar via new
load_manifest()(TAOS_SKALD_MANIFEST/ default path), withSOFTWARE_TO_BACKEND_TYPEtying manifest entries to TAOS backend types.WorkerAgent.detect_backends()attaches per-backendavailable_models(metadata +loadedvsavailablefrom probe names).WorkerInfogainsavailable_models, andClusterManager.heartbeat()flattens/dedupes that list from heartbeat backends when present; heartbeats without backends leave existing values unchanged.Coverage is added in
test_skald_manifest.pyandtest_cluster_worker_protocol.py.Reviewed by Cursor Bugbot for commit 7d58e6e. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes