fix: address Codex review from #148 (3 P1 + 1 P2)#149
fix: address Codex review from #148 (3 P1 + 1 P2)#149
Conversation
- Fix PromptRouter regex: \s not s for whitespace splitting - Fix ReactiveCompaction: downgrade least-relevant half, not most-relevant - Fix reactivePackerWrapper: truncate content after depth adjustment - Fix agentSearchIntegration: include metadata in cache key for freshness
![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: 1eee49d9fe
ℹ️ 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".
src/graph/reactivePackerWrapper.ts
Outdated
| ? contentStr.slice(0, truncatedLength) + ' | ||
| [... truncated by reactive compaction]' |
There was a problem hiding this comment.
This string literal contains a raw line break between opening and closing quotes, which makes reactivePackerWrapper.ts fail to parse (TS1002: Unterminated string literal) and blocks builds/imports that touch this module. Use an escaped newline (for example \n) or a template literal so compaction can run without syntax errors.
Useful? React with 👍 / 👎.
| knowledge: KnowledgeSource[] = [], | ||
| ): AgentSearchService { | ||
| const hash = JSON.stringify(agents.map(a => a.id).sort()); | ||
| const hash = JSON.stringify(agents.map(a => [a.id, a.description, a.role, ...(a.tags ?? [])].join('|')).sort()); |
There was a problem hiding this comment.
This hash now includes description/role/tags, but reindex() still writes _lastIndexHash using only agent IDs, so the next createAgentSearchService() call will always see a mismatch and rebuild the index even when nothing changed. That effectively disables the intended cache fast path after any reindex() call and adds avoidable reindex cost.
Useful? React with 👍 / 👎.
- Fix unterminated string literal: escape newline in truncation marker - Fix reindex() hash: use same format as createAgentSearchService()
All join('\n') calls had raw newlines instead of escaped.
Fixes TS1002 Unterminated string literal in 3 files.
Fixes .join('\n\n') double-newline patterns that also had raw newlines.
|
Closing: all 4 fixes (3 P1 + 1 P2) are already present on master. Verified each one:
|
1 participant
Codex Review Fixes
Addresses all 4 issues flagged by Codex on PR #148 (now merged).
PromptRouter.tss→\sfor whitespace splittingReactiveCompaction.ts0..half(least relevant) instead ofhalf..end(most relevant)reactivePackerWrapper.tsagentSearchIntegration.ts