Skip to content

Commit d49b59c

Browse files
jopemachineclaude
andauthored
feat(BA-6628): move fragment rank to the allow list with scope defaults (#12516)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 824b272 commit d49b59c

19 files changed

Lines changed: 268 additions & 155 deletions

File tree

changes/12516.feature.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Move the app config merge `rank` from `app_config_fragments` to the admin-owned `app_config_allow_list` with per-scope-type defaults (public=100, domain=200, user=300)

proposals/BEP-1052-scoped-app-config-redesign.md

Lines changed: 50 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@ each managed independently.
1919

2020
**Reads are the hot path.** A user's effective configuration for a
2121
`config_name` is the **deep merge of every fragment that applies to
22-
them**, taken
23-
straight from `app_config_fragments` in `rank` order — a single-table
24-
query with **no joins and no permission lookup**.
22+
them**, taken from `app_config_fragments` ordered by the `rank` of each
23+
fragment's allow-list entry — one indexed join, **no permission
24+
lookup**.
2525

2626
**Write authorization is set up ahead of time.** Every config name is
2727
**explicitly registered** in `app_config_definitions`, and `app_config_allow_list`
@@ -68,18 +68,20 @@ Three scopes cover the use cases (`public` for the pre-login shell):
6868
- **Explicitly registered names.** Every config name lives in
6969
`app_config_definitions`; fragments and allow-list entries reference it by
7070
foreign key. No fragment may exist for an unregistered `config_name`.
71-
- **Reads are join-free and unconditional.** The merge reads
72-
`app_config_fragments` alone, ordering the existing fragments by
73-
`rank`. No allow-list or policy is consulted at read time — the hot
74-
path stays a single indexed scan.
71+
- **Reads are unconditional.** The merge reads `app_config_fragments`
72+
joined to `app_config_allow_list` only for each fragment's `rank` — an
73+
indexed `(config_name, scope_type)` join. No permission or policy is
74+
evaluated at read time.
7575
- **Allow-list = the write gate for every fragment.** `app_config_allow_list`
7676
holds **one record per `(config_name, scope_type)`**; a fragment at
7777
that scope may be created/updated/purged **only if** the record exists
7878
— through the admin mutations and the regular ones alike. What sets
7979
admins apart is that they alone manage the allow-list (and the
8080
`app_config_definitions`) itself. It governs **writes only** — never reads.
81-
- **`rank` lives on the fragment.** A fragment's `rank` is its merge
82-
priority within a `config_name`; the read merge orders fragments by it.
81+
- **`rank` lives on the allow-list entry.** Merge priority is an
82+
admin-owned policy: it sits on the (admin-managed) allow-list entry,
83+
not on the fragment — a fragment owner editing their own fragment can
84+
never re-order the merge (see §2).
8385
- **Single source-of-truth table.** One `app_config_fragments` table
8486
holds every scope; only the exposure layer is split.
8587

@@ -109,10 +111,11 @@ Keyed by the natural composite `(scope_type, scope_id, config_name)`
109111
- `scope_id` — the scope's identifier (see convention below).
110112
- `config_name` — FK → `app_config_definitions.config_name`.
111113
- `config` — schema-less JSON payload.
112-
- `rank` — integer merge priority within the `config_name` (low → high;
113-
higher wins). Assigned on create (see §2).
114114
- `created_at` / `updated_at`.
115115

116+
The fragment carries no rank of its own — its merge priority is its
117+
allow-list entry's `rank` (see §2).
118+
116119
### `app_config_allow_list` — the per-`(config_name, scope_type)` write gate
117120

118121
One row per `(config_name, scope_type)` (unique) — a normalized,
@@ -125,12 +128,14 @@ advance.
125128
(`public | domain | user`). A user-overridable config carries a
126129
`(config_name, user)` row; an admin-only value carries
127130
`(config_name, domain)` and/or `(config_name, public)`.
131+
- `rank` — the merge priority every fragment under this entry carries
132+
(low → high; higher wins). Defaults per scope type (see §2); admins
133+
may set it explicitly.
128134
- `created_at` / `updated_at`.
129135

130-
A row's **presence** is the entire signal — there is no `rank` here,
131-
because the allow-list never participates in the merge. It gates **both**
132-
write paths; admins, unlike users, may also create/update/purge the
133-
allow-list rows themselves.
136+
A row's **presence** is the write grant, and its `rank` is the merge
137+
order of its fragments. It gates **both** write paths; admins, unlike
138+
users, may also create/update/purge the allow-list rows themselves.
134139

135140
### Scope-ID convention
136141

@@ -185,7 +190,8 @@ boundary.
185190
merged value is the admin's (`public` / `domain` fragments only).
186191
- **Overridable**: the admin grants `(config_name, user)`. The admin
187192
sets the `domain` default; the user freely creates/updates/purges
188-
their own `user` fragment, which (higher `rank`) wins on merge.
193+
their own `user` fragment, which wins on merge (the `user` entry's
194+
default `rank` is the highest).
189195
- **User-only**: the grant exists and no admin fragment is published.
190196

191197
To promote a fixed value to user-customizable, the admin adds a single
@@ -198,19 +204,25 @@ is already stored).
198204

199205
## 2. `rank` — merge priority
200206

201-
`rank` is the integer priority a **fragment** carries within a
202-
`config_name`; the read merge applies fragments in `rank` order (low →
203-
high, higher wins).
204-
205-
- **Assignment.** A new fragment is placed after the existing ones for
206-
the same `config_name`, so a later-created fragment outranks earlier ones by
207-
default. Admins re-order by setting `rank` explicitly. (Publishing the
208-
`domain` default before a user writes their own copy naturally gives
209-
the `user` fragment the higher rank, so the user value wins.)
210-
- **No tier defaults.** Priority is the fragment's `rank`, not derived
211-
from `scope_type` — a `user` fragment outranks a `domain` fragment
212-
only because its `rank` is higher, not because `user` is "above"
213-
`domain`.
207+
`rank` is the integer priority an **allow-list entry** carries; every
208+
fragment written under the entry merges at that priority (low → high,
209+
higher wins). It lives on the allow-list — not the fragment — because
210+
merge order is admin policy: fragment writes are partly user-owned, and
211+
a rank on the fragment would let a user re-order the merge by editing
212+
their own row.
213+
214+
- **Scope-type defaults.** A new entry defaults its `rank` from its
215+
`scope_type`: `public` = 100, `domain` = 200, `user` = 300. The
216+
defaults order the scopes so a user's own fragment overrides the
217+
domain default, which overrides the public value; the 100 gap leaves
218+
room for custom placement in between.
219+
- **Admin override.** The admin may set `rank` explicitly when creating
220+
the entry — e.g. a `domain` entry ranked above `user` yields a
221+
domain-enforced value that user fragments cannot beat even where a
222+
user grant exists.
223+
- **Per-caller totality.** For one caller and one `config_name`, at most
224+
one fragment applies per scope type (§3), so the entry ranks totally
225+
order every merge input.
214226

215227
---
216228

@@ -219,7 +231,7 @@ high, higher wins).
219231
Two read shapes exist:
220232

221233
- **`AppConfigFragment`** — one raw row, regardless of scope. Carries
222-
`scopeType`, `scopeId`, `configName`, `rank`, and `config`. Callers
234+
`scopeType`, `scopeId`, `configName`, and `config`. Callers
223235
disambiguate scope by reading `scopeType` (no per-scope wrapper
224236
types).
225237
- **`AppConfig`** — the merged per-user view for a `config_name`: the
@@ -235,10 +247,10 @@ those whose scope applies to them:
235247
- the user's `user` fragment (`scope_id = the user's id`).
236248

237249
A single `app_config_fragments` query selects exactly those rows (the
238-
user's domain is known from the session — **no join, no allow-list
239-
lookup**), orders them by `rank` (low → high), and deep-merges: nested
240-
objects recurse, scalars and lists are wholesale-replaced, and the
241-
higher `rank` wins on conflict.
250+
user's domain is known from the session — no permission check), joins
251+
each to its allow-list entry for the `rank`, orders by it (low → high),
252+
and deep-merges: nested objects recurse, scalars and lists are
253+
wholesale-replaced, and the higher `rank` wins on conflict.
242254

243255
**Null projection.** A stored `config` of `{}` reads back as `null`, and
244256
a merged `config` that is empty after combining every fragment is
@@ -276,7 +288,8 @@ likewise `null` — clients fall back to their built-in defaults.
276288
be written only after its scope's row exists.
277289
- **Pre-login public config** — anonymous read of `public` `theme`.
278290
- **Bootstrap after login** — read merged `AppConfig`s in one round of
279-
queries; each is a single-table rank merge, no per-scope stitching.
291+
queries; each is a rank merge over the fragments joined to their
292+
allow-list entries, no per-scope stitching.
280293
- **User edits a config** — a regular create/update/purge on the
281294
caller's own `user` row (only where the grant exists); the response is
282295
the recomputed merge.
@@ -289,7 +302,8 @@ likewise `null` — clients fall back to their built-in defaults.
289302
- **Admin locks a value back down** — remove the `(config_name, user)`
290303
grant **and** purge existing `user` fragments (the grant gates writes,
291304
not reads).
292-
- **Admin reorders contributions** — adjust fragment `rank`s.
305+
- **Admin reorders contributions** — set the allow-list entries'
306+
`rank`s (per `(config_name, scope_type)`, not per fragment).
293307
- **Admin retires a config name** — purge the fragments, then the
294308
allow-list entries, then the `app_config_definitions` row (purge is rejected
295309
while references remain).

src/ai/backend/common/data/app_config/types.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,3 +41,19 @@ def to_rbac_scope_id(self, scope_id: str) -> str:
4141
their own ``scope_id``.
4242
"""
4343
return "" if self is AppConfigScopeType.PUBLIC else scope_id
44+
45+
def default_rank(self) -> int:
46+
"""Default merge rank for an allow-list entry at this scope type (BEP-1052).
47+
48+
The merge applies fragments in rank order (low → high; higher wins), so the
49+
defaults order the scopes as ``public`` < ``domain`` < ``user`` — a user's own
50+
fragment overrides the domain default, which overrides the public value. The
51+
100 gap leaves room for admins to place custom ranks in between.
52+
"""
53+
match self:
54+
case AppConfigScopeType.PUBLIC:
55+
return 100
56+
case AppConfigScopeType.DOMAIN:
57+
return 200
58+
case AppConfigScopeType.USER:
59+
return 300

src/ai/backend/manager/data/app_config_allow_list/types.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,16 @@
99

1010
@dataclass(frozen=True)
1111
class AppConfigAllowListData:
12-
"""Domain data for one app config allow-list entry — a per-``(config_name, scope_type)`` write gate."""
12+
"""Domain data for one app config allow-list entry — a per-``(config_name, scope_type)`` write gate.
13+
14+
``rank`` is the merge priority applied to every fragment written under the entry
15+
(low → high; higher wins).
16+
"""
1317

1418
id: AppConfigAllowListID
1519
config_name: str
1620
scope_type: AppConfigScopeType
21+
rank: int
1722
created_at: datetime
1823
updated_at: datetime
1924

src/ai/backend/manager/data/app_config_fragment/types.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ class AppConfigFragmentData:
1616
config_name: str
1717
scope_type: AppConfigScopeType
1818
scope_id: str
19-
rank: int
2019
config: dict[str, Any]
2120
created_at: datetime
2221
updated_at: datetime
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
"""move fragment rank to app_config_allow_list
2+
3+
Move the merge-priority ``rank`` from ``app_config_fragments`` to
4+
``app_config_allow_list`` (BEP-1052). Fragment writes are partly user-owned, so
5+
a rank on the fragment would let a fragment owner re-order the merge; the
6+
allow-list entry is admin-owned, making it the right carrier for merge
7+
priority. Existing allow-list rows are backfilled with the per-scope-type
8+
default ranks (public=100, domain=200, user=300).
9+
10+
Revision ID: 66d0f891ed20
11+
Revises: ba6615a4d2f1
12+
Create Date: 2026-07-03
13+
14+
"""
15+
16+
import sqlalchemy as sa
17+
from alembic import op
18+
19+
# revision identifiers, used by Alembic.
20+
revision = "66d0f891ed20"
21+
down_revision = "ba6615a4d2f1"
22+
# Part of: NEXT_RELEASE_VERSION
23+
branch_labels = None
24+
depends_on = None
25+
26+
27+
def upgrade() -> None:
28+
# Allow-list entries carry the merge rank; backfill by scope-type default.
29+
op.add_column("app_config_allow_list", sa.Column("rank", sa.Integer(), nullable=True))
30+
op.execute(
31+
sa.text(
32+
"""
33+
UPDATE app_config_allow_list
34+
SET rank = CASE scope_type
35+
WHEN 'public' THEN 100
36+
WHEN 'domain' THEN 200
37+
WHEN 'user' THEN 300
38+
END
39+
WHERE rank IS NULL
40+
"""
41+
)
42+
)
43+
op.alter_column("app_config_allow_list", "rank", nullable=False)
44+
op.drop_column("app_config_fragments", "rank")
45+
46+
47+
def downgrade() -> None:
48+
op.add_column("app_config_fragments", sa.Column("rank", sa.Integer(), nullable=True))
49+
# Restore each fragment's rank from its allow-list entry. Fragments whose entry
50+
# was removed fall back to the scope-type default; fragments sharing a
51+
# ``(config_name, scope_type)`` end up with equal ranks — the pre-move
52+
# per-fragment next-value ranks are not recoverable.
53+
op.execute(
54+
sa.text(
55+
"""
56+
UPDATE app_config_fragments f
57+
SET rank = al.rank
58+
FROM app_config_allow_list al
59+
WHERE al.config_name = f.config_name
60+
AND al.scope_type = f.scope_type
61+
"""
62+
)
63+
)
64+
op.execute(
65+
sa.text(
66+
"""
67+
UPDATE app_config_fragments
68+
SET rank = CASE scope_type
69+
WHEN 'public' THEN 100
70+
WHEN 'domain' THEN 200
71+
WHEN 'user' THEN 300
72+
END
73+
WHERE rank IS NULL
74+
"""
75+
)
76+
)
77+
op.alter_column("app_config_fragments", "rank", nullable=False)
78+
op.drop_column("app_config_allow_list", "rank")

src/ai/backend/manager/models/app_config_allow_list/orders.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ def scope_type(ascending: bool = True) -> QueryOrder:
2727
return AppConfigAllowListRow.scope_type.asc()
2828
return AppConfigAllowListRow.scope_type.desc()
2929

30+
@staticmethod
31+
def rank(ascending: bool = True) -> QueryOrder:
32+
if ascending:
33+
return AppConfigAllowListRow.rank.asc()
34+
return AppConfigAllowListRow.rank.desc()
35+
3036
@staticmethod
3137
def created_at(ascending: bool = True) -> QueryOrder:
3238
if ascending:

src/ai/backend/manager/models/app_config_allow_list/row.py

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@ class AppConfigAllowListRow(Base): # type: ignore[misc]
1919
"""Permission to write config fragments for one config name at one scope type.
2020
2121
A config fragment may be created, updated, or purged only when a matching row
22-
exists here; without one, the write is rejected. Reads never consult this table.
23-
There is at most one row per ``(config_name, scope_type)``, and only admins
24-
create or remove these rows.
22+
exists here; without one, the write is rejected. ``rank`` is the merge priority
23+
the entry's fragments carry (low → high; higher wins) — admin-owned here so
24+
fragment owners cannot re-order the merge. There is at most one row per
25+
``(config_name, scope_type)``, and only admins create or remove these rows.
2526
"""
2627

2728
__tablename__ = "app_config_allow_list"
@@ -48,6 +49,11 @@ class AppConfigAllowListRow(Base): # type: ignore[misc]
4849
StrEnumType(AppConfigScopeType),
4950
nullable=False,
5051
)
52+
rank: Mapped[int] = mapped_column(
53+
"rank",
54+
sa.Integer,
55+
nullable=False,
56+
)
5157
created_at: Mapped[datetime] = mapped_column(
5258
"created_at",
5359
sa.DateTime(timezone=True),
@@ -67,6 +73,7 @@ def to_data(self) -> AppConfigAllowListData:
6773
id=self.id,
6874
config_name=self.config_name,
6975
scope_type=self.scope_type,
76+
rank=self.rank,
7077
created_at=self.created_at,
7178
updated_at=self.updated_at,
7279
)

src/ai/backend/manager/models/app_config_fragment/orders.py

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,6 @@ def scope_id(ascending: bool = True) -> QueryOrder:
3333
return AppConfigFragmentRow.scope_id.asc()
3434
return AppConfigFragmentRow.scope_id.desc()
3535

36-
@staticmethod
37-
def rank(ascending: bool = True) -> QueryOrder:
38-
if ascending:
39-
return AppConfigFragmentRow.rank.asc()
40-
return AppConfigFragmentRow.rank.desc()
41-
4236
@staticmethod
4337
def created_at(ascending: bool = True) -> QueryOrder:
4438
if ascending:

src/ai/backend/manager/models/app_config_fragment/row.py

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,11 @@
1818

1919

2020
class AppConfigFragmentRow(Base): # type: ignore[misc]
21-
"""One scoped app config fragment — a single JSON document at ``(config_name, scope_type, scope_id)``."""
21+
"""One scoped app config fragment — a single JSON document at ``(config_name, scope_type, scope_id)``.
22+
23+
The fragment carries no merge priority of its own — its rank is the ``rank`` of
24+
the allow-list entry for its ``(config_name, scope_type)``.
25+
"""
2226

2327
__tablename__ = "app_config_fragments"
2428
__table_args__ = (
@@ -52,11 +56,6 @@ class AppConfigFragmentRow(Base): # type: ignore[misc]
5256
sa.String(length=255),
5357
nullable=False,
5458
)
55-
rank: Mapped[int] = mapped_column(
56-
"rank",
57-
sa.Integer,
58-
nullable=False,
59-
)
6059
config: Mapped[dict[str, Any]] = mapped_column(
6160
"config",
6261
JSONB,
@@ -82,7 +81,6 @@ def to_data(self) -> AppConfigFragmentData:
8281
config_name=self.config_name,
8382
scope_type=self.scope_type,
8483
scope_id=self.scope_id,
85-
rank=self.rank,
8684
config=self.config,
8785
created_at=self.created_at,
8886
updated_at=self.updated_at,

0 commit comments

Comments
 (0)