Polish object editor asset browsing - #100
Conversation
|
@codex review |
![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: 916664a087
ℹ️ 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".
| if (!isModel && !isTex && !isSound) continue; | ||
| const resolved = path.resolve(full); | ||
| const fileKey = process.platform === 'win32' ? resolved.toLowerCase() : resolved; | ||
| if (seenFile.has(fileKey)) continue; |
There was a problem hiding this comment.
When candidate roots overlap (for example, imports/ and its workspace parent), the new child-first ordering walks imported files first, but the later parent walk decrements budget before this seenFile check. Already-seen files can therefore exhaust the 4,000-entry budget and prevent unique files later in the parent root from being discovered—for example, 2,500 files under imports/ can consume another 1,500 slots during the duplicate parent traversal before a root-level model is reached. Check seenFile before charging the budget, or avoid descending into roots that were already walked separately.
Useful? React with 👍 / 👎.
| const importedModelsDir = path.join(dir, 'imports', 'units'); | ||
| fs.mkdirSync(importedModelsDir, { recursive: true }); | ||
| for (const name of ['Footman.mdx', 'FootmanPortrait.mdx', 'CaptainFootman.mdx', 'confirmation.mdx', 'AltarOfKings.mdx']) { | ||
| fs.writeFileSync(path.join(importedModelsDir, name), 'objmod search fixture'); |
There was a problem hiding this comment.
When the local harness uses its default generated project, these .mdx files contain plain text rather than model bytes. After the search assertions clear the query, the unfiltered thumbnail phase encounters these imported entries first; parsing marks them missing, while the harness later requires every initial visible slot to contain a loaded image, so the documented local thumbnail validation fails instead of measuring FIFO rendering and timing. Use actual small model fixtures or keep these search-only controls out of the model catalog.
AGENTS.md reference: AGENTS.md:L95-L95
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb2bc260c2
ℹ️ 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".
| const isTex = ext === 'blp' || ext === 'dds' || ext === 'tga'; | ||
| const isSound = SOUND_EXTS.has(ext); | ||
| if (!isModel && !isTex && !isSound) continue; | ||
| seenFile.add(fileKey); |
There was a problem hiding this comment.
When overlapping roots contain many non-asset files, those files are still charged repeatedly because seenFile is populated only after the extension filter. For example, walking imports/ first with 2,500 TXT files consumes 2,500 entries, and the later workspace-root walk can consume the remaining budget revisiting the same files before reaching a unique root-level model. Fresh evidence beyond the earlier comment is that the new guard still never records files rejected at line 276; record the physical path before decrementing the budget or before filtering by extension.
Useful? React with 👍 / 👎.
1 participant
Summary
Root cause
The two asset-picker hosts had drifted: the source-code picker still used a scattered-letter subsequence matcher, while the object-data picker used a boolean fuzzy matcher over a concatenated label/path/detail string. Neither produced relevance scores, so accepted entries stayed in catalog order. Imported assets were also reachable through overlapping candidate roots, producing duplicate paths.
Impact
Searches such as
footmannow return exact filename, prefix, substring, and bounded typo matches in relevance order, without unrelated entries such asconfirmation.mdx. Imported files appear once, ordinary fields retain the normal VS Code font, and density is visibly interactive while compact remains the default.Validation
npx tsc -p . --noEmitnpm run lint(0 errors; one pre-existingCopyFilePluginwarning)npm testnpm run vscode:prepublishfootmanquery, relevance-order assertions, negative controls, and tooltip-font scoping