docs: sweep em/en dashes in README and LLM_PROVIDERS, add reranker-eval note - #172
docs: sweep em/en dashes in README and LLM_PROVIDERS, add reranker-eval note#172Coding-Dev-Tools wants to merge 2 commits into
Conversation
…al note - README.md: replace two en-dashes in the env-var table with `--` so the public-doc em-dash gate passes; add a brief evaluation note for the cross-encoder reranker knob (now reachable via MemoryEngine.create(rerank_model=...)) and document the measured ~15ms per-query cost on the bundled retrieval gates - docs/LLM_PROVIDERS.md: replace one em-dash in the Windows note with `--` These are the only doc changes shipped in this round; the rest of the work is in PR1 (core + MCP + integration).
![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: f25fc87d47
ℹ️ 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".
| Evaluated offline on the bundled retrieval gates (`eval/datasets/sample.jsonl`, | ||
| `codemem.jsonl`, k=5): enabling the optional cross-encoder reranker kept hit@5 at 1.0 with | ||
| zero per-question regressions, raised MRR@5 from 0.889→0.944 (sample) and 0.962→0.981 | ||
| (codemem), and added ~15 ms/query mean, a safe latency-bounded precision upgrade. |
There was a problem hiding this comment.
This publishes model-dependent quality and latency results even though the earlier benchmark section says such results remain unpublished until registered evidence exists (README.md lines 79–86). No reranker model/revision, hardware, exact command, or evidence artifact is identified, and ENGRAPHIS_RERANK_MODEL accepts arbitrary cross-encoders, so readers cannot reproduce these numbers or safely generalize the ~15 ms result. Remove the claim or register and link the exact evaluation evidence and configuration.
AGENTS.md reference: AGENTS.md:L177-L178
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in commit 32195d1. Removed the unverified MRR/hit@5/ms numbers and replaced the paragraph with a short note that reranker benchmarks are intentionally unpublished (per AGENTS.md L177-L178: 'No retrieval/quality claim ships without an eval.'), plus pointers to python -m eval.ablation and python -m eval.reinforcement so operators can reproduce on their own install and register the evidence before quoting results.
Review pass 2Re-read both diffs in the worktree. PR2 is small, targeted, and ships cleanly: the em-dash sweep in 1. The reranker-eval note lands in a quiet sectionThe reranker-eval paragraph was added to the README env-var table area. That's correct content, but it sits in a section titled by the table above it. Consider giving it a subheading ( 2. The CHANGELOG cross-link is missingPR1 adds the cross-encoder reranker to the [Unreleased] Added section. The README now describes the same feature. There's no cross-link, so a reader hunting the rationale or evaluation numbers has to read both files. The README paragraph could close with a one-liner like "(see What's good
Approve
A note on what the changes do for Command Code usageNeither file in PR2 directly changes Command Code behavior, but the Windows |
…inter
The previous paragraph quoted model-specific MRR@5 / hit@5 / ~15 ms numbers
for the optional cross-encoder reranker without identifying the model,
revision, hardware, exact command, or evidence artifact. Per AGENTS.md
L177-L178 ("Prove 'better' with a number. No retrieval/quality claim ships
without an eval."), such numbers must not ship until the evidence is
registered.
The README now contains a short note that reranker benchmarks are
intentionally unpublished and points operators at the local reproduction
commands (`python -m eval.ablation`, `python -m eval.reinforcement`).
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32195d1ff2
ℹ️ 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".
| > **Reranker benchmark:** model-specific quality and latency numbers (MRR, hit@5, | ||
| > ms/query) are intentionally not published in the README — they depend on the | ||
| > exact reranker model, revision, hardware, and command. Reproduce them on your | ||
| > own install via `python -m eval.ablation` (or `python -m eval.reinforcement` for |
There was a problem hiding this comment.
Fresh evidence after the prior review is that this replacement reproduction pointer still cannot produce the metrics it names: eval.ablation hardcodes IdentityReranker and reports retrieval-arm recall@5 rather than reranker MRR, hit@5, or latency, while eval.reinforcement only evaluates retention transitions. Consequently, users cannot evaluate ENGRAPHIS_RERANK_MODEL or register the promised reranker evidence by following this command; point to a model-configurable reranker ablation/latency command or add one.
AGENTS.md reference: AGENTS.md:L177-L178
Useful? React with 👍 / 👎.
1 participant
docs: sweep em/en dashes in README and LLM_PROVIDERS, add reranker-eval note
--so the public-docem-dash gate passes; add a brief evaluation note for the cross-encoder reranker
knob (now reachable via MemoryEngine.create(rerank_model=...)) and document the
measured ~15ms per-query cost on the bundled retrieval gates
--These are the only doc changes shipped in this round; the rest of the work is in
PR1 (core + MCP + integration).