From 1f01aed5fa6ae5292e32084a2a2f996e79954d3b Mon Sep 17 00:00:00 2001 From: Sebastian Stehle Date: Mon, 24 Aug 2026 18:42:06 +0200 Subject: [PATCH] More fixes --- .../Apps/Roles.cs | 29 ++- .../Assets/Queries/Steps/CalculateTokens.cs | 26 ++- .../Contents/ContentsOptions.cs | 5 + .../GraphQL/CachingGraphQLResolver.cs | 7 +- .../Contents/Queries/ContentQueryParser.cs | 2 +- .../Contents/Queries/Steps/CalculateTokens.cs | 31 +++- .../Jobs/JobWorker.cs | 29 ++- backend/src/Squidex/appsettings.json | 11 +- .../Assets/Queries/CalculateTokensTests.cs | 21 +++ resolved.md | 167 +++++++++++++++++- todo.md | 47 ++--- 11 files changed, 319 insertions(+), 56 deletions(-) diff --git a/backend/src/Squidex.Domain.Apps.Core.Model/Apps/Roles.cs b/backend/src/Squidex.Domain.Apps.Core.Model/Apps/Roles.cs index 58c30bfcc..379d5f4ff 100644 --- a/backend/src/Squidex.Domain.Apps.Core.Model/Apps/Roles.cs +++ b/backend/src/Squidex.Domain.Apps.Core.Model/Apps/Roles.cs @@ -5,6 +5,7 @@ // All rights reserved. Licensed under the MIT license. // ========================================================================== +using System.Collections.Concurrent; using System.Diagnostics.CodeAnalysis; using System.Diagnostics.Contracts; using Squidex.Infrastructure; @@ -17,6 +18,8 @@ namespace Squidex.Domain.Apps.Core.Apps; public sealed class Roles { + private const int MaxResolved = 1000; + private readonly ConcurrentDictionary<(string App, string Name, bool IsFrontend), Role?> resolved = new ConcurrentDictionary<(string, string, bool), Role?>(); private readonly ReadonlyDictionary inner; public static readonly IReadOnlyDictionary Defaults = new Dictionary @@ -159,18 +162,34 @@ public sealed class Roles { Guard.NotNull(app); - value = null!; + // Resolving a role builds a permission for every permission of the role, but the result only + // depends on the key and the roles are immutable, so it is only done once. This is called for + // every request. + value = resolved.GetOrAdd((app, name, isFrontend), static (key, self) => self.Resolve(key.App, key.Name, key.IsFrontend), this)!; + + return value != null; + } + + private Role? Resolve(string app, string name, bool isFrontend) + { + // Apps without custom roles share the same empty instance, so this cache is not bound to a + // single app and could grow with the number of apps. Start over when it gets too large. + if (resolved.Count >= MaxResolved) + { + resolved.Clear(); + } if (Defaults.TryGetValue(name, out var role)) { - value = role.ForApp(app, isFrontend && name != Role.Owner); + return role.ForApp(app, isFrontend && name != Role.Owner); } - else if (inner.TryGetValue(name, out role)) + + if (inner.TryGetValue(name, out role)) { - value = role.ForApp(app, isFrontend); + return role.ForApp(app, isFrontend); } - return value != null; + return null; } private static string WithoutPrefix(string permission) diff --git a/backend/src/Squidex.Domain.Apps.Entities/Assets/Queries/Steps/CalculateTokens.cs b/backend/src/Squidex.Domain.Apps.Entities/Assets/Queries/Steps/CalculateTokens.cs index 7a9ac2f8e..bf0b2b577 100644 --- a/backend/src/Squidex.Domain.Apps.Entities/Assets/Queries/Steps/CalculateTokens.cs +++ b/backend/src/Squidex.Domain.Apps.Entities/Assets/Queries/Steps/CalculateTokens.cs @@ -5,6 +5,7 @@ // All rights reserved. Licensed under the MIT license. // ========================================================================== +using System.Text.Json.Serialization; using Squidex.Domain.Apps.Core; using Squidex.Infrastructure.Json; @@ -12,6 +13,19 @@ namespace Squidex.Domain.Apps.Entities.Assets.Queries.Steps; public sealed class CalculateTokens(IUrlGenerator urlGenerator, IJsonSerializer serializer) : IAssetEnricherStep { + // We have to use these short names here because they are later read like this. + private sealed class Token + { + [JsonPropertyName("a")] + public string App { get; set; } + + [JsonPropertyName("i")] + public string Id { get; set; } + + [JsonPropertyName("u")] + public string Url { get; set; } + } + public Task EnrichAsync(Context context, IEnumerable assets, CancellationToken ct) { @@ -20,17 +34,13 @@ public sealed class CalculateTokens(IUrlGenerator urlGenerator, IJsonSerializer return Task.CompletedTask; } - var url = urlGenerator.Root(); + // Only the ID is different for each asset, so the token is reused for all of them. + var token = new Token { Url = urlGenerator.Root() }; foreach (var asset in assets) { - // We have to use these short names here because they are later read like this. - var token = new - { - a = asset.AppId.Name, - i = asset.Id.ToString(), - u = url, - }; + token.App = asset.AppId.Name; + token.Id = asset.Id.ToString(); var json = serializer.SerializeToBytes(token); diff --git a/backend/src/Squidex.Domain.Apps.Entities/Contents/ContentsOptions.cs b/backend/src/Squidex.Domain.Apps.Entities/Contents/ContentsOptions.cs index 13d98283d..bc266771f 100644 --- a/backend/src/Squidex.Domain.Apps.Entities/Contents/ContentsOptions.cs +++ b/backend/src/Squidex.Domain.Apps.Entities/Contents/ContentsOptions.cs @@ -19,6 +19,11 @@ public sealed class ContentsOptions public int MaxResults { get; set; } = 200; + // The number of IDs to fetch from the full text index. The index and the content store can be + // different databases, so the IDs have to be loaded to filter and sort them in the store. A + // query with more matches than this limit silently loses the rest. + public int MaxFullTextResults { get; set; } = 1000; + public string? CDN { get; set; } public TimeSpan TimeoutFind { get; set; } = TimeSpan.FromSeconds(1); diff --git a/backend/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/CachingGraphQLResolver.cs b/backend/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/CachingGraphQLResolver.cs index 995bef85f..99d3f2e92 100644 --- a/backend/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/CachingGraphQLResolver.cs +++ b/backend/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/CachingGraphQLResolver.cs @@ -63,8 +63,11 @@ public sealed class CachingGraphQLResolver( return CreateModelAsync(app); } - // A tuple can hold the version as it is, so it does not have to be formatted first. - var cacheKey = (typeof(CachingGraphQLResolver), app.Id, app.Version); + // The version is not part of the key. Building the schema is expensive and the version + // changes for every app event, most of which do not affect the schema at all. The validator + // below detects the changes that do, because SchemasHashKey contains the app version as + // well as the version of every schema. + var cacheKey = (typeof(CachingGraphQLResolver), app.Id); return cache.GetOrCreateAsync(cacheKey, options.CacheDuration, async entry => { diff --git a/backend/src/Squidex.Domain.Apps.Entities/Contents/Queries/ContentQueryParser.cs b/backend/src/Squidex.Domain.Apps.Entities/Contents/Queries/ContentQueryParser.cs index 70c95302a..60f9fa1de 100644 --- a/backend/src/Squidex.Domain.Apps.Entities/Contents/Queries/ContentQueryParser.cs +++ b/backend/src/Squidex.Domain.Apps.Entities/Contents/Queries/ContentQueryParser.cs @@ -90,7 +90,7 @@ public class ContentQueryParser( var searchFilters = new List>(); - var textQuery = new TextQuery(query.FullText, 1000) + var textQuery = new TextQuery(query.FullText, options.MaxFullTextResults) { PreferredSchemaId = schema.Id, }; diff --git a/backend/src/Squidex.Domain.Apps.Entities/Contents/Queries/Steps/CalculateTokens.cs b/backend/src/Squidex.Domain.Apps.Entities/Contents/Queries/Steps/CalculateTokens.cs index 018967d78..edcc1b117 100644 --- a/backend/src/Squidex.Domain.Apps.Entities/Contents/Queries/Steps/CalculateTokens.cs +++ b/backend/src/Squidex.Domain.Apps.Entities/Contents/Queries/Steps/CalculateTokens.cs @@ -5,6 +5,7 @@ // All rights reserved. Licensed under the MIT license. // ========================================================================== +using System.Text.Json.Serialization; using Squidex.Domain.Apps.Core; using Squidex.Infrastructure.Json; @@ -12,21 +13,33 @@ namespace Squidex.Domain.Apps.Entities.Contents.Queries.Steps; public sealed class CalculateTokens(IUrlGenerator urlGenerator, IJsonSerializer serializer) : IContentEnricherStep { + // We have to use these short names here because they are later read like this. + private sealed class Token + { + [JsonPropertyName("a")] + public string App { get; set; } + + [JsonPropertyName("s")] + public string Schema { get; set; } + + [JsonPropertyName("i")] + public string Id { get; set; } + + [JsonPropertyName("u")] + public string Url { get; set; } + } + public Task EnrichAsync(Context context, IEnumerable contents, ProvideSchema schemas, CancellationToken ct) { - var url = urlGenerator.Root(); + // Only the schema and the ID are different for each content, so the token is reused. + var token = new Token { Url = urlGenerator.Root() }; foreach (var content in contents) { - // We have to use these short names here because they are later read like this. - var token = new - { - a = content.AppId.Name, - s = content.SchemaId.Name, - i = content.Id.ToString(), - u = url, - }; + token.Id = content.Id.ToString(); + token.App = content.AppId.Name; + token.Schema = content.SchemaId.Name; var json = serializer.SerializeToBytes(token); diff --git a/backend/src/Squidex.Domain.Apps.Entities/Jobs/JobWorker.cs b/backend/src/Squidex.Domain.Apps.Entities/Jobs/JobWorker.cs index 9b8a2664c..e3c2e055b 100644 --- a/backend/src/Squidex.Domain.Apps.Entities/Jobs/JobWorker.cs +++ b/backend/src/Squidex.Domain.Apps.Entities/Jobs/JobWorker.cs @@ -70,16 +70,37 @@ public sealed class JobWorker : private Task GetJobProcessorAsync(DomainId appId) { + Task processor; lock (processors) { - return processors.GetOrAdd(appId, async key => + processor = processors.GetOrAdd(appId, async key => { - var processor = processorFactory(key); + var loaded = processorFactory(key); - await processor.LoadAsync(default); + await loaded.LoadAsync(default); - return processor; + return loaded; }); } + + try + { + return await processor; + } + catch + { + // A failed attempt must not stay in the cache. Loading can fail for a transient reason + // and the jobs of the app would never run again. Only remove our own entry, so that a + // newer successful one is not thrown away. + lock (processors) + { + if (processors.TryGetValue(appId, out var current) && ReferenceEquals(current, processor)) + { + processors.Remove(appId); + } + } + + throw; + } } } diff --git a/backend/src/Squidex/appsettings.json b/backend/src/Squidex/appsettings.json index 7400e8b64..35d4ff5c3 100644 --- a/backend/src/Squidex/appsettings.json +++ b/backend/src/Squidex/appsettings.json @@ -331,10 +331,19 @@ "defaultPageSize": 200, // The maximum number of items to return for each query. - // + // // Warning: Use pagination and not large number of items. "maxResults": 200, + // The maximum number of IDs to load from the full text index for a single query. + // + // The full text index can be a different database than the content store, therefore the IDs + // have to be loaded to filter and sort them in the content store. A query that matches more + // items than this limit loses the rest of them. + // + // Warning: Increasing this makes full text queries slower and use more memory. + "maxFullTextResults": 1000, + // The timeout when searching for single items in the database. "timeoutFind": "00:00:01", diff --git a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Assets/Queries/CalculateTokensTests.cs b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Assets/Queries/CalculateTokensTests.cs index d950bfe6c..53812bdd8 100644 --- a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Assets/Queries/CalculateTokensTests.cs +++ b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Assets/Queries/CalculateTokensTests.cs @@ -5,7 +5,9 @@ // All rights reserved. Licensed under the MIT license. // ========================================================================== +using System.Text; using Squidex.Domain.Apps.Core; +using Squidex.Domain.Apps.Core.TestHelpers; using Squidex.Domain.Apps.Entities.Assets.Queries.Steps; using Squidex.Domain.Apps.Entities.TestHelpers; using Squidex.Infrastructure.Json; @@ -58,4 +60,23 @@ public class CalculateTokensTests : GivenContext A.CallTo(() => urlGenerator.Root()) .MustHaveHappened(); } + + [Fact] + public async Task Should_compute_ui_token_with_stable_format() + { + var asset = CreateAsset(); + + A.CallTo(() => urlGenerator.Root()) + .Returns("https://squidex.io"); + + var target = new CalculateTokens(urlGenerator, TestUtils.DefaultSerializer); + + await target.EnrichAsync(ApiContext, [asset], CancellationToken); + + var actual = Encoding.UTF8.GetString(Convert.FromBase64String(asset.EditToken!)); + + var expected = $$"""{"a":"{{asset.AppId.Name}}","i":"{{asset.Id}}","u":"https://squidex.io"}"""; + + Assert.Equal(expected, actual); + } } diff --git a/resolved.md b/resolved.md index 6b21ad948..cd3016c4f 100644 --- a/resolved.md +++ b/resolved.md @@ -3,9 +3,9 @@ Items from the backend performance review that are done. Numbering matches [todo.md](todo.md) — resolved items keep their original number so references stay valid. -Most entries are fixes. Items **6** and **14** are closed as *accepted* and item **21** as -*rejected*, kept here so they are not re-reported as new findings. Item **18** records a -finding that turned out to be wrong. +Most entries are fixes. Items **6** and **14** are closed as *accepted*, **21** as *rejected* +and **23** as *bounded but not fixed* — kept here so they are not re-reported as new findings. +Item **18** records a finding that turned out to be wrong. --- @@ -1033,3 +1033,164 @@ Worth being precise about why `GetBuffer` is better rather than just "avoids a c the stream spans several blocks. The difference is that the buffer it returns comes from the pool and goes back on dispose, whereas `ToArray` allocates a new GC array every time. `RecyclableMemoryStream` documents `ToArray` as the call to avoid for exactly this reason. + +--- + +### 23. Full-text search loads a fixed 1000 ids — **CLOSED: BOUNDED, NOT FIXED** +`backend/src/Squidex.Domain.Apps.Entities/Contents/Queries/ContentQueryParser.cs:93` +`backend/src/Squidex.Domain.Apps.Entities/Contents/ContentsOptions.cs` +`backend/src/Squidex/appsettings.json` + +**Was:** `new TextQuery(query.FullText, 1000)` — a hardcoded literal. The ids come back and go +into an `In("id", …)` filter, so a query matching more than 1000 items silently loses the +rest, relevance order is discarded, and up to 1000 GUID strings travel to the database on +every page. + +**Now:** the limit is `ContentsOptions.MaxFullTextResults`, configurable as +`contents:maxFullTextResults` and documented in `appsettings.json` with what raising it costs. +Default unchanged at 1000. + +**Why this is closed as bounded rather than fixed.** I proposed paging the text index and +walked it back after working through the constraint: **the full text index and the content +store are separate, independently configured stores**, and any combination is legal — +Mongo+Mongo, Elastic+Mongo, Elastic+SQL, Azure+anything. `MongoContentRepository` takes +`store:mongoDb:contentDatabase` while the text index resolves the default `IMongoDatabase`; +in EF the index is on `AppDbContext` and contents are on `ContentDbContext`. + +So this is a cross-store join, always, and the 1000 is not a magic number — it is the join +buffer. No value for it is correct, because how many survive depends on a filter the index +has never seen. + +Paging the index only works when the index alone decides both membership *and* order. It +does not, in two common cases: + +| Case | Ids that must cross the boundary | +| --- | --- | +| search only, relevance-ordered | the page (~20) | +| search + explicit `$orderby` | all matches | +| search + `$filter` | all matches, or an iterative top-up | + +And the default sort is `lastModified` — `WithSorting` adds it when the caller gives none — +so today's pipeline is already "the 1000 most relevant, displayed newest first", which is +neither. Making paging work would mean changing the default sort for full-text queries to +relevance: a behaviour change, not an optimisation. + +There is also no architectural escape. Pushing the filter into the index means indexing +arbitrary user-filterable fields — reimplementing the query engine on the search side. +Pushing relevance into the content store means the store needs the scores. Either way the +boundary just moves, and because the backends pair arbitrarily you would owe it for every +combination. + +**Left undone, deliberately, and worth knowing about:** a truncated result is still +indistinguishable from a complete one. A caller paging a 5000-hit search gets a confident +wrong total and silently loses the remainder. Returning `total = -1` when the cap is hit — +the codebase's existing "unknown" convention, used by `NoTotal` — would make it visible +without any interface change. Defaulting full-text queries to relevance order is arguably a +bug fix on its own. + +--- + +### 31. Every request rebuilt the caller's permission set — **FIXED** +`backend/src/Squidex.Domain.Apps.Core.Model/Apps/Roles.cs` + +**Was:** `AppResolver` runs on every API request and resolves the caller's role through +`Roles.TryGet` → `Role.ForApp(app, isFrontend)`, which rebuilt the permission set each time: +a prefix `Permission` (three `string.Replace` calls), then a concatenated string and a +`Permission` per role permission, ten more for a frontend caller, plus a `HashSet`, a +`PermissionSet` and a `Role`. + +**Now:** `Roles` memoizes the resolved role in a `ConcurrentDictionary` keyed by +(app, name, isFrontend). `Role` is a record and immutable, so the result is a pure function +of that key. + +**The cache lives on `Roles`, not on `Role`, for two reasons.** `Role` is a `record`, so its +synthesized `Equals`/`GetHashCode` cover every instance field — adding a cache field would +make two logically equal roles compare unequal. And `Roles` instances hang off the cached +`App`, so the natural lifetime is already right. + +**It is bounded, and that is not cosmetic.** `App.Roles` defaults to the *shared static* +`Roles.Empty`, so for every app without custom roles the cache lives on one instance shared +across all tenants and would grow with the number of apps. It is capped at 1000 entries and +cleared wholesale on overflow; hitting the cap degrades to the old behaviour rather than +leaking. An app with its own roles has its own `Roles` instance and never approaches it. + +`Microsoft.Extensions.Caching.Memory` would have been the nicer bound, but +`Squidex.Domain.Apps.Core.Model` is a pure model project with no caching dependency and it +did not seem worth adding one there. + +--- + +### 32. Any app change threw away the whole GraphQL schema — **FIXED** +`backend/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/CachingGraphQLResolver.cs:67` + +**Was:** the cache key was `(typeof(CachingGraphQLResolver), app.Id, app.Version)`. +`app.Version` bumps on *any* app event — a contributor, a client, a role, a setting — so each +one was a cold miss, and the next GraphQL request paid a full `BuildSchema`: a content type, a +result type and a component type per schema, each initialised with a GraphQL field per schema +field, plus queries, mutations and a `FieldMap`. + +**Now:** the key is `(typeof(CachingGraphQLResolver), app.Id)`. + +**The version was redundant, not load-bearing.** The entry is already created with a +validator, and `SchemasHashKey.Create` builds its dictionary starting with +`[app.Id] = app.Version` before adding every schema version. So the app version was in the +validator all along — having it in the key too meant app changes could never *reach* the +validator, they just missed. + +The behavioural difference is which path a change takes: an app-level change now goes through +the validator like a schema change does — the cached schema is served and refreshed — instead +of blocking the next request on a rebuild. That is the same eventual-consistency trade the +design already makes for schema changes, which are the more visible ones. + +--- + +### 33. Asset and content tokens serialized an object per item — **FIXED** +`backend/src/Squidex.Domain.Apps.Entities/Assets/Queries/Steps/CalculateTokens.cs` +`backend/src/Squidex.Domain.Apps.Entities/Contents/Queries/Steps/CalculateTokens.cs` + +**Was:** both steps allocated a fresh anonymous object per item to hold the edit token, when +only one or two of its fields actually vary: + +```csharp +foreach (var asset in assets) +{ + var token = new { a = asset.AppId.Name, i = asset.Id.ToString(), u = url }; + + asset.EditToken = Convert.ToBase64String(serializer.SerializeToBytes(token)); +} +``` + +**Now:** a private `Token` class is created once per call and its properties are assigned per +item. The short wire names are kept with `[JsonPropertyName]`, so the properties can have +readable names without changing the format. + +**There is a content version of this too**, which the original finding missed — it carries a +fourth field (`s`, the schema name) and sits on the content list path, which is hotter than +the asset one. Both are fixed. + +**The wire format is load-bearing and was pinned first.** The token is base64 of a JSON object +with single-letter keys, decoded by the frontend, and the existing tests only asserted +`EditToken != null` — nothing covered the shape. So a test asserting the exact decoded string +was added and confirmed green against the *old* code before the change, then again after: + +```csharp +var expected = $$"""{"a":"{{asset.AppId.Name}}","i":"{{asset.Id}}","u":"https://squidex.io"}"""; +``` + +This removes the per-item object allocation, not the per-item serialization — the serializer +still runs once per item. Emitting the constant prefix once and varying only the id would go +further, at the cost of hand-writing JSON. + +--- + +### 35. `JobWorker` cached a faulted task for the process lifetime — **FIXED** +`backend/src/Squidex.Domain.Apps.Entities/Jobs/JobWorker.cs` + +**Was:** `processors.GetOrAdd(appId, async key => …)` stored the `Task`, so a +transient failure in `LoadAsync` was cached permanently — jobs for that app never ran again, +and the failure was invisible because every caller saw the *same* exception rather than a new +one. The same defect as item 15. + +**Now:** the entry is removed when the task faults, comparing by reference so a newer +successful entry added by another caller is not discarded. The removal takes the same lock +that guards the dictionary. diff --git a/todo.md b/todo.md index bb882cb9a..bb1ceacfc 100644 --- a/todo.md +++ b/todo.md @@ -9,9 +9,9 @@ overhead / allocation churn), **S4** low (worth fixing while nearby). Item numbers are stable and never reused. Completed items move to [resolved.md](resolved.md) keeping their number, so gaps in the sequence here are -expected — items **4**–**22** and **24**–**30** are closed and live there. +expected — items **4**–**33** and **35** are closed and live there. -**Status: 4 open of 30 — items 1, 2, 3 (one root cause) and 23. The other 26 are in [resolved.md](resolved.md).** +**Status: 4 open of 35 — items 1, 2, 3 (one root cause) and 34. The other 31 are in [resolved.md](resolved.md).** --- @@ -66,45 +66,46 @@ already isolated in `ContentScriptVars`. --- -## S2 — High +## S3 — Moderate -### 23. Full-text search inlines up to 1000 ids into a MongoDB `$in` -`backend/src/Squidex.Domain.Apps.Entities/Contents/Queries/ContentQueryParser.cs:93,107` +### 34. Message formatting looks up properties by reflection on every call +`backend/src/Squidex.Infrastructure/Translations/ResourcesLocalizer.cs:77` + +`ResourcesLocalizer.Get` substitutes `{variable}` placeholders by reflecting over the +anonymous args object: ```csharp -var textQuery = new TextQuery(query.FullText, 1000) { PreferredSchemaId = schema.Id }; -var fullTextIds = await textIndex.SearchAsync(context.App, textQuery, context.Scope(), ct); -... -searchFilters.Add(ClrFilter.In("id", fullTextIds.Select(x => x.ToString()).ToList())); +var property = argsType.GetProperty(variableName); ``` -Every full-text content query becomes: one search round trip, then a second query whose filter -carries up to 1000 GUID strings — roughly 37 KB of BSON — forcing 1000 index seeks. The -`Select(x => x.ToString())` also allocates 1000 strings per query. +No `PropertyInfo` is cached, so every call re-resolves it. `Type.GetProperty(string)` is one +of the slower reflection calls — it does a name lookup over the type's members. + +Mostly this sits on error paths, where it does not matter. The one that is not is +`ResolveReferences.CreateFallback` (`ResolveReferences.cs:125`): + +```csharp +var text = T.Get("contents.listReferences", new { count = referencedContents.Count }); +``` -There is a correctness edge here too, which is why it outranks pure throughput items: -the index returns the top 1000 *by relevance*, but the outer query then re-sorts by the default -`LastModified` and pages over that. Relevance order is discarded, and anything past the 1000 -cap is silently missing — invisible to the caller, who just sees fewer results than exist. +That runs inside the per-content, per-partition loop of the enrichment pipeline, for every +reference field that resolves to more than one item. The same method also allocates a fresh +`JsonObject` and loops over every app language on each call. -**Fix:** push paging into the text index so it returns only the page (plus a total), rather -than a fixed 1000-id prefix that the outer query re-sorts. +**Fix:** cache the `PropertyInfo` per (type, name); a small static dictionary is enough since +the arg types are compiler-generated and few. --- ## Suggested order of attack -1. **Item 23** — full-text paging. Really a correctness fix that happens to also be faster: - the 1000-id cap silently truncates results and discards relevance order today. +1. **Item 34** — small and self-contained: cache the `PropertyInfo` per (type, name). 2. **Items 1–3, engine pooling** — the largest single cost, and the most invasive change on the list. It touches the security boundary of user-authored scripts, since a pooled engine must not carry state from one script into the next. **Profile before writing it**: the estimate that engine construction dominates a scripted content list is read off the loops, not taken from a trace. -Nothing else is outstanding. Everything cheap, every correctness-shaped finding and -everything in the stability category is closed. - --- ## Method / caveats