haeaeae/silo-server
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(settings): split quality into two axes and register the orphan keys

Browse Source 
Two manifest changes the cutover needs.

**Quality becomes resolution + bitrate.** The legacy ladder values
(1080p-high, 720p-medium, 1080p-8, 420p, 328p) were never a third dimension
— they are a bitrate spelled into the resolution string. The web player
already decomposes them: useTranscodeQuality.ts defines 1080p-high as
{resolution: 1080p, bitrate: 10000} and sends the two separately, so the
compound form never reached the wire. Downloads went further and kept only
a bitrate ladder.

So playback.preferred_quality keeps the six clean resolutions and
playback.max_bitrate_kbps becomes the second axis, nullable because
"uncapped" is a real answer and a numeric sentinel would need widening
every time hardware improves. Clients compose their own presets from the
pair, which means retuning what "High" means is a client release rather
than a contract break. Migration decomposes each legacy value losslessly,
so none of them lands in the rejects table.

**The five extension-bag keys are now definitions.** card_overlays,
next_up_mode, sidebar_pins, disabled_library_ids and library_order reached
the server only through the unknown-key path, stored as unvalidated
strings. Two of them the server reads back — next_up_mode decides home
section assembly and card_overlays falls back to an admin default — so
they cannot be demoted to client-local. Registering them is what lets the
extension bag close.

Adds three schemas for their shapes and a test that exercises every
schema_ref against a real value: each of these is nullable with a null
default, so the existing default-validation test returns at the null branch
without ever compiling the reference.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Quick
2026-07-27 22:57:34 +00:00
co-authored by Claude Opus 5
parent 1291316c34
commit f44240e5e5
5 changed files with 373 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
contracts/settings/v1/manifest.json
+111 -1
View File
@@ -140,7 +140,27 @@
"label": "Preferred quality",
"description": "Pick the quality Silo should prefer.",
"recommended_control": "select",
"notes": "Members are exactly the vocabulary the server already speaks: NormalizeQualityV3 in internal/playback/protocol_v3.go accepts auto, 480p, 720p, 1080p, 2160p and original and normalizes anything else to auto with a degradation warning. Transcode ladder rungs (328p, 720p-high, 1080p-8 and friends) are deliberately absent — they are encoder outputs, not user-facing choices, and registering them would have promised a preference the planner throws away. Members are listed ascending so the ceiling constraint has a defined direction; \"auto\" sorts lowest because it never exceeds a cap, and \"original\" highest because it is the uncapped source. Enforcing the ceiling needs internal/access/quality.go to learn both sentinels: qualityRank ranks neither today, so auto and original both tie at 0 with unset and a cap would let original through. That is Phase 2 work and the ordering it must implement is the one declared here. Migrates user_profiles.quality_preference as the profile fallback; because that column is NOT NULL DEFAULT '1080p', migration writes a profile row only where the value is not the column default, or every profile would be pinned to 1080p having never chosen it. The account/profile max_playback_quality columns stay in internal/policy and are NOT settings."
"notes": "The resolution axis. Members are exactly the vocabulary the server already speaks: NormalizeQualityV3 in internal/playback/protocol_v3.go accepts auto, 480p, 720p, 1080p, 2160p and original and normalizes anything else to auto with a degradation warning. Transcode ladder rungs (328p, 720p-high, 1080p-8 and friends) are deliberately absent — they were never a third dimension, only a bitrate spelled into the resolution string. web/src/player/hooks/useTranscodeQuality.ts already decomposes them, defining 1080p-high as {resolution: 1080p, bitrate: 10000} and sending the two to the server separately, so the compound form never reached the wire. playback.max_bitrate_kbps is now that second axis, and clients compose the two into whatever presets they want to show. Members are listed ascending so the ceiling constraint has a defined direction; \"auto\" sorts lowest because it never exceeds a cap, and \"original\" highest because it is the uncapped source. Enforcing the ceiling needs internal/access/quality.go to learn both sentinels: qualityRank ranks neither today, so auto and original both tie at 0 with unset and a cap would let original through. Migrates user_profiles.quality_preference as the profile fallback; because that column is NOT NULL DEFAULT '1080p', migration writes a profile row only where the value is not the column default, or every profile would be pinned to 1080p having never chosen it. The account/profile max_playback_quality columns stay in internal/policy and are NOT settings."
},
{
"key": "playback.max_bitrate_kbps",
"introduced_in": 1,
"persistence": "remote",
"allowed_scopes": ["profile", "profile_device"],
"resolution_order": ["profile_device", "profile", "default"],
"value_schema": {
"type": "integer",
"nullable": true,
"minimum": 100,
"maximum": 200000
},
"default_value": null,
"unit": "kbps",
"category": "playback",
"label": "Maximum bitrate",
"description": "Cap how much bandwidth playback may use. No cap means Silo picks for the chosen resolution.",
"recommended_control": "select",
"notes": "The bitrate axis, orthogonal to playback.preferred_quality. Splitting them is what the clients were already doing: the in-player switcher sends resolution and bitrate as separate fields, and downloads (DownloadQuality in silo-android) dropped resolution entirely and kept only a bitrate ladder. Two values rather than one compound enum means a client can offer \"1080p High\" without the server having to agree on what \"High\" means — retuning a preset is a client release, not a contract break, and it stays additive under the widening rule. null is uncapped, which is why this is nullable rather than defaulting to a large number: absent and \"as much as you like\" are the same statement, and a numeric sentinel would have to be widened every time hardware improves. The bounds are deliberately loose — 100 kbps is below any watchable stream and 200 Mbps is above any remux — because this caps a preference, not a policy; entitlement limits live in internal/policy. Migration decomposes the legacy compound values: 1080p-high becomes (1080p, 10000), 720p-medium becomes (720p, 3000), 420p becomes (480p, 720), following the bitrates in web/src/player/hooks/useTranscodeQuality.ts, so no stored preference is lost to the rejects table."
},
{
"key": "playback.auto_skip_intro",
@@ -615,6 +635,96 @@
"recommended_control": "select",
"notes": "Moved from account to profile scope; the account row is copied to every profile during migration."
},
{
"key": "ui.card_overlays",
"introduced_in": 1,
"persistence": "remote",
"allowed_scopes": ["profile"],
"resolution_order": ["profile", "default"],
"value_schema": {
"type": "object",
"schema_ref": "card-overlays.json",
"nullable": true
},
"default_value": null,
"platforms": ["web"],
"category": "appearance",
"label": "Poster badges",
"description": "Which badges appear on poster cards, and where.",
"notes": "Registered from the legacy unprefixed key card_overlays, which reached the server only through the unknown-key extension bag — stored as an arbitrary string with no validation. null means the user has expressed no preference, which is what lets the server-wide admin default in the overlay-config endpoint apply; writing a resolved-but-unchosen value would silently pin them. The admin default and the enabled kill switch stay in server_settings and are not user settings."
},
{
"key": "ui.next_up_mode",
"introduced_in": 1,
"persistence": "remote",
"allowed_scopes": ["profile"],
"resolution_order": ["profile", "default"],
"value_schema": {
"type": "enum",
"values": [
{ "value": "combined", "label": "With Continue Watching" },
{ "value": "separate", "label": "Separate row" }
]
},
"default_value": "combined",
"category": "navigation",
"label": "Next up episodes",
"description": "Whether upcoming episodes stay with Continue Watching or get their own row.",
"recommended_control": "select",
"notes": "Registered from the legacy unprefixed key next_up_mode. The server reads it directly when assembling home sections, so it cannot be client-local; that read moves to the canonical resolver at cutover. The legacy value was untyped and absent meant combined, which is why combined is the default rather than a third \"unset\" member."
},
{
"key": "ui.sidebar_pins",
"introduced_in": 1,
"persistence": "remote",
"allowed_scopes": ["profile"],
"resolution_order": ["profile", "default"],
"value_schema": {
"type": "object",
"schema_ref": "sidebar-pins.json",
"nullable": true
},
"default_value": null,
"platforms": ["web"],
"category": "navigation",
"label": "Pinned sidebar items",
"description": "Sections and collections pinned into the sidebar.",
"notes": "Registered from the legacy unprefixed key sidebar_pins. Navigation state rather than an authored preference, so it has no control; it is written by the pin affordances themselves."
},
{
"key": "ui.disabled_library_ids",
"introduced_in": 1,
"persistence": "remote",
"allowed_scopes": ["profile"],
"resolution_order": ["profile", "default"],
"value_schema": {
"type": "object",
"schema_ref": "library-id-list.json",
"nullable": true
},
"default_value": null,
"category": "navigation",
"label": "Hidden libraries",
"description": "Libraries you have hidden from your own browsing.",
"notes": "Registered from the legacy unprefixed key disabled_library_ids. This is the user hiding a library from themselves — it is not an access control. Library visibility enforcement lives in internal/access and internal/policy, and nothing here may be read as a permission. Profile scope rather than profile_device because hiding a library is a statement about what you want to see, not about one screen."
},
{
"key": "ui.library_order",
"introduced_in": 1,
"persistence": "remote",
"allowed_scopes": ["profile"],
"resolution_order": ["profile", "default"],
"value_schema": {
"type": "object",
"schema_ref": "library-id-list.json",
"nullable": true
},
"default_value": null,
"category": "navigation",
"label": "Library order",
"description": "The order your libraries appear in.",
"notes": "Registered from the legacy unprefixed key library_order. Shares library-id-list.json with ui.disabled_library_ids: both are normalized by the same normalizeLibraryIDs in web/src/hooks/queries/libraries.ts, which drops non-integers and duplicates. A library id absent from the list sorts after the ones present, so a stale id for a deleted library is inert and needs no cleanup hook."
},
{
"key": "downloads.wifi_only",
"introduced_in": 1,
contracts/settings/v1/schemas/card-overlays.json
+79
View File
@@ -0,0 +1,79 @@
{
"$schema": "https://json-schema.org/draft/2020-12/schema",
"$id": "https://silo-server.dev/contracts/settings/v1/schemas/card-overlays.json",
"title": "Card overlay preferences",
"description": "Badges painted on poster cards. Mirrors CardOverlayPrefs in web/src/lib/overlays/types.ts.",
"type": "object",
"additionalProperties": false,
"required": ["version", "preset", "order", "items"],
"properties": {
"version": { "const": 2 },
"preset": {
"type": "string",
"enum": ["minimal", "classic", "vibrant", "pill", "square"]
},
"order": {
"description": "Explicit render order. Empty means use the registry's own order.",
"type": "array",
"maxItems": 64,
"items": { "$ref": "#/$defs/overlayId" }
},
"items": {
"type": "object",
"propertyNames": { "$ref": "#/$defs/overlayId" },
"additionalProperties": {
"type": "object",
"additionalProperties": false,
"required": ["enabled", "position"],
"properties": {
"enabled": { "type": "boolean" },
"position": {
"type": "string",
"enum": ["top-left", "top-right", "bottom-left", "bottom-right"]
},
"accentColor": {
"description": "Hex colour. Absent means the overlay's own default accent.",
"type": "string",
"pattern": "^#[0-9a-fA-F]{6}$"
},
"showIcon": {
"description": "Absent means inherit from the preset.",
"type": "boolean"
}
}
}
}
},
"$defs": {
"overlayId": {
"type": "string",
"enum": [
"resolution",
"hdr",
"resolution_hdr",
"audio",
"audio_channels",
"video_codec",
"container",
"aspect_ratio",
"release_type",
"edition",
"multi_audio",
"multi_sub",
"rating_imdb",
"rating_tmdb",
"rating_rt",
"rating_rt_audience",
"content_rating",
"year",
"runtime",
"original_language",
"studio",
"network",
"show_status",
"imdb_top_250",
"rt_certified_fresh"
]
}
}
}
contracts/settings/v1/schemas/library-id-list.json
+13
View File
@@ -0,0 +1,13 @@
{
"$schema": "https://json-schema.org/draft/2020-12/schema",
"$id": "https://silo-server.dev/contracts/settings/v1/schemas/library-id-list.json",
"title": "Library id list",
"description": "An ordered, duplicate-free list of library ids. Backs both ui.disabled_library_ids and ui.library_order; the web client normalizes with normalizeLibraryIDs in web/src/hooks/queries/libraries.ts, which drops non-integers and anything below 1.",
"type": "array",
"maxItems": 512,
"uniqueItems": true,
"items": {
"type": "integer",
"minimum": 1
}
}
contracts/settings/v1/schemas/sidebar-pins.json
+27
View File
@@ -0,0 +1,27 @@
{
"$schema": "https://json-schema.org/draft/2020-12/schema",
"$id": "https://silo-server.dev/contracts/settings/v1/schemas/sidebar-pins.json",
"title": "Sidebar pins",
"description": "Sections and collections pinned into the sidebar, grouped by the library they belong to. Mirrors SidebarPins in web/src/api/types.ts.",
"type": "object",
"propertyNames": {
"description": "The group the pins sit under — a library id, or a well-known group name.",
"type": "string",
"maxLength": 64
},
"maxProperties": 512,
"additionalProperties": {
"type": "array",
"maxItems": 128,
"items": {
"type": "object",
"additionalProperties": false,
"required": ["type", "id", "label"],
"properties": {
"type": { "type": "string", "enum": ["section", "collection"] },
"id": { "type": "string", "minLength": 1, "maxLength": 128 },
"label": { "type": "string", "maxLength": 256 }
}
}
}
}
internal/settingscontract/contract_test.go
+143
View File
@@ -95,6 +95,16 @@ func TestEveryCurrentServerKeyIsRegistered(t *testing.T) {
"player.sleep_timer_default_minutes": "player.sleep_timer_default_minutes",
"player.next_up_prompt_seconds": keyNextUpPrompt,
// Unregistered keys the web client writes through the extension bag.
// Two of these the server also reads back, so they cannot be demoted to
// client-local state: next_up_mode decides home section assembly, and
// card_overlays falls back to a server-wide admin default.
"card_overlays": "ui.card_overlays",
"next_up_mode": "ui.next_up_mode",
"sidebar_pins": "ui.sidebar_pins",
"disabled_library_ids": "ui.disabled_library_ids",
"library_order": "ui.library_order",
// Profile columns that become settings.
"user_profiles.language": keyAudioLanguage,
"user_profiles.subtitle_language": "playback.subtitle_language",
@@ -1345,3 +1355,136 @@ func TestLibraryPageStateAcceptsTheWebClientsRealSearchStrings(t *testing.T) {
t.Error("an 8KiB search string was accepted; the bound is not enforced")
}
}
// TestObjectSchemasAcceptTheShapesClientsStore exercises every schema_ref
// against a real value.
//
// Nothing else does. Each of these definitions is nullable with a null default,
// so TestDefaultsValidateAgainstTheirOwnSchema returns at the null branch
// without ever compiling the reference — a schema_ref naming a file that does
// not exist, or a schema that rejects the shape its client actually stores,
// would pass every other test in this file.
func TestObjectSchemasAcceptTheShapesClientsStore(t *testing.T) {
manifest, err := Load()
if err != nil {
t.Fatalf("loading manifest: %v", err)
}
valid := map[string]string{
"ui.card_overlays": `{"version":2,"preset":"minimal","order":["hdr","year"],` +
`"items":{"hdr":{"enabled":true,"position":"top-left","accentColor":"#f5c518"},` +
`"year":{"enabled":false,"position":"bottom-right","showIcon":true}}}`,
"ui.sidebar_pins": `{"7":[{"type":"section","id":"recently-added","label":"Recently Added"}],` +
`"12":[{"type":"collection","id":"c-9","label":"Marvel"}]}`,
"ui.disabled_library_ids": `[3,9]`,
"ui.library_order": `[9,3,1]`,
"playback.subtitle_appearance": `{"fontSize":"large","position":"top"}`,
"ui.custom_theme_vars": `{"color-bg":"#101014"}`,
}
for key, value := range valid {
t.Run(key, func(t *testing.T) {
def, ok := manifest.Lookup(key)
if !ok {
t.Fatalf("%s is not registered", key)
}
if err := def.ValueSchema.ValidateValue(json.RawMessage(value), objSchemas); err != nil {
t.Fatalf("a value the client stores was rejected: %v", err)
}
})
}
// Each schema must actually constrain something, or it is decoration.
invalid := map[string]string{
"ui.card_overlays": `{"version":2,"preset":"nonesuch","order":[],"items":{}}`,
"ui.sidebar_pins": `{"7":[{"type":"bogus","id":"x","label":"L"}]}`,
"ui.disabled_library_ids": `[0,-1]`,
"ui.library_order": `[1,1]`,
}
for key, value := range invalid {
t.Run(key+" rejects", func(t *testing.T) {
def, ok := manifest.Lookup(key)
if !ok {
t.Fatalf("%s is not registered", key)
}
if err := def.ValueSchema.ValidateValue(json.RawMessage(value), objSchemas); err == nil {
t.Fatalf("%s was accepted", value)
}
})
}
}
// TestQualityIsTwoIndependentAxes pins the split that replaced the compound
// ladder values. A client composes its own presets from these two, so the
// contract must not constrain them jointly.
func TestQualityIsTwoIndependentAxes(t *testing.T) {
manifest, err := Load()
if err != nil {
t.Fatalf("loading manifest: %v", err)
}
quality, ok := manifest.Lookup(keyPreferredQuality)
if !ok {
t.Fatal("playback.preferred_quality is not registered")
}
bitrate, ok := manifest.Lookup("playback.max_bitrate_kbps")
if !ok {
t.Fatal("playback.max_bitrate_kbps is not registered")
}
// The resolution axis carries no bitrate spellings.
for _, member := range quality.ValueSchema.Values {
if s, isString := member.Value.(string); isString && strings.Contains(s, "-") {
t.Errorf("quality member %q looks like a compound ladder rung; "+
"bitrate belongs to playback.max_bitrate_kbps", s)
}
}
// Uncapped has to be expressible, or every client invents a sentinel.
if !bitrate.ValueSchema.Nullable {
t.Error("max_bitrate_kbps must be nullable so uncapped is a real value")
}
if string(bitrate.DefaultValue) != jsonNull {
t.Errorf("max_bitrate_kbps defaults to %s, want null (uncapped)", bitrate.DefaultValue)
}
// Both axes must resolve identically, or a device override of one and a
// profile value of the other would compose into a pair the user never chose.
if len(quality.ResolutionOrder) != len(bitrate.ResolutionOrder) {
t.Fatalf("resolution orders differ: %v vs %v",
quality.ResolutionOrder, bitrate.ResolutionOrder)
}
for i := range quality.ResolutionOrder {
if quality.ResolutionOrder[i] != bitrate.ResolutionOrder[i] {
t.Errorf("resolution orders differ at %d: %q vs %q",
i, quality.ResolutionOrder[i], bitrate.ResolutionOrder[i])
}
}
// The legacy compound values decompose losslessly; this is the table the
// migration implements.
for _, tc := range []struct {
legacy string
resolution string
kbps int
}{
{"1080p-high", "1080p", 10000},
{"1080p", "1080p", 6000},
{"1080p-8", "1080p", 6000},
{"720p-high", "720p", 4000},
{"720p-medium", "720p", 3000},
{"720p", "720p", 2000},
{"480p", "480p", 1500},
{"420p", "480p", 720},
{"328p", "480p", 720},
} {
t.Run("decompose "+tc.legacy, func(t *testing.T) {
res, _ := json.Marshal(tc.resolution)
if err := quality.ValueSchema.ValidateValue(res, objSchemas); err != nil {
t.Errorf("resolution %q is not a member: %v", tc.resolution, err)
}
kbps, _ := json.Marshal(tc.kbps)
if err := bitrate.ValueSchema.ValidateValue(kbps, objSchemas); err != nil {
t.Errorf("bitrate %d is out of range: %v", tc.kbps, err)
}
})
}
}