fix(proxy): repair the comment #1601 landed malformed, and the claim it left standing - #1602
Conversation
…it left standing Two defects in the block 4e909f9 put on release, one cosmetic and one not. The MSIE bullet opens with `space-hyphen` instead of `space-asterisk-space-hyphen`, so that bullet and every line indented under it sits outside the block comment. Nothing breaks — tsc is happy and prettier does not normalise the asterisk prefix inside a block comment, so all five required checks stayed green over a comment that no longer reads as one. Our format gate cannot see this class of defect at all. The substantive one: the paragraph introducing that bullet still says Copilot CLI is the one client here that cannot be identified from its User-Agent, and both of the strings it sends are actively unsafe to key on which is the exact claim #1601 existed to retract. The corrected bullet says the MSIE string is curl's and not a CLI's; the sentence two lines above it still says Copilot sends it. A reader has no way to tell which half is current, and the retraction is the half that looks like an afterthought. That is worth naming rather than quietly fixing. The correction was written by editing the bullet in place, so the framing sentence was never re-read — the same failure the commit was about, one paragraph higher. Fixing a wrong claim means re-reading everything that claim supports, not just the line it was written on. The block is now restructured so the two strings are not presented as a pair. They were only ever grouped because both were Copilot-mapping candidates; they have nothing else in common, and grouping them under "both of the strings it sends" is what made the false framing easy to write and hard to see. Also fix the one other block comment in src/ with the same defect, in evaluationProviders.ts. Found by sweeping for it, since neither tsc nor prettier reports it and there was no reason to think this instance was unique.
✅ Single Commit Policy - COMPLIANTStatus: Policy requirements met • 1 commit • Valid format • Ready for merge 📊 View validation details📝 Commit Details
✅ Validation Results
🤖 Automated validation by NeuroLink Single Commit Enforcement |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe pull request updates comments in provider evaluation and client attribution code. It clarifies provider performance recording and corrects attribution notes for unmapped User-Agent strings and Copilot headers. No executable behavior changes. ChangesDocumentation updates
Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: ⚪ Minimal · up to This change corrects misleading attribution documentation and malformed comment formatting without changing runtime behavior. No actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 ESLint
src/lib/core/evaluationProviders.tsParsing error: Unable to parse the specified 'tsconfig' file. Ensure it's correct and has valid syntax. error TS5012: Cannot read file '/.svelte-kit/tsconfig.json': ENOENT: no such file or directory, open '/.svelte-kit/tsconfig.json'. src/lib/proxy/clientAttribution.tsESLint skipped: the matched ESLint configuration already failed (missing-dependency). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🎉 This PR is included in version 12.7.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Follow-up to #1601 (4e909f9). Two defects in the block it landed — one cosmetic, one not.
1. The comment no longer parses as one
src/lib/proxy/clientAttribution.ts— the MSIE bullet opens withspace-hypheninstead ofspace-asterisk-space-hyphen, so that bullet and everything indented under it sits outside the block comment:Caught in review by a second session. Nothing breaks — tsc is happy, and prettier does not normalise the asterisk prefix inside a block comment, so
format:checkpasses and all five required checks stayed green over it. Our format gate cannot see this class of defect.2. The claim #1601 existed to retract is still there
The paragraph that introduces the corrected bullet still reads:
The bullet below it says the MSIE string is curl's and not any CLI's. The sentence above says Copilot sends it. Both are on
releaseright now, and a reader has no way to tell which is current — with the retraction looking like the afterthought.This is the more serious of the two, and it happened because the correction was made by editing the bullet in place, so the framing sentence was never re-read. That is the same failure the commit was about, one paragraph higher up. Fixing a wrong claim means re-reading everything that claim supports, not just the line it was written on.
The block is now restructured so the two strings are not presented as a pair. They were only ever grouped because both had been Copilot-mapping candidates; they have nothing else in common, and grouping them under "both of the strings it sends" is what made the false framing easy to write and hard to notice.
3. One other instance of the same defect
Swept
src/for block-comment lines missing their leading asterisk, on the grounds that neither tsc nor prettier reports it and there was no reason to assume this instance was unique. Found exactly one more, inevaluationProviders.ts, and fixed it. Sweep now reports zero.Verification
pnpm run check— 0 errorspnpm run lint— 0 errorspnpm run check:tools-tests— cleanpnpm run build— cleanOne note for whoever runs these next:
check:tools-teststype-checks test files that import../dist/index.js, so running it against a staledistproduces confident errors in unrelated suites. It reported four TTS type errors here untilbuildwas re-run, and they had nothing to do with this change. Runbuildfirst.Summary by CodeRabbit