feat(core): filter discover actions across namespaces - #112
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e4d0e0054
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af26d3f63f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
kunkunGames
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9d5f942f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review All five comments from your pass are addressed, and one follow-up defect found afterwards is fixed in Worth re-checking specifically, since the obvious fix is wrong: exempting registry identifiers from the filler list globally reintroduces the original defect you reported — |
|
Conflict/merge-order note for my six open PRs is in #104 (comment #104 (comment)). Short version: a pairwise trial merge shows these PRs conflict only in |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab60f8424e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 279bab051e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Please re-review the latest head, |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ac77c26b1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Latest head: |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Queued — the gap you identified is real and I am taking it, though in a much smaller form than this PR. Verified: What I am not taking is the ranking engine. Handing an MCP client a filtered candidate list lets its own model rank semantically, which beats six hand-tuned float tiers and a curated English filler list — and the tiers become a permanent maintenance surface ("why did find rank X above Y"). The plan is to extend the no-namespace branch to honour Two things from the review worth passing on regardless of what ships: The sort predicate at
Also taking the |
a8d4fde to
379993c
Compare
kunkunGames
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 379993c3c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
2ea2243 to
5437cdc
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5437cdc43a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
5437cdc to
63be521
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63be521be1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
63be521 to
936e2c2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 936e2c2e8f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
936e2c2 to
0323239
Compare
0323239 to
6d6a988
Compare
Drops everything outside the minimal shape agreed in review: extend the no-namespace branch to honour `filter` plus `offset`/`limit`, reusing the existing predicate and pagination. Removed: - The five response-shaping properties (`_fields`, `_omit`, `_row_fields`, `_path_fields`, `_compact_json`) added to the registered `discover` schema. `FMonolithParamSchema` still accepts them universally; no other registered action advertises them, so advertising them only here was new schema surface rather than a fix. - The proxy-seed synchronization in `Scripts/monolith_proxy.py` and `Tools/MonolithProxy/monolith_proxy.cpp`, including the cached-descriptor overlay. Both files are now byte-identical to master. The offline seed already lags the live schema on master (it predates `filter`, `offset`, `limit`, `detail`, and `verbose`), so this branch neither introduces nor widens that drift; refreshing the seeds belongs to its own change. The `detail=true` test assertion that depended on the removed properties now asserts the reused `filter`/`offset`/`limit` contract instead, so the advertised schema still cannot drift from the handler. Verified at this exact head on both engines: - UE 5.8 (`MonolithPR112ProtectedHost`): editor build Succeeded, `Monolith.Discover` 9/9, 0 failed, exit 0. - UE 5.7 (`MonolithPR112FinalUE57Host`): editor build Succeeded, `Monolith.Discover` 9/9 under `-RenderOffscreen`, 0 failed, exit 0.
|
Rescoped to exactly what you described, in Removed since your comment:
Kept: the no-namespace branch, the Diff against master is 9 files, +430/−132, and the only source file touched besides the new Verified at this exact head on both engines:
The |
The cross-namespace branch answered "which actions match" but not the question that actually blocks a caller: which namespace to open. With a small `limit` that was unanswerable without paging the whole candidate list. The filtered response now also carries `matched_namespaces` — every namespace containing at least one match, each with its `match_count`, in registry order. It is computed before pagination, like `total`, so `limit=1` still describes the whole filtered set. The intended flow is: filter, read `matched_namespaces`, pick a namespace, then call `monolith_discover(namespace="<ns>")`. `match_count` is the size of the filtered set within a namespace, not a relevance score. Namespaces are never ordered by it, and the change introduces no distance function, weight, scoring tier, or curated word list. Semantic ranking stays a client concern, which is the whole point of handing back a bounded candidate list. No new request parameter: this is a response field on an existing mode. The no-argument namespace inventory is unchanged and does not carry it. Verified at this head on both engines: editor builds succeed and `Monolith.Discover` passes 9/9 with 0 failed on UE 5.7 (`-RenderOffscreen`) and UE 5.8. Coverage asserts every row is a distinct namespace with a positive count, that the counts sum to the pre-pagination total under `limit=1`, and that the no-argument inventory does not gain the field.
|
Your point about the ranking engine landed, and following it through exposed that the trimmed version had only solved half the problem. Fixed in The branch answered which actions match, but the question that actually blocks a caller is which namespace to open — and with a small "matched_namespaces": [
{ "namespace": "blueprint", "match_count": 7 },
{ "namespace": "ui", "match_count": 2 }
]Every namespace containing at least one match, in registry order, computed before pagination like To be explicit about the thing you rejected, since a count next to a name can look like a score: It is also not new request surface: Verified at this head on both engines:
Coverage asserts every row is a distinct namespace with a positive count, that the counts sum to the pre-pagination Diff against master is 9 files; the only source touched is |
|
@codex review Latest head |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
kunkunGames
changed the title
kunkunGames
changed the title
…ading as zero results Three changes to discovery and project search, plus their offline mirrors. CROSS-NAMESPACE DISCOVER (#112) discover's filter predicate lived inside the namespace branch, and the no-namespace branch never parsed filter, offset, limit or detail at all. So discover(filter="set_property") with no namespace was SILENTLY IGNORED -- the caller got the full namespace inventory back and no error, and finding a capability by partial name meant iterating every namespace by hand. A non-empty filter with no namespace now searches the whole registry in registry order, tagging each row with its owning namespace and reporting matched_namespaces pre-pagination, so "which namespace owns this" stays answerable even when rows are capped. Unlike the per-namespace path this one is not naturally bounded -- ~150 actions worst case there versus ~1,400+ here -- so an absent limit caps at 50 while an explicit limit=0 still means ALL, matching the existing contract. Uncapped it would emit a row per match across the entire registry, from the tool whose terse redesign existed to cut payload. PAGINATION OVERFLOW (#112, independently shippable) SliceEnd = FMath::Clamp(SliceStart + Limit, ...) formed SliceStart + Limit as int32 before clamping. offset=1 with limit=2147483647 signed-overflows -- and that call is well-formed, because TryGetNumberField range-checks against TNumericLimits<int32> and so accepts INT32_MAX verbatim while rejecting anything larger. On MSVC it wrapped negative, Clamp collapsed the slice, and the caller got an EMPTY actions array plus a NEGATIVE next_offset. Min() is now taken first so every intermediate stays in range, and next_offset emits SliceEnd, which is value-identical and structurally overflow-free. SEARCH ERROR CLASSIFICATION AND VALIDATION (#113 slices) while (Step() == Row) treats a statement error as end-of-results, and Stmt.Create / SetBindingValueByIndex returns were unchecked, so a genuine FTS5 syntax error or a prepare failure surfaced as "no matches" -- the caller could not tell a bad query from an empty index. Caller errors now return -32602 and storage or schema failures -32603, with LIMIT bound as a parameter rather than formatted into the SQL. Adds query validation, a limit clamp and a 4096-character query cap. DELIBERATE DEVIATION FROM THE PLAN, on review of live SQLite behaviour: the plan's literal classification would have regressed column-qualified search. search runs two independent statements, and a query like node_name:Branch errors on the asset table before returning real hits from the node table -- so it works today only because the error is swallowed as end-of-results. Treating any per-table error as fatal would turn a working feature into a hard error. A table reporting "no such column" is therefore treated as not applicable and skipped; the query is only rejected when BOTH tables reject it, or when the error matches the table-independent syntax patterns. That behaviour is now documented rather than accidental, and has a regression test. PR #113's 1449-line FTS5 grammar parser is NOT taken. It introduced an unguarded recursive AST teardown with no query-length cap, which kills the editor process at roughly 32KB of chained terms -- reachable over MCP, on the game thread, with no recovery -- and a 15,935-query differential against real SQLite showed the safety property it claimed was actually delivered by an eight-line error classifier. Queries continue to bind straight into MATCH ?, where SQLite's own bounded grammar handles them. Reported and prototyped by @kunkunGames (#112, #113).
|
Shipped in v0.22.0. I land contributor fixes as my own commits rather than merging the branch — I keep the shipped history single-author for release integrity, and credit you in the release notes instead. No reflection on the patch. Thanks for rescoping this yourself after the earlier review — the trimmed version is the right change, and The gap was as you described: Two deviations. I kept the terse-description helper as a file-static rather than extracting a new header/source pair — it has exactly one call site in one translation unit, and two new files add surface to the release's full-unity collision gate for no gain. And I added a default cap: absent Your pagination overflow fix shipped as its own change and it is a real bug in released code: |
2 participants
Goal
Extend
monolith.discoverwith the smallest possible change so that a non-emptyfilterwithout a namespace searches matching actions across the existing live action registry.Plain-language explanation
A client that knows only part of a capability name can find candidates with one existing
discovercall instead of iterating over every namespace. This reuses the current registry, filtering, and pagination behavior; it does not add a new search engine or MCP tool.Improvements
discover(filter="...")searches actions across all namespaces.offset,limit, terse descriptions, anddetail/verbosebehavior are reused.tools/listoverlay.limit.Before → After
tools/listcache could omit current discover inputsSide-effect analysis
monolith_discoverdescriptor and preserves other Editor tools and live metadata.Current status
Re-reviewed the already minimized public head,
5fdfbe0951782e66095d50bbb11fab97dbc318a9, against the maintainer-requested scope. All 15 review threads are resolved, and the latest Codex review of this exact head found no material issue. This maintenance pass required no additional code change.Verification
Monolith.Discover: 9/9 PASS, with 0 test warnings, 0 errors, and process exit 0.D:\P4\MonolithPR112ProtectedHost\Saved\Automation\PR112MaintenanceFinalUE58\index.json.git diff --check: PASS. The worktree was clean and the branch was 0 commits behind the latesttumourlove/master.Docs/testing/2026-07-26-cross-namespace-discover.md.WorkFingerprint
agent: Codexcategory: discovery / orchestrationmodule: MonolithCorecomponent/action/helper:monolith.discover, terse action description, proxy seed overlayintended files:MonolithCoreTools.cpp,MonolithToolText.{h,cpp},MonolithDiscoverTerseTest.cpp, proxy seed/schema surfaces, matching Core docsrisk type: discovery regression, stale schema cache, pagination overflowpublic API impact: yes, additive behavior on the existingfilterdocs/spec impact: yesDuplicate check
On 2026-07-27, open PRs #104, #112, #113, and #114, their related remote branches, and their actual changed files were checked again. #104 targets updater binary selection, #113 targets project FTS search and repair, and #114 targets persistent activation and the index-writer lifecycle. #114 overlaps physically in
MonolithCoreTools.cppand Core documentation, but it changes different actions and sections; no implementation overlaps this PR's cross-namespace discovery contract. Mechanical rebasing may be required depending on merge order, but semantic ownership remains separate.