From ece3578e911b10be5361de52937e79ddea7852d0 Mon Sep 17 00:00:00 2001 From: Quick <31828688+Quick104@users.noreply.github.com> Date: Fri, 24 Jul 2026 15:15:19 -0400 Subject: [PATCH] docs: rightsize repo context for Claude 5 context engineering (#467) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Applies the practices from Anthropic's "new rules of context engineering for Claude 5 generation models" to this repo's always-on context. AGENTS.md (symlinked as CLAUDE.md) drops content derivable from the filesystem — the module-by-module structure tour, the Makefile target list, and most of the style section, all of which Claude reads directly from internal/, the Makefile, .golangci and web/.prettierrc. What stays is the part that isn't derivable: the Goose migration rules, the v1 additive-only API contract, multi-repo boundaries, and the workspace gotchas. Also fixes a dead pointer to .claude/skills/deployment-debugging, which does not exist; the runbook is the dev-environment-debugging skill. The external-contributor AI disclosure block moves to docs/ai-contributions.md, reached by a one-line pointer, so it costs nothing in the common case where no external PR is being prepared. issue-to-pr sheds the generic agent hygiene now covered by the harness system prompt and keeps the Silo-specific gates. Its commit trailer no longer pins a stale model name. scripts/jellycompat-diff.sh replaces the hand-typed curl/python one-liners the jellycompat-diagnosis skill used to carry as prose. It unions item keys across all returned items rather than reading item[0], which was hiding fields present on only some items. Verified: make verify-local-paths, bash -n, shellcheck, and a mock two-server run of the diff script. Co-authored-by: Claude Opus 5 --- .claude/skills/issue-to-pr/SKILL.md | 46 ++-- AGENTS.md | 174 ++++++-------- docs/ai-contributions.md | 40 ++++ scripts/jellycompat-diff.sh | 355 ++++++++++++++++++++++++++++ 4 files changed, 486 insertions(+), 129 deletions(-) create mode 100644 docs/ai-contributions.md create mode 100755 scripts/jellycompat-diff.sh diff --git a/.claude/skills/issue-to-pr/SKILL.md b/.claude/skills/issue-to-pr/SKILL.md index c43c5624..4337870b 100644 --- a/.claude/skills/issue-to-pr/SKILL.md +++ b/.claude/skills/issue-to-pr/SKILL.md @@ -33,29 +33,17 @@ doing these in order, not from skipping them. ## Operating principles -- **Default to "maybe nothing needs fixing."** Coding agents "fix" already-correct - or stale code in a large fraction of cases because they don't stop to confirm the - problem is real. Confirm the issue reproduces on current `main` before changing - anything — it may already be fixed. -- **Root cause over symptom.** A fix you can't explain is a fix you can't trust. -- **Verify by execution, not by reading.** Build/lint/tests are the primary - correctness signal. The adversarial review is a *second* layer on top — never a - substitute for green checks. If you can't verify it, don't ship it. -- **Smallest correct change.** Match surrounding code's idioms, naming, and comment - density. Put code in the package that owns the behavior (CLAUDE.md); don't create - catch-all helpers or duplicate logic — search for an existing helper before - writing a new one, and extract shared logic instead of copying. -- **Don't trust recall for APIs.** Verify every symbol, function, and signature you - call actually exists by grepping the codebase — agents hallucinate plausible-looking - APIs. -- **Don't gold-plate.** A reviewer told to find gaps will always find some. Address - what affects correctness or the issue's stated requirements; treat the rest as - optional. Extra abstraction "just in case" is a defect, not diligence. -- **Know when to stop.** See [Stop and ask](#stop-and-ask). Autonomy is for - execution, not for inventing product decisions or shipping unverifiable guesses. +- **Default to "maybe nothing needs fixing."** Agents "fix" already-correct or stale + code in a large fraction of cases because they don't stop to confirm the problem is + real. Confirm the issue reproduces on current `main` before changing anything. +- **Verify by execution.** Build/lint/tests are the primary correctness signal. The + adversarial review is a *second* layer on top, never a substitute for green checks. +- **Don't trust recall for APIs.** Grep for every symbol and signature you call — + hallucinated-but-plausible APIs are a common failure here. +- **Know when to stop.** See [Stop and ask](#stop-and-ask). Autonomy is for execution, + not for inventing product decisions or shipping unverifiable guesses. -Track the phases below with a task list (TaskCreate/TaskUpdate) so progress is -visible and nothing is skipped on long runs. +Track the phases below with a task list so progress is visible on long runs. ## Workflow @@ -122,8 +110,8 @@ build-out. ### 4. Implement -Make the change, scoped to this one concern (one concern per PR). Respect the hard -rules: +Make the change, scoped to this one concern (one concern per PR). The two hard rules +that a reviewer cannot un-break after the fact: - **API:** additive-only within `/api/v1` — never rename/remove a response field, change a field's type, or repurpose a status code. New behavior = new @@ -131,8 +119,6 @@ rules: - **Migrations:** new DB changes are Goose SQL migrations via `make migrate-create NAME=...` (timestamped). Never `goose fix`, never paired `.up.sql`/`.down.sql`. -- **Maintainability:** extract shared logic instead of duplicating; change existing - code rather than bolting on local workarounds. For a bug, the failing test from step 3 should now pass. @@ -179,11 +165,9 @@ Do not open the PR while a material, un-rebutted adversarial finding stands. ### 7. Commit, push, open the PR Conventional Commit subjects scoped to the domain, e.g. -`fix(playback): guard against nil session on reconnect`. End commit messages with: - -``` -Co-Authored-By: Claude Opus 4.8 -``` +`fix(playback): guard against nil session on reconnect`, ending with the +`Co-Authored-By:` trailer for the model you are actually running as — don't copy a +model name from this file or from an older commit. Push and open a **ready** (non-draft) PR against `main`: diff --git a/AGENTS.md b/AGENTS.md index 51c77014..7b567275 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,123 +1,101 @@ -# Repository Guidelines +# Silo Server -## Project Structure & Module Organization -`cmd/silo` contains the main server entrypoint. Backend code lives in `internal/`, organized by domain (`api`, `catalog`, `metadata`, `playback`, `scanner`, `jellycompat`, etc.); keep new code in the package that owns the behavior instead of creating catch-all helpers. Database changes belong in `migrations/sql/` as Goose SQL migrations. Legacy converted migrations intentionally keep their original numeric versions so existing `schema_versions` rows can bootstrap cleanly into Goose. New migrations must use timestamped filenames created with `make migrate-create NAME=add_thing`; do not run `goose fix` or create paired `.up.sql` / `.down.sql` files. The React frontend lives in `web/src/`, with feature code split across `components/`, `pages/`, `hooks/`, `player/`, and `lib/`. Reference material belongs in `docs/architecture/` or `docs/superpowers/{specs,plans}/`; ad hoc SQL helpers live in `scripts/`. +Go backend for Silo: API contracts, auth/session, catalog/scanner/playback services, database +migrations, Jellyfin compatibility, and the host-side plugin runtime. `cmd/silo` is the +entrypoint, backend code is under `internal/` by domain, the React frontend is `web/src/`. -When creating or editing `docs/superpowers/specs/` or `docs/superpowers/plans/`, never include local absolute filesystem paths or transient worktree IDs. Use repository-relative paths and wording like "Commands assume the repository root is the cwd." +This repository is a VERY EARLY WIP. Proposing sweeping changes that improve long-term +maintainability is encouraged. -This repository is a VERY EARLY WIP. Proposing sweeping changes that improve long-term maintainability is encouraged. +## Priorities +Performance and reliability first. Keep behavior predictable under load and during failures — +session restarts, reconnects, partial streams. When a tradeoff is forced, choose correctness and +robustness over short-term convenience. -## Core Priorities +Put new code in the package that owns the behavior rather than in a catch-all helper. Prefer +extracting shared logic over duplicating it, and prefer changing existing code over bolting a +local workaround onto it. -1. Performance first. -2. Reliability first. -3. Keep behavior predictable under load and during failures (session restarts, reconnects, partial streams). +## Gotchas -If a tradeoff is required, choose correctness and robustness over short-term convenience. +**Migrations.** New DB changes are Goose SQL migrations in `migrations/sql/`, created with +`make migrate-create NAME=add_thing` so they get timestamped filenames. Never run `goose fix`, +and never create paired `.up.sql` / `.down.sql` files. Legacy converted migrations deliberately +keep their original numeric versions so existing `schema_versions` rows bootstrap cleanly — do +not renumber them. -## Maintainability +**Encrypted settings.** Encrypted `server_settings` rows are GCM-bound to their key name. +Renaming a row in SQL makes its value undecryptable. -Long term maintainability is a core priority. If you add new functionality, first check if there is shared logic that can be extracted to a separate module. Duplicate logic across multiple files is a code smell and should be avoided. Don't be afraid to change existing code. Don't take shortcuts by just adding local logic to solve a problem. +**Profiles vs accounts.** Login accounts (`users`) are separate from household profiles; several +profiles on one account share a `user_id`. A profile's `is_primary` marks the household parent, +which is *not* the server-wide `admin` role on the account. -This repository is part of a broader multi-repo Silo workspace. The sibling -repositories are usually checked out side-by-side in the same parent directory. +**Docs hygiene.** Files under `docs/superpowers/{specs,plans}/` must not contain local absolute +filesystem paths or transient worktree IDs — use repository-relative paths and wording like +"Commands assume the repository root is the cwd." `make verify-local-paths` enforces this. -- `silo-server` owns the Go backend, web admin UI, API contracts, auth/session - behavior, catalog/scanner/playback services, database migrations, Jellyfin - compatibility, and host-side plugin runtime. -- `silo-android` owns the Android phone and TV clients. Client-visible API, - auth, playback, session, library, and metadata changes may require Android - follow-up. -- `silo-apple` owns the iOS, tvOS, and macOS clients. Client-visible API, auth, - playback, session, library, and metadata changes may require Apple follow-up. +**Dev frontend against a remote backend.** Set `VITE_API_PROXY_TARGET` in `web/.env.local` before +`make dev-frontend`; the frontend calls relative `/api` URLs that Vite proxies. -When changing server behavior consumed by clients, check whether both client -repos need model, routing, playback, or UX updates. Prefer coordinated -multi-repo changes over leaving one platform behind. +**Working from a plan.** When implementing from an attached plan, don't edit the plan file. -Do not assume all plugin-related code lives in this repo. +## Multi-repo -- `silo-plugin-sdk` owns the public plugin SDK, protobuf contracts, generated plugin API code, manifest helpers, and runtime bootstrap. -- `silo-plugins` owns the central plugin catalog / repository manifest. -- First-party plugins such as `silo-plugin-metadata-tmdb` and - `silo-plugin-metadata-tvdb` live in their own repositories. -- `Silo` owns host-side plugin installation, runtime management, API handlers, and integration logic. +Sibling repos are usually checked out side-by-side in the same parent directory. -When a task mentions plugins, first determine whether the change belongs in this repo, the SDK repo, the catalog repo, or a specific plugin repo. Prefer coordinated multi-repo changes over forcing plugin work into `Silo`. +- `silo-android` — Android phone and TV clients. +- `silo-apple` — iOS, tvOS, and macOS clients. +- `silo-plugin-sdk` — public plugin SDK, protobuf contracts, generated plugin API, manifest + helpers, runtime bootstrap. +- `silo-plugins` — central plugin catalog / repository manifest. +- First-party plugins (`silo-plugin-metadata-tmdb`, `silo-plugin-metadata-tvdb`, …) each have + their own repo. -## Build, and Development Commands -Use the checked-in `Makefile` for the common paths: +Client-visible changes to API, auth, playback, session, library, or metadata behavior usually +need follow-up in both client repos — prefer coordinated multi-repo changes over leaving a +platform behind. When a task mentions plugins, work out first whether it belongs here, in the +SDK, in the catalog, or in a specific plugin repo. -- `make build`: install frontend deps, build `web/dist`, compile `./silo` -- `make dev-backend`: run the Go server in integrated mode -- `make dev-frontend`: start the Vite dev server with HMR -- `make dev-proxy` / `make dev-transcode`: run standalone worker modes -- `make lint`: run `golangci-lint` and frontend ESLint -- `make migrate-status` / `make migrate-up`: inspect or apply Goose migrations through Silo's legacy-safe bootstrapping runner +## Building and verifying -Run before opening a merge request: +`make build`, `make dev-backend`, `make dev-frontend`, `make lint`, `make migrate-status` / +`make migrate-up` — read the `Makefile` for the rest. Local services: +`docker compose up -d postgres redis`. -- `cd web && pnpm run lint` -- `cd web && pnpm run format:check` -- `make verify-local-paths` +Before opening a merge request: -For local services, start PostgreSQL and Redis with `docker compose up -d postgres redis`. - -## Coding Style & Naming Conventions -Go code must stay `gofmt`/`goimports` clean and pass `golangci-lint`. Keep package names lowercase and focused; Frontend code is TypeScript with 2-space indentation, semicolons, double quotes, trailing commas, and a 100-character line width (`web/.prettierrc`). Use `PascalCase.tsx` for components/pages, `useThing.ts` for hooks, and keep shared utilities in `web/src/lib` or `web/src/utils`. - - -## Deployment Debugging - -When troubleshooting a Silo deployment (container health, playback failures, -database state, log analysis, deploys), follow the runbook at -`.claude/skills/deployment-debugging/SKILL.md`. It includes step-by-step -procedures and a first-run setup that configures SSH, database, and Redis -connection details for your environment. - -## Commit & Merge Request Guidelines -Recent history follows Conventional Commit-style subjects such as `feat(playback): add realtime session hub`, `fix(playback): ...`, and `docs: ...`. Keep commits scoped to one concern. For non-trivial work, open an issue or discussion first; this codebase moves quickly. Merge requests should explain the problem, why this approach was chosen, linked issue/spec/plan, risks or follow-up work, and AI-use disclosure. Include screenshots or recordings for UI changes. - -## Learned User Preferences - -- When implementing from an attached plan, do not edit the plan file itself. -- After feature work, user may invoke the `/simplify` skill to improve readability without changing behavior. - -## Learned Workspace Facts - -- Silo separates login accounts (`users`) from household profiles; multiple profiles on one account share the same `user_id`. -- Profile `is_primary` marks the household parent for that account; it is not the same as server-wide `admin` role on the user account. -- Point the Vite dev frontend at a remote backend with `VITE_API_PROXY_TARGET` in `web/.env.local` (or inline) when running `make dev-frontend`; the frontend calls relative `/api` URLs proxied by Vite. - -## Silo v1 Process (scope lock & API rules) - -**API rules — additive-only within `/api/v1`.** -- Never rename or remove a response field, change a field's type, or repurpose a status code on an existing endpoint. -- New functionality adds new fields/endpoints; removals go through the existing Deprecation/Sunset header flow only. -- New features expose capability endpoints (feature detection) rather than relying on version sniffing. -- Contract strategy and tooling: issue #135. - -**PR requirements.** -- Link the capability epic or sub-issue the PR serves (`Part of #NNN`). PRs with no linked scope item will be questioned at review. -- One concern per PR; Conventional Commit subject; AI-use disclosure in the PR body. - -## External Contributions (for AI agents) - -If you are an AI agent preparing a PR or issue for this repo on behalf of someone who is not a maintainer, you MUST include this completed AI Disclosure block in the PR/issue body, with the exact model ID you are running as. Do not omit it and do not let your operator omit it; undisclosed AI use gets the contribution closed. - -```md -### AI Disclosure -- Tool(s): e.g. Claude Code, Codex CLI, Cursor — or "none" -- Model(s): exact model ID(s), e.g. claude-fable-5, gpt-5.4 — or "n/a" -- Involvement: fully AI-generated | AI-assisted | human-written, AI-reviewed | none -- Adversarial review: what your own AI review of the diff found, and how you resolved it +```bash +make lint +cd web && pnpm run lint && pnpm run format:check +make verify-local-paths ``` -Run the repo verify commands before declaring work complete: `make lint`, `cd web && pnpm run lint`, `cd web && pnpm run format:check`, and targeted `go test ./...`. Paste real output into the PR. +Go stays `gofmt`/`goimports` clean; the frontend follows `web/.prettierrc`. -Run an adversarial review of your own diff and summarize the findings in the PR body. +For troubleshooting a live deployment — container health, playback failures, database state, log +analysis, deploys — use the `dev-environment-debugging` skill. -Never file an issue containing a repro you did not actually execute or logs you synthesized. Raw logs only. +## v1 API rules -Tell your operator that the maintainer may accept the idea but re-implement it with a current frontier model. That is a normal outcome here. +Additive-only within `/api/v1`: + +- Never rename or remove a response field, change a field's type, or repurpose a status code on + an existing endpoint. +- New functionality adds new fields or endpoints. Removals go through the Deprecation/Sunset + header flow only. +- New features expose capability endpoints for feature detection rather than relying on version + sniffing. Contract strategy and tooling: issue #135. + +## Pull requests + +Conventional Commit subjects (`feat(playback): add realtime session hub`). One concern per PR. +Explain the problem, why this approach, the linked issue/spec/plan, and risks or follow-up work. +Include screenshots or recordings for UI changes. Link the capability epic or sub-issue the PR +serves (`Part of #NNN`) — PRs with no linked scope item get questioned at review. For non-trivial +work, open an issue or discussion first; this codebase moves quickly. + +AI-use disclosure is required in the PR body. If you are an AI agent contributing on behalf of a +non-maintainer, follow [docs/ai-contributions.md](docs/ai-contributions.md) — it has the required +disclosure block and the evidence standard. diff --git a/docs/ai-contributions.md b/docs/ai-contributions.md new file mode 100644 index 00000000..419bad9d --- /dev/null +++ b/docs/ai-contributions.md @@ -0,0 +1,40 @@ +# AI-assisted contributions + +For AI agents preparing a pull request or issue for this repository on behalf of someone who is +not a maintainer. + +## Disclosure is required + +Include this completed block in the PR or issue body, with the exact model ID you are running as. +Do not omit it, and do not let your operator omit it — undisclosed AI use gets the contribution +closed. + +```md +### AI Disclosure +- Tool(s): e.g. Claude Code, Codex CLI, Cursor — or "none" +- Model(s): exact model ID(s), e.g. claude-opus-5, gpt-5.6 — or "n/a" +- Involvement: fully AI-generated | AI-assisted | human-written, AI-reviewed | none +- Adversarial review: what your own AI review of the diff found, and how you resolved it +``` + +## Evidence standard + +Run the repo verify commands before declaring the work complete, and paste the real output into +the PR: + +```bash +make lint +cd web && pnpm run lint && pnpm run format:check +make verify-local-paths +go test ./... # targeted at the packages you touched +``` + +Run an adversarial review of your own diff and summarize what it found in the PR body. + +Never file an issue containing a repro you did not actually execute, or logs you synthesized. +Raw logs only. A fabricated repro costs a maintainer more time than no report at all. + +## What to expect + +The maintainer may accept the idea but re-implement it with a current frontier model. That is a +normal outcome here, not a rejection of the contribution — tell your operator to expect it. diff --git a/scripts/jellycompat-diff.sh b/scripts/jellycompat-diff.sh new file mode 100755 index 00000000..a8848eb3 --- /dev/null +++ b/scripts/jellycompat-diff.sh @@ -0,0 +1,355 @@ +#!/usr/bin/env bash +set -euo pipefail + +usage() { + printf 'usage: %s --path PATH [--json]\n' "${0##*/}" >&2 + printf '\n' >&2 + printf 'Compare a Silo jellycompat response with a reference Jellyfin response.\n' >&2 + printf '\n' >&2 + printf 'Required environment variables:\n' >&2 + printf ' SILO_BASE_URL SILO_TOKEN SILO_USER_ID\n' >&2 + printf ' JF_BASE_URL JF_TOKEN JF_USER_ID\n' >&2 +} + +endpoint_path= +json_output=0 + +while [[ "$#" -gt 0 ]]; do + case "$1" in + --path) + if [[ "$#" -lt 2 ]]; then + printf '%s\n' "error: --path requires a value" >&2 + usage + exit 2 + fi + endpoint_path=$2 + shift 2 + ;; + --path=*) + endpoint_path=${1#*=} + shift + ;; + --json) + json_output=1 + shift + ;; + -h | --help) + usage + exit 0 + ;; + *) + printf 'error: unknown argument: %s\n' "$1" >&2 + usage + exit 2 + ;; + esac +done + +if [[ -z "$endpoint_path" ]]; then + printf '%s\n' "error: --path is required" >&2 + usage + exit 2 +fi + +if [[ "$endpoint_path" != /* ]]; then + printf '%s\n' "error: --path must start with /" >&2 + exit 2 +fi + +require_env() { + local name=$1 + + if [[ -z "${!name:-}" ]]; then + printf 'error: required environment variable %s is unset or empty\n' "$name" >&2 + exit 1 + fi +} + +require_env SILO_BASE_URL +require_env SILO_TOKEN +require_env SILO_USER_ID +require_env JF_BASE_URL +require_env JF_TOKEN +require_env JF_USER_ID + +temp_dir=$(mktemp -d) +trap 'rm -rf "$temp_dir"' EXIT + +FETCH_CURL_EXIT=0 +FETCH_HTTP_STATUS=000 +FETCH_TIME_SECONDS= + +fetch_side() { + local base_url=$1 + local token=$2 + local request_path=$3 + local body_file=$4 + local error_file=$5 + local metrics + + FETCH_CURL_EXIT=0 + FETCH_HTTP_STATUS=000 + FETCH_TIME_SECONDS= + + metrics=$( + curl \ + --silent \ + --show-error \ + --output "$body_file" \ + --write-out $'%{http_code}\t%{time_total}' \ + --header "X-Emby-Token: ${token}" \ + "${base_url%/}${request_path}" \ + 2>"$error_file" + ) || FETCH_CURL_EXIT=$? + + IFS=$'\t' read -r FETCH_HTTP_STATUS FETCH_TIME_SECONDS <<<"$metrics" || true +} + +silo_path=${endpoint_path//\{userId\}/$SILO_USER_ID} +jf_path=${endpoint_path//\{userId\}/$JF_USER_ID} + +fetch_side "$SILO_BASE_URL" "$SILO_TOKEN" "$silo_path" "$temp_dir/silo.body" "$temp_dir/silo.error" +silo_curl_exit=$FETCH_CURL_EXIT +silo_http_status=$FETCH_HTTP_STATUS +silo_time_seconds=$FETCH_TIME_SECONDS + +fetch_side "$JF_BASE_URL" "$JF_TOKEN" "$jf_path" "$temp_dir/jellyfin.body" "$temp_dir/jellyfin.error" +jf_curl_exit=$FETCH_CURL_EXIT +jf_http_status=$FETCH_HTTP_STATUS +jf_time_seconds=$FETCH_TIME_SECONDS + +python3 - \ + "$json_output" \ + "$endpoint_path" \ + "$temp_dir/silo.body" \ + "$temp_dir/silo.error" \ + "$silo_curl_exit" \ + "$silo_http_status" \ + "$silo_time_seconds" \ + "$silo_path" \ + "$temp_dir/jellyfin.body" \ + "$temp_dir/jellyfin.error" \ + "$jf_curl_exit" \ + "$jf_http_status" \ + "$jf_time_seconds" \ + "$jf_path" <<'PY' +import json +import os +import sys + + +def parse_status(raw_status): + try: + return int(raw_status) + except ValueError: + return 0 + + +def parse_latency_ms(raw_seconds): + try: + return round(float(raw_seconds) * 1000, 3) + except ValueError: + return None + + +def read_text(path): + try: + with open(path, encoding="utf-8", errors="replace") as handle: + return handle.read().strip() + except FileNotFoundError: + return "" + + +def analyze_side(body_path, error_path, curl_exit, raw_status, raw_latency, request_path): + status = parse_status(raw_status) + body_bytes = os.path.getsize(body_path) if os.path.exists(body_path) else 0 + request_error = read_text(error_path) if curl_exit != 0 else None + response_error = None + total_record_count = None + items_length = None + shape = None + item_keys = set() + + if curl_exit == 0 and 200 <= status < 300: + try: + with open(body_path, encoding="utf-8") as handle: + payload = json.load(handle) + + # Three response shapes are all legitimate here: + # envelope — {Items, TotalRecordCount, ...} from list endpoints + # array — bare BaseItemDto[] from LocalTrailers / SpecialFeatures + # object — the item itself, from detail endpoints like /Items/{id} + # Treating anything but the envelope as an error, or as zero items, + # would report a false clean on exactly the endpoints most likely to + # have drifted. + if isinstance(payload, list): + shape = "array" + items = payload + elif isinstance(payload, dict): + total_record_count = payload.get("TotalRecordCount") + if "Items" in payload: + shape = "envelope" + items = payload["Items"] + if not isinstance(items, list): + raise ValueError("Items is not an array") + else: + shape = "object" + items = [payload] + else: + raise ValueError("top-level JSON value is neither an object nor an array") + + items_length = len(items) + for index, item in enumerate(items): + if not isinstance(item, dict): + raise ValueError(f"item[{index}] is not an object") + item_keys.update(item) + except (OSError, UnicodeError, json.JSONDecodeError, ValueError) as error: + response_error = str(error) + + bytes_per_item = None + if items_length: + bytes_per_item = round(body_bytes / items_length, 2) + + return { + "requestPath": request_path, + "curlExitCode": curl_exit, + "httpStatus": status, + "latencyMs": parse_latency_ms(raw_latency), + "bodyBytes": body_bytes, + "totalRecordCount": total_record_count, + "responseShape": shape, + "itemsLength": items_length, + "itemKeys": sorted(item_keys), + "bytesPerItem": bytes_per_item, + "requestError": request_error, + "responseError": response_error, + } + + +def is_success(side): + return ( + side["curlExitCode"] == 0 + and 200 <= side["httpStatus"] < 300 + and side["responseError"] is None + ) + + +def display_value(value): + if value is None: + return "not present" + return str(value) + + +def display_keys(keys): + return ", ".join(keys) if keys else "(none)" + + +def print_side(name, side): + latency = side["latencyMs"] + latency_display = f"{latency:.3f} ms" if latency is not None else "not available" + bytes_per_item = side["bytesPerItem"] + bytes_per_item_display = ( + f"{bytes_per_item:.2f}" if bytes_per_item is not None else "not available" + ) + + print(f"{name}:") + print(f" Request path: {side['requestPath']}") + print(f" HTTP status: {side['httpStatus']}") + print(f" Latency: {latency_display}") + print(f" Response body: {side['bodyBytes']} bytes") + print(f" Response shape: {display_value(side['responseShape'])}") + print(f" TotalRecordCount: {display_value(side['totalRecordCount'])}") + print(f" Items length: {display_value(side['itemsLength'])}") + print(f" Per-item keys ({len(side['itemKeys'])}): {display_keys(side['itemKeys'])}") + print(f" Approx. bytes per item: {bytes_per_item_display}") + if side["requestError"]: + print(f" Request error: {side['requestError']}") + if side["responseError"]: + print(f" Response error: {side['responseError']}") + + +json_output = sys.argv[1] == "1" +endpoint_path = sys.argv[2] +silo = analyze_side( + sys.argv[3], + sys.argv[4], + int(sys.argv[5]), + sys.argv[6], + sys.argv[7], + sys.argv[8], +) +jellyfin = analyze_side( + sys.argv[9], + sys.argv[10], + int(sys.argv[11]), + sys.argv[12], + sys.argv[13], + sys.argv[14], +) + +silo_keys = set(silo["itemKeys"]) +jellyfin_keys = set(jellyfin["itemKeys"]) +silo_bytes_per_item = silo["bytesPerItem"] +jellyfin_bytes_per_item = jellyfin["bytesPerItem"] +bytes_warning = ( + silo_bytes_per_item is not None + and jellyfin_bytes_per_item is not None + and silo_bytes_per_item > jellyfin_bytes_per_item * 2 +) + +# A shape difference is itself a compat bug — an envelope where Jellyfin sends a +# bare array breaks clients regardless of whether the per-item keys agree. +shape_mismatch = ( + is_success(silo) + and is_success(jellyfin) + and silo["responseShape"] != jellyfin["responseShape"] +) + +comparison = { + "path": endpoint_path, + "ok": is_success(silo) and is_success(jellyfin), + "silo": silo, + "jellyfin": jellyfin, + "diff": { + "missingFromSilo": sorted(jellyfin_keys - silo_keys), + "siloOnly": sorted(silo_keys - jellyfin_keys), + "shared": sorted(silo_keys & jellyfin_keys), + "responseShapeMismatch": shape_mismatch, + "siloBytesPerItemExceedsJellyfinByMoreThan2x": bytes_warning, + }, +} + +if json_output: + print(json.dumps(comparison, separators=(",", ":"), sort_keys=True)) +else: + print_side("Silo", silo) + print() + print_side("Jellyfin", jellyfin) + print() + print("Diff:") + print( + " Jellyfin keys missing from Silo " + f"({len(comparison['diff']['missingFromSilo'])}): " + f"{display_keys(comparison['diff']['missingFromSilo'])}" + ) + print( + f" Silo-only keys ({len(comparison['diff']['siloOnly'])}): " + f"{display_keys(comparison['diff']['siloOnly'])}" + ) + print( + f" Keys in both ({len(comparison['diff']['shared'])}): " + f"{display_keys(comparison['diff']['shared'])}" + ) + if shape_mismatch: + print( + f" WARNING: response shape differs — Silo returned " + f"{silo['responseShape']}, Jellyfin returned {jellyfin['responseShape']}." + ) + if bytes_warning: + print( + " WARNING: Silo's approximate bytes per item exceeds " + "Jellyfin's by more than 2x." + ) + +sys.exit(0 if comparison["ok"] else 1) +PY