Skip to content

feat: add OpenRouter support with catalog-driven model resolution and cost-based routing#109

Open
samueltuyizere wants to merge 160 commits into
mainfrom
add-open-router
Open

feat: add OpenRouter support with catalog-driven model resolution and cost-based routing#109
samueltuyizere wants to merge 160 commits into
mainfrom
add-open-router

Conversation

@samueltuyizere

@samueltuyizere samueltuyizere commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

This PR adds OpenRouter provider support, a local model catalog system, and cost-based routing to routatic-proxy.

What's new

OpenRouter Provider (ba0045c, 1b50001)

  • Adds OpenRouter as a first-class provider alongside OpenCode Go / Zen / AWS Bedrock
  • Provider model index creation and synchronization via a local catalog

Model Catalog System (bdc792f56ed94c)

  • Catalog synchronization with configurable auto-sync intervals and directory resolution
  • Catalog loading, validation, and indexing from JSON-based model manifests
  • Lock file management for concurrent-safe syncs
  • Catalog management CLI commands (routatic-proxy catalog)
  • HTTP serving of catalog data for connected clients

Catalog-Based Model Resolution (bfa3e81, 032c1dd, b6ef72e, 0411bc4)

  • ModelRouter enhanced to resolve models from the catalog instead of static lists
  • Graceful handling of unknown providers with improved error messages
  • Provider filtering in models list command with cleaner output formatting

Cost-Based Routing (b6ecfdf, 3754f07, 4640470)

  • SelectCheapest routing strategy that selects the most cost-effective model satisfying request constraints
  • Respects tool usage, vision, reasoning effort, and context window constraints
  • Config-driven via routing.strategy with fallback to existing scenario-based routing

GUI Updates (56ed94c)

  • Dashboard updates to display catalog and OpenRouter provider status

@samueltuyizere samueltuyizere self-assigned this Jun 30, 2026
Comment thread internal/catalog/sync.go Outdated
Comment thread internal/catalog/sync.go Outdated
Comment thread internal/router/model_router.go Outdated
Comment thread internal/config/config.go
@kilo-code-bot

kilo-code-bot Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Code Review Roast 🔥

Verdict: 1 New Issue | 3 Previous Issues Resolved

Overview

Severity Count
🤏 nitpick 1
New Issue Details (click to expand)
File Line Roast
internal/gui/assets/app.js 1702 Duplicate classList.remove('copied') — copy-paste error in copyResponse()

Resolved Issues (from previous review)

Previous Location Issue Resolution
app.js line 1695 Orphaned object-literal body with no const LogsModule = { opener FIXED — orphaned block removed entirely
app.js line 1605 sendTest never checks r.ok FIXEDr.ok guard added at line 1613
app.js line 1582 Regressed to blocking native alert() FIXED — inline <div class="error"> used instead
style.css lines 946–1010 Orphaned CSS declarations and dead .log-entry* rules FIXED — unused styles removed

🏆 Best part: Three of the four prior review findings are genuinely dead — the orphaned LogsModule block, the missing r.ok guard, and the alert() regression are all gone. That's real progress, not just moving the cheese.

💀 Worst part: Line 1702. Someone copy-pasted this.testCopyBtn.classList.remove('copied') and forgot to delete the original. It's a harmless duplicate, but it's also a neon sign that says "I diffed this without reading it."

📊 Overall: Like a band-aid that was ripped off and put back on — the big wounds are closed, but there's still a sticky residue of carelessness. One quick trim and this is clean.

Files Reviewed (2 files)
  • internal/gui/assets/app.js - 1 new issue, 3 resolved
  • internal/gui/assets/style.css - resolved

Fix these issues in Kilo Cloud

Previous Review Summaries (13 snapshots, latest commit 087cbc4)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 087cbc4)

Verdict: 3 Issues Found | Recommendation: Address before merge

Overview

Severity Count
🚨 critical 1
⚠️ warning 0
💡 suggestion 1
🤏 nitpick 1
Issue Details (click to expand)
File Line Roast
internal/gui/assets/app.js 1695 Orphaned object-literal body with no const LogsModule = { opener — a SyntaxError that nukes the whole dashboard; also a verbatim clone of TestModule.
internal/gui/assets/app.js 1605 sendTest never checks r.ok, so HTTP 4xx/5xx fall through into the success path and raw error bodies get shown as answers.
internal/gui/assets/app.js 1582 Regressed from inline showError() to blocking native alert() on missing model/prompt.

🏆 Best part: The increment actually fixes the prior finding — lastModelID is gone from messages.go and the double-counted failure at line 778 is now a clean h.metrics.RecordFailure(). That continuity error from the last review is patched. Respect.

💀 Worst part: The app.js refactor is a merge car crash. One commit deleted const TestModule = { (the prior critical), the next re-added TestModule but left the old LogsModule body dangling without its const LogsModule = { opener — so the file is still unparseable, just via a different syntax error. The dashboard currently does not load at all.

📊 Overall: Like a game of whack-a-mole where the mole is a SyntaxError — you smacked it in messages.go only to have it pop up in app.js. Fix the orphaned block (lines 1694–1889) and the whole thing parses.

Fix these issues in Kilo Cloud

Files Reviewed (2 files, incremental)
  • internal/handlers/messages.go - prior warning resolved (no new issues)
  • internal/gui/assets/app.js - 3 issues (orphaned block @1695, missing r.ok @1605, alert() @1582)

Previous review (commit a1d57a6)

Verdict: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
🚨 critical 0
⚠️ warning 1
💡 suggestion 0
🤏 nitpick 0
Issue Details (click to expand)
File Line Roast
internal/handlers/messages.go 778 RecordFailureForModel(lastModelID) double-counts the final model's failure — handleStreamError (lines 593/601) already recorded it on the way out, so modelFailed[lastModelID] is inflated. (Note: the posted inline comment's suggestion block has a typo — it should read h.metrics.RecordFailure(), not Recfailure.)

🏆 Best part: This increment actually closes the prior RecordFailureForModel-is-never-called finding (metrics.go line 83) at 591, 599, and 1101 — exactly the wiring that comment asked for. Clean follow-through.

💀 Worst part: At the streaming finish line (line 778) you attributed the failure to lastModelID again after handleStreamError already counted it, so the last fallback model's failure rate is double-cooked. Revert line 778 to the model-less h.metrics.RecordFailure() and drop the now-unused lastModelID var (lines 516/519) — per-model failures are already recorded inside handleStreamError.

📊 Overall: Like a sequel that fixes the original plot hole but accidentally leaves a clone of the villain in the final scene — one small continuity error to patch before shipping.

Files Reviewed (1 file, incremental)
  • internal/handlers/messages.go - 1 issue (double-counted per-model failure at line 778)

Fix these issues in Kilo Cloud

Previous review (commit cd91105)

Verdict: No Issues Found | Recommendation: Merge

Oh wait, this PR increment is actually clean. I need to sit down. I had my flamethrower warmed up and everything.

The previous duplicate success/failure collection block in metrics.go is gone — fixed properly, not papered over. The Logs tab removal is also clean: zero dangling JS references, zero orphaned HTML IDs, no selector rot. It's the kind of boring, correct deletion that makes a reviewer suspicious.

📊 Overall: Like finding a unicorn in production — I didn't think clean PR increments existed anymore, but here we are.

Files Reviewed (3 files, incremental)
  • internal/gui/assets/app.js - 0 issues (dead unreachable break removed)
  • internal/gui/assets/index.html - 0 issues (Logs tab cleanly removed, no dangling references)
  • internal/metrics/metrics.go - 0 issues (prior duplicate-block finding resolved)

Previous review (commit 96143b8)

Verdict: 2 Issues Found | Recommendation: Address before merge

Overview

Severity Count
🚨 critical 0
⚠️ warning 1
💡 suggestion 1
🤏 nitpick 0
Issue Details (click to expand)
File Line Roast
internal/metrics/metrics.go 164 Duplicate success/failure collection block — lines 163-177 re-make() and re-populate maps already built at 148-161 (dead copy-paste)
internal/metrics/metrics.go 83 RecordFailureForModel is defined and read by perf.go but never called; all failure paths use RecordFailure(), so ModelFailed stays empty and the perf tab reports a permanent 100% success rate

🏆 Best part: The per-model Success plumbing is actually wired correctly — RecordSuccess populates modelSuccess and perf.go reads it as intended. Half the feature shows real intent.

💀 Worst part: RecordFailureForModel is a function-shaped ghost. It's defined, exported, and consumed by perf.go, but no failure path ever calls it, so every model gets an eternal 100% success rate. The dashboard is lying to you with a straight face.

📊 Overall: Like a fitness tracker that counts your steps but silently zeroes your heart-rate — the metrics half-work, but the scary numbers are missing, and you'd never know from looking.

Files Reviewed (2 files, incremental)
  • internal/metrics/metrics.go - 2 issues (duplicate collection block, dead RecordFailureForModel)
  • internal/gui/perf.go - 0 new (reads the added fields correctly; depends on the upstream model wiring that's missing)

Fix these issues in Kilo Cloud

Previous review (commit 2f3cbbb)

Verdict: 2 Issues Found | Recommendation: Address before merge

Overview

Severity Count
🚨 critical 1
⚠️ warning 0
💡 suggestion 1
🤏 nitpick 0
Issue Details (click to expand)
File Line Roast
internal/gui/assets/app.js 1519 const TestModule = { was deleted and merged into LogsModule, but TestModule.init() (1686), TestModule.testModal (1087) and TestModule.close() (1088) still reference the now-undefined TestModule → ReferenceError, dead Quick Model Test feature
internal/gui/assets/style.css 946 Orphaned CSS: .log-entry selector deleted but its body left dangling (stray }), plus dead .log-* rules (951–1010)

🏆 Best part: internal/gui/server.go + internal/gui/logs.go — the Go-side teardown of the logs feature is genuinely clean. logBuffer, the strconv import, handleLogsStream, and handleLogsHistory are all gone with zero dangling references. The backend understood the assignment. (This also makes the previous review's server.go:595 level-filter finding moot — the whole endpoint was deleted.)

💀 Worst part: internal/gui/assets/app.js:1519 — the frontend cleanup is a car crash. Deleting const TestModule = { and merging its members into LogsModule left TestModule.init() (line 1686) pointing at a variable that no longer exists. The Quick Model Test panel is now 100% non-functional, and pressing Escape near that logic throws too.

📊 Overall: Like a magician's assistant who sawed the volunteer in half and forgot the "put back together" half. The backend disappeared the logs feature cleanly; the frontend left the test module bleeding out under a Logs Module name tag.

Fix these issues in Kilo Cloud

Files Reviewed (5 files, incremental)
  • internal/gui/assets/app.js - 1 critical (TestModule undefined)
  • internal/gui/assets/style.css - 1 suggestion (orphaned/dead log CSS)
  • internal/gui/assets/index.html - 0 new (Logs tab button + container still present but backend-orphaned; not changed by this increment)
  • internal/gui/server.go - 0 new (clean teardown; prior level filter issue resolved by deletion)
  • internal/gui/logs.go - deleted (resolves previous review's level filter finding)

Previous review (commit ebd3e41)

Verdict: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
🚨 critical 0
⚠️ warning 0
💡 suggestion 1
🤏 nitpick 0
Issue Details (click to expand)
File Line Roast
internal/gui/server.go 595 The level filter is still not implemented — the param is now read purely to be discarded (_ = ...). Linter appeased, feature still missing.

🏆 Best part: The stream.go SA6002 fix is textbook. Pooling *[]byte instead of a boxed []byte, New returns &b, Read(*readBuf) and (*readBuf)[i] dereference cleanly, and Put(readBuf) hands the pointer back — no allocation on Put, no aliasing surprises. The anti-allocation code no longer allocates. The parking brake is off; the race car actually races now.

💀 Worst part: internal/gui/server.go:595 — the log-level filter got a fix that fixes nothing user-facing. Previously it parsed and defaulted level then ignored it; now it reads level and immediately throws it away with _ =. The linter warning is gone, but ?level=error still returns every log line. Silencing the smoke alarm isn't the same as putting out the fire.

📊 Overall: A clean, correct refactor round — one real bug (SA6002) squashed properly, one dead feature that got camouflaged instead of built. Like painting over a leak: looks better, still drips.

Files Reviewed (3 files, incremental)
  • internal/transformer/stream.go - 0 issues (SA6002 pool boxing fixed correctly with *[]byte)
  • internal/gui/server.go - 1 issue (level filter still not applied)
  • scripts/git-hooks/pre-push - 0 issues (golangci-lint step added, checks renumbered to 5)

Fix these issues in Kilo Cloud

Previous review (commit 15f4486)

Verdict: 2 Issues Found | Recommendation: Address before merge

Overview

Severity Count
🚨 critical 0
⚠️ warning 1
💡 suggestion 1
🤏 nitpick 0
Issue Details (click to expand)
File Line Roast
internal/gui/server.go 595 level is parsed and defaulted, then completely ignored — logs returned unfiltered. Ineffectual assignment + no-op filter.
internal/transformer/stream.go 30 An allocation-reduction refactor whose own sync.Pool boxes a []byte value on every Put (SA6002). Pool a *[]byte instead.

🏆 Best part: The stream.go string→[]byte conversion is genuinely careful — line[6:] correctly matches the old TrimPrefix("data: "), content gets copied via string(content) before the line buffer is reused, and json.Unmarshal on the aliased slice is safe because RawMessage copies. No use-after-reset bugs in a refactor that was begging for one. I'm mildly stunned. And the app.js stray-brace removal actually un-breaks a top-level syntax error, while the Array.isArray guard is a tidy defensive fix.

💀 Worst part: internal/gui/server.go:595 — a three-line ceremony to parse and default a level filter that the very next line throws away by calling s.logBuffer.Last(n) unfiltered. It's confident, well-formatted, and does absolutely nothing.

📊 Overall: A solid, safe refactor with one dead-on-arrival feature and one ironic allocation in the anti-allocation code. Like a race car with the parking brake gently on — fast, but you can smell it.

Files Reviewed (4 files, incremental)
  • internal/gui/server.go - 1 issue
  • internal/transformer/stream.go - 1 issue
  • cmd/routatic-proxy/ui_linux_nocgo.go - 0 issues (existing-proxy detection now uses connectedToExisting)
  • internal/gui/assets/app.js - 0 issues (stray brace fixed, array guard added)

Fix these issues in Kilo Cloud

Previous review (commit f845473)

Verdict: 2 Issues Found | Recommendation: Address before merge

Overview

Severity Count
🚨 critical 0
⚠️ warning 1
💡 suggestion 1
🤏 nitpick 0
Issue Details (click to expand)
File Line Roast
internal/gui/server.go 595 level is parsed and defaulted, then completely ignored — logs returned unfiltered. Ineffectual assignment + no-op filter.
internal/transformer/stream.go 30 An allocation-reduction refactor whose own sync.Pool boxes a []byte value on every Put (SA6002). Pool a *[]byte instead.

🏆 Best part: The stream.go string→[]byte conversion is genuinely careful — line[6:] correctly matches the old TrimPrefix("data: "), content gets copied via string(content) before the line buffer is reused, and json.Unmarshal on the aliased slice is safe because RawMessage copies. No use-after-reset bugs in a refactor that was begging for one. I'm mildly stunned. And the app.js stray-brace removal actually un-breaks a top-level syntax error, while the Array.isArray guard is a tidy defensive fix.

💀 Worst part: internal/gui/server.go:595 — a three-line ceremony to parse and default a level filter that the very next line throws away by calling s.logBuffer.Last(n) unfiltered. It's confident, well-formatted, and does absolutely nothing.

📊 Overall: A solid, safe refactor with one dead-on-arrival feature and one ironic allocation in the anti-allocation code. Like a race car with the parking brake gently on — fast, but you can smell it.

Files Reviewed (4 files, incremental)
  • internal/gui/server.go - 1 issue
  • internal/transformer/stream.go - 1 issue
  • cmd/routatic-proxy/ui_linux_nocgo.go - 0 issues (existing-proxy detection now uses connectedToExisting)
  • internal/gui/assets/app.js - 0 issues (stray brace fixed, array guard added)

Fix these issues in Kilo Cloud

Previous review (commit d6917eb)

Verdict: No Issues Found | Recommendation: Merge

Oh wait, this PR increment is actually clean. I need to sit down. I had my flamethrower warmed up and everything.

The lone change at Makefile:36 is a genuine fix: it removes the self-defeating || true that masked gofmt failures, restores tee /dev/stderr so the formatting diff is visible, and replaces the broken grep . && ... pattern with a correct test -z check that actually fails the build when gofmt finds issues. Previous review findings on this line are resolved.

📊 Overall: Like finding a unicorn in production — I didn't think clean PR increments existed anymore, but here we are.

Files Reviewed (1 file, incremental)
  • Makefile - 0 issues (prior lint-gate issues resolved)

Previous review (commit 99cb066)

Verdict: No Issues Found | Recommendation: Merge

Oh wait, this incremental diff is actually clean. I need to sit down. I had my flamethrower warmed up and everything.

The lone change wraps a deferred rows.Close() in defer func() { _ = rows.Close() }() — the textbook, idiomatic way to shush errcheck on a deferred Close() that you have no sane way to handle anyway. No logic change, no runtime impact, no new bugs. Boring in the best way possible.

📊 Overall: Like finding a unicorn in production — I didn't think clean PR increments existed anymore, but here we are.

Files Reviewed (1 file, incremental)
  • internal/storage/requests.go - 0 issues (linter-pacification on defer rows.Close())

Previous review (commit 3ceb0f4)

Verdict: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
🚨 critical 0
⚠️ warning 1
💡 suggestion 0
🤏 nitpick 0
Issue Details (click to expand)
File Line Roast
Makefile 36 The lint gofmt guard ends in `

🏆 Best part: The catalog.go cleanup is genuinely good — the dbPath ~/ expansion block was dead code (computed but never passed to storage.Open), so dropping it and the now-unused strings import is the right call. The defer func() { _ = db.Close() }() and _ = encoder.Encode(...) changes are standard linter-pacification that don't introduce new bugs.

💀 Worst part: Makefile line 36 turns the gofmt gate into a no-op that always reports success. A guardrail that can't fail is just decorative plumbing.

📊 Overall: Like a smoke detector with the battery taped over — it still beeps reassuringly, but it won't stop the fire.

Files Reviewed (6 files, incremental)
  • Makefile - 1 issue (gofmt guard never fails)
  • cmd/routatic-proxy/catalog.go - 0 issues (dead-code cleanup)
  • cmd/routatic-proxy/main.go - 0 issues
  • internal/gui/config_io.go - 0 issues
  • internal/storage/latency.go - 0 issues
  • internal/storage/logs.go - 0 issues

Previous review (commit d6c1f5e)

Verdict: No Issues Found | Recommendation: Merge

Incremental Scope

This run covers only the changes since the previous review SHA (81beeed33bce3960b401c1350837df95edf18247). The change set is cleanup and linter hygiene across multiple files.

Resolved Findings

File Prior Issue Status
cmd/routatic-proxy/catalog.go dbPath[:2] == "~/" panic on short/missing path ✅ Fixed — replaced with strings.HasPrefix
cmd/routatic-proxy/main.go dbPath[:2] panic and missing strings import in same block ✅ Fixed — block removed entirely
scripts/git-hooks/pre-push `go test while read pipe swallows exit code (set -ewithoutpipefail`)
Makefile Dropped tee /dev/stderr hid gofmt diff output ✅ Fixed — restored in this increment

New Issues

Severity Count
🚨 critical 0
⚠️ warning 0
💡 suggestion 0
🤏 nitpick 0

🏆 Best part: This diff is a masterclass in "make the linter happy" — panic-prone slice hacks fixed, pipefail restored, dead code purged, and the gofmt diff output brought back. The kind of boring, correct cleanup every codebase needs periodically.

💀 Worst part: Nothing. I'm actually suspicious. A PR this clean in a codebase this size is either a phantom or a setup. But I'll take the win.

📊 Overall: Like a quiet Tuesday in operations — no pages, no fires, just maintenance that makes the next deployment slightly less painful.

Files Reviewed (14 files, incremental)
  • .gitignore - 0 issues
  • Makefile - 0 issues (prior issue resolved)
  • cmd/routatic-proxy/catalog.go - 0 issues (prior issue resolved)
  • cmd/routatic-proxy/ui_linux_nocgo.go - 0 issues (dead code cleanup)
  • internal/catalog/migrate_test.go - 0 issues
  • internal/gui/catalog_stats_test.go - 0 issues
  • internal/gui/config_io.go - 0 issues
  • internal/gui/logs.go - 0 issues
  • internal/server/server.go - 0 issues
  • internal/storage/catalog.go - 0 issues
  • internal/storage/database.go - 0 issues
  • scripts/git-hooks/pre-push - 0 issues (prior issue resolved)
  • internal/catalog/testdata/catalog.json - 0 issues
  • Deleted planning docs (5 files) - not code issues

Previous review (commit 81beeed)

Verdict: 1 Warning | Recommendation: Address before merge

Incremental Scope

This run covers only the changes since the previous review SHA (596d324). The incremental diff spans 2 files: .github/workflows/ci.yml (trailing newline removed) and .golangci.yml (file deleted).

Resolved Findings (from prior review)

File Prior Issue Status
.golangci.yml -_unused typo (critical CI-breaker) ✅ Resolved — file deleted
.golangci.yml default: none dropping errcheck/staticcheck/unused ✅ Resolved — file deleted
.golangci.yml exclude-dirs: internal/tray taping over check-engine light ✅ Resolved — file deleted

New Issues

Severity Count
🚨 critical 0
⚠️ warning 1
💡 suggestion 0
🤏 nitpick 0
Issue Details (click to expand)
File Line Roast
.golangci.yml N/A 🔥 The Roast: The explicit linter whitelist is gone. This commit deletes .golangci.yml entirely, so golangci-lint-action@v7 falls back to its built-in defaults — a much broader set of enabled linters than the previous whitelist (govet, errcheck, staticcheck, unused, ineffassign). CI behavior silently changes, and if the codebase trips any of the newly-active linters, pipelines start failing with no warning to the reviewer. \nn🩹 The Fix: Either replace .golangci.yml with an equivalent config (even a minimal one restoring the explicit whitelist), or explicitly set config: '' in the action inputs to document that defaults are intentional. Don't let CI drift happen by accident. \nn📏 Severity: warning \nn--- \nnReply with `@kilocode-bot fix

[Snapshot truncated.]

Additional previous summary content was truncated to keep this comment within platform limits.


Reviewed by step-3.7-flash · Input: 364.1K · Output: 45K · Cached: 3.5M

@samueltuyizere samueltuyizere changed the title Add open router feat: add OpenRouter support with catalog-driven model resolution and cost-based routing Jun 30, 2026
- Fix 'configModels is not iterable' error in app.js populateModels()
  by using Array.isArray() check instead of optional chaining fallback
- Restore stream.go to working state from commit c4616e9
  (removes inconsistent sync.Pool usage and bytes.Buffer confusion)
- Add pre-push hook documentation to CONTRIBUTING.md
- Pass []byte directly to fmt.Fprintf instead of string(data)
- Use bytes functions instead of strings for SSE parsing
- Change lineBuf from bytes.Buffer to []byte slice
- Remove heap allocations from hot SSE parse loop
Comment thread internal/gui/server.go Outdated
Comment thread internal/transformer/stream.go
- Add golangci-lint run as check 3/5 (between go vet and tests)
- Skip with warning if golangci-lint not installed
- Renumber remaining checks to 4/5 (tests) and 5/5 (build)
- Remove unused level variable in gui/server.go
- Fix SA6002: store pointers to slices in sync.Pool to avoid allocations
- Update Get/Put calls to use *[]byte and dereference on read
Comment thread internal/gui/server.go Outdated
- Remove /api/logs/stream and /api/logs/history endpoints
- Delete internal/gui/logs.go (LogBuffer, LogHandler, SSE streaming)
- Remove Logs tab from index.html and navigation
- Remove LogsModule, log translations, and log CSS from frontend
- Remove goto-logs command palette action and keyboard shortcut
- Clean up unused imports (strconv) and struct fields (logBuffer)
Comment thread internal/gui/assets/app.js
Comment thread internal/gui/assets/style.css Outdated
…ure tracking

- Add modelSuccess/modelFailed maps to Metrics struct with dedicated mutexes
- RecordSuccess now increments per-model success counter
- Add RecordFailureForModel for future per-model failure tracking
- Snapshot now includes ModelSuccess and ModelFailed maps
- perf.go handlePerformance now populates Success/Failed from Snapshot when using in-memory metrics
- Frontend success rate calculation now correctly shows (success / count * 100) per model
Comment thread internal/metrics/metrics.go Outdated
Comment thread internal/metrics/metrics.go
- Change 4 RecordFailure() calls in messages.go to RecordFailureForModel(model.ModelID)
- Line 591: stream idle error after SSE started
- Line 599: upstream failure after SSE started
- Line 776: final fallback failure in handleStreamingRequest
- Line 1099: final fallback failure in handleNonStreamingRequest (uses result.ModelID)
- This populates snap.ModelFailed so Performance tab shows real per-model failure rates instead of always 100% success
Comment thread internal/handlers/messages.go Outdated
- Remove duplicate RecordFailureForModel declaration in metrics.go
- Add lastModelID declaration before modelChain loop in handleStreamingRequest
- Use lastModelID at final fallback failure recording point
- Revert line 778 from RecordFailureForModel(lastModelID) to RecordFailure()
- handleStreamError already recorded per-model failures inside the loop
- Using RecordFailureForModel at the 'all failed' point double-counted the last model
- Remove unused lastModelID variable (was only needed for per-model recording)
Comment thread internal/gui/assets/app.js Outdated
Comment thread internal/gui/assets/app.js
Comment thread internal/gui/assets/app.js Outdated
samueltuyizere and others added 3 commits July 12, 2026 05:56
Co-authored-by: kilo-code-bot[bot] <240665456+kilo-code-bot[bot]@users.noreply.github.com>
Signed-off-by: TUYIZERE Samuel <tuyizeres0@gmail.com>
setTimeout(() => {
this.testCopyBtn.innerHTML = originalText;
this.testCopyBtn.classList.remove('copied');
this.testCopyBtn.classList.remove('copied');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 The Roast: Someone held down the Ctrl+V key a little too long. this.testCopyBtn.classList.remove('copied') now appears twice in a row at lines 1701–1702 — the second invocation is the coding equivalent of nodding at your own joke after everyone else stopped laughing. It's harmless, but it's also a tell that this diff was copy-pasted from somewhere and nobody proofread it.

🩹 The Fix:

Suggested change
this.testCopyBtn.classList.remove('copied');
this.testCopyBtn.classList.remove('copied');
}, 2000);

Reply with @kilocode-bot fix it to have Kilo Code address this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant