Repository navigation
fix: correct backend and mixed-repo smoke failures - #160
PatrickSys wants to merge 2 commits into
Conversation
Stop treating JWT and email Subject as RxJS, tell edit preflight to narrow the query when the candidate budget is exhausted, and load persisted index stats so status shows the keyword chunk cap.
Rewrite a candidate-budget miss in the search tool response so the guidance does not tell callers to refresh a current index.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ebc132455a
ℹ️ 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".
| # Next | ||
|
|
||
| Remaining gaps from the backend and mixed-repo operational smoke in `docs/benchmark.md`. These are not Phase 1 work. |
There was a problem hiding this comment.
Exclude local planning artifacts from the commit
This commit includes a .planning/ROADMAP.md update despite the repository's explicit guardrail against committing .planning/** without a direct user request, and its pre-commit requirement to abort when those files are staged. Remove this roadmap change from the proposed patch unless its inclusion was explicitly authorized; the public smoke results can remain in docs/benchmark.md.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
[Medium impact] The changes affect status, edit-search guidance, and convention maps across indexed repositories. The PR appears safe to merge, with non-blocking improvements needed for status cost and stale-index advice. FindingsSummaryThe PR fixes three smoke-test problems:
Two non-blocking concerns remain: status reloads the full keyword index, and budget advice can hide the stale-index refresh step. PatrickSys explicitly deferred Python and Go import graphs, chunk selection beyond the cap, style-guide lookup gaps, language-native patterns, and low-confidence prose preflight. These known gaps were not reported as new findings. Reviews (1) · Last reviewed commit: "fix: narrow edit preflight outside the s..." · Reviewed by Greptile |
| const [statsRaw, indexRaw] = await Promise.all([ | ||
| fs.readFile(statsPath, 'utf-8'), | ||
| fs.readFile(keywordPath, 'utf-8') | ||
| ]); | ||
| const statsParsed: unknown = JSON.parse(statsRaw); | ||
| const indexParsed: unknown = JSON.parse(indexRaw); | ||
| const searchableChunks = searchableChunkCount(indexParsed); |
There was a problem hiding this comment.
readDiskIndexSummary reads and parses all of index.json on every status call, even when in-memory stats exist. That file holds up to 5,000 full code chunks by default, including source text and metadata. Repeated progress checks therefore add avoidable disk reads, memory use, and blocking JSON parsing.
Persist the kept-chunk count in indexing-stats.json, or cache it per published index, instead of loading every chunk just to count them.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| export function presentSourceCoverage<T extends SourceCoverageActionInput>(coverage: T): T { | ||
| if (coverage.skippedFiles > 0 || coverage.candidateRetrievalFailed === true) return coverage; | ||
| const refreshAction = sourceCoverageRefreshAction({ | ||
| skippedFiles: 0, | ||
| candidateBudgetExhausted: coverage.candidateBudgetExhausted === true, | ||
| candidateRetrievalFailed: false | ||
| }); | ||
| if (!refreshAction || refreshAction === coverage.refreshAction) return coverage; | ||
| return { ...coverage, refreshAction }; |
There was a problem hiding this comment.
When the index is older than seven days and the candidate budget runs out with no skipped files, presentSourceCoverage replaces the refresh action with “narrow the query.” The partial-results branch then uses that text to overwrite the stale-index action from buildEvidenceLock.
Editing stays blocked, but narrowing the query does not clear the stale-index gate. Keep the stale-index refresh instruction ahead of the budget advice.
What / Why
Operational smoke of four real repositories: a NestJS API, Django healthchecks, the Dex Go service, and the FastAPI full-stack template (TypeScript frontend plus Python backend).
init,reindex,status, convention lookup, edit preflight, and symbol search were timed on each. The pass/fail matrix is indocs/benchmark.mdunder "Repo-kind operational smoke", as a backend table in that section. Remaining gaps are under Next in.planning/ROADMAP.md.Symbol search passed on all four.
initand model-freereindexpassed. Three results were wrong rather than merely thin:Subjectas RxJS.statusin a new process omitted persisted file counts and the 5,000-chunk keyword cap (Dex omitted 8,940 chunks; healthchecks omitted 812).The fixes are small and covered by fixture tests. Python and Go import graphs are still empty, and prose edit preflight stays low-confidence on those repos. That is recorded as a gap, not a pass.
Release notes