Feat/channel success rates#6270
Conversation
… fixture, and relay extraction
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
WalkthroughPerformance metrics now support channel-scoped persistence, aggregation, in-memory processing, Redis keys, and HTTP endpoints. Migrations preserve legacy rows, while tests cover storage, pipeline behavior, API responses, filtering, and test database isolation. ChangesChannel performance metrics
Estimated code review effort: 4 (Complex) | ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pkg/perf_metrics/metrics.go (1)
216-231: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winExclude channel-specific buckets from model summaries.
Every positive-channel sample exists in both channel 0 and its channel-specific bucket. This loop merges both as channel 0, so
QuerySummaryAlldouble-counts all unflushed channel traffic.Proposed fix
hotBuckets.Range(func(key, value any) bool { k := key.(bucketKey) + if k.channel != 0 { + return true + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/perf_metrics/metrics.go` around lines 216 - 231, Update the hotBuckets iteration in QuerySummaryAll to skip channel-specific buckets before calling mergeModelTotals and mergeModelBucket, retaining only channel 0 entries for model summaries. Preserve the existing timestamp, group, and request-count filters.router/api-router.go (1)
35-41: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not expose channel telemetry through public pricing access.
These routes inherit
HeaderNavModulePublicOrUserAuth("pricing"), so they can be anonymous when pricing is public. That exposes internal channel IDs, model assignments, traffic volumes, and success rates despite the stated authenticated-route contract.Apply an explicit authenticated/admin middleware appropriate for channel-level operational data.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@router/api-router.go` around lines 35 - 41, Update the channel-level routes in the perfMetricsRoute group, specifically /channels and /channels/:channel_id/models/:model_name, to use an explicit authenticated/admin middleware instead of inheriting public pricing access. Keep the general pricing middleware for non-channel performance routes, while ensuring channel telemetry is never anonymously accessible.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controller/perf_metrics_channel_test.go`:
- Around line 173-176: Replace the t.Skip in
TestGetChannelPerfMetrics_GroupFilter with a real contract test: initialize
commonGroupCol and the required database state explicitly, seed performance
metrics belonging to two distinct groups, request the endpoint with ?group= set
to one group, and assert the response contains only that group’s metrics.
In `@controller/perf_metrics.go`:
- Around line 120-129: Normalize the parsed hours value in the affected handlers
before calculating startTs and endTs, matching perfmetrics.Query: use 24 hours
for non-positive values and cap positive values at 30 days. Apply the same
normalization in both handler sections, preserving the existing timestamp
calculations after normalization.
- Around line 131-140: Update the channel metrics handlers around
GetPerfMetricChannelTotals and GetPerfMetricChannelModelDetails to use a
pkg/perf_metrics query path that includes active hotBuckets, rather than
querying persisted rows only. Expose the required channel aggregates through
pkg/perf_metrics, merge matching hot buckets into the totals and model-detail
data before constructing either response, and preserve the existing error and
response behavior.
In `@model/main.go`:
- Around line 270-273: Update migratePerfMetricAddChannelId so it always
reconciles idx_perf_model_group_bucket, even when channel_id already exists, and
propagates failures from dropping the legacy index instead of suppressing them.
Preserve migration compatibility across SQLite, MySQL 5.7.8+, and PostgreSQL
9.6+, and add a regression test covering an interrupted migration that verifies
the corrected index includes channel_id.
In `@model/perf_metric_aggregation_test.go`:
- Around line 15-17: Update the database setup in the affected performance
metric aggregation tests to initialize DB explicitly through the test fixture,
and fail the tests when initialization is unavailable instead of calling t.Skip.
Ensure both database regression cases exercise their compatibility invariants
with the required database, request, user, settings, and cache state
initialized.
In `@model/perf_metric_migration_test.go`:
- Around line 18-24: Restore the process-wide database dialect in both affected
tests: in model/perf_metric_migration_test.go, snapshot the value managed by
common.SetMainDatabaseType alongside DB and restore it during cleanup; in
pkg/perf_metrics/pipeline_test.go, avoid forcing SQLite for an arbitrary
model.DB or snapshot and restore the prior dialect with t.Cleanup. Keep each
fixture’s database and settings state explicitly initialized without leaking
changes to later tests.
In `@model/perf_metric_persistence_test.go`:
- Around line 328-331: Update the cleanup predicate in the deferred function to
use commonGroupCol when referencing the reserved group column, while preserving
the existing bucket_ts condition and deletion behavior.
- Around line 145-176: The NULL-channel upsert test must no longer accept
duplicate model-level rows. Update the test setup and repeated calls to use a
non-null conflict key while keeping the persisted channel_id nullable, then
query that model-level bucket and assert exactly one row with the accumulated
expected counts on every supported database; remove the database-dependent
branching and logging around nullMetrics.
In `@model/perf_metric.go`:
- Around line 17-19: Update the PerfMetric model and its flush/upsert logic so
model-level metrics use a non-null normalized channel conflict key, or route
ChannelId == nil through a separate atomic upsert path that matches existing
buckets. Ensure repeated model-level flushes conflict on the same group and
BucketTs instead of inserting duplicates, while preserving channel-specific
behavior.
- Around line 201-204: Update the migration logic around the column-existence
check in the relevant PerfMetric migration function so legacy unique index
reconciliation always runs, even when channel_id already exists; do not return
early from the HasColumn branch. Ensure removal of idx_perf_model_group_bucket
is attempted in both fresh and partially completed migrations, and propagate any
index-drop error instead of ignoring it before allowing startup to continue.
In `@pkg/perf_metrics/fixture_test.go`:
- Around line 138-142: Update the fixture setup and cleanup around the seeded
channels and logs to avoid writing fixed IDs into shared model.DB. Use an
isolated test database or an owning transaction that is rolled back, initialize
the required database state explicitly, and ensure cleanup only removes records
created by this fixture rather than deleting channels/logs by hard-coded IDs.
In `@pkg/perf_metrics/metrics.go`:
- Around line 264-266: Restore the channel query functions in the performance
metrics layer and update the channel handlers to use them instead of
database-only model queries. Ensure these functions merge persisted channel
totals and details with currently buffered hot-bucket samples before returning
results, matching the existing model endpoint behavior; keep the controller’s
response contract unchanged.
In `@pkg/perf_metrics/pipeline_test.go`:
- Around line 419-433: The bucketStart test currently ignores origBucketSeconds
and hardcodes one-hour expectations, making it configuration-dependent. Update
the test around origBucketSeconds to explicitly initialize the bucket interval
or derive all timestamps and expected boundaries from GetBucketSeconds(), while
preserving deterministic assertions for within-bucket and next-bucket cases.
- Around line 109-160: The test around bucket flushing currently bypasses the
production flush path by calling atomicBucket.drain and model.UpsertPerfMetric
directly. Replace those manual operations with the actual flush operation used
in production, then query the persisted metric and assert the expected
channel_id, counters, latency, and retry-related behavior across both flushes
while retaining the existing record setup.
In `@pkg/perf_metrics/relay_sample_test.go`:
- Around line 28-33: Update the query setup assertions in the relay sample test,
including the analogous block around lines 52–64, to use testify/require.NoError
instead of assert.NoError before inspecting result. Add or reuse the require
import, while leaving the subsequent result assertions unchanged.
---
Outside diff comments:
In `@pkg/perf_metrics/metrics.go`:
- Around line 216-231: Update the hotBuckets iteration in QuerySummaryAll to
skip channel-specific buckets before calling mergeModelTotals and
mergeModelBucket, retaining only channel 0 entries for model summaries. Preserve
the existing timestamp, group, and request-count filters.
In `@router/api-router.go`:
- Around line 35-41: Update the channel-level routes in the perfMetricsRoute
group, specifically /channels and /channels/:channel_id/models/:model_name, to
use an explicit authenticated/admin middleware instead of inheriting public
pricing access. Keep the general pricing middleware for non-channel performance
routes, while ensuring channel telemetry is never anonymously accessible.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 56bcbd24-19d8-4956-90a5-5617e6b7209f
📒 Files selected for processing (18)
.gitignorecontroller/main_test.gocontroller/model_list_test.gocontroller/perf_metrics.gocontroller/perf_metrics_channel_test.gocontroller/token_test.gomodel/main.gomodel/perf_metric.gomodel/perf_metric_aggregation_test.gomodel/perf_metric_migration_test.gomodel/perf_metric_persistence_test.gopkg/perf_metrics/fixture_test.gopkg/perf_metrics/flush.gopkg/perf_metrics/metrics.gopkg/perf_metrics/pipeline_test.gopkg/perf_metrics/relay_sample_test.gopkg/perf_metrics/types.gorouter/api-router.go
| // TestGetChannelPerfMetrics_GroupFilter validates group query parameter filtering. | ||
| func TestGetChannelPerfMetrics_GroupFilter(t *testing.T) { | ||
| t.Skip("group filter requires model package internal initialization (commonGroupCol)") | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Implement the group-filter contract test instead of skipping it.
Seed metrics in two groups, initialize commonGroupCol, and assert that ?group= returns only the selected group. The current test protects no behavior despite group filtering being part of the new API contract.
As per coding guidelines, backend tests must protect real API contracts and initialize database state explicitly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@controller/perf_metrics_channel_test.go` around lines 173 - 176, Replace the
t.Skip in TestGetChannelPerfMetrics_GroupFilter with a real contract test:
initialize commonGroupCol and the required database state explicitly, seed
performance metrics belonging to two distinct groups, request the endpoint with
?group= set to one group, and assert the response contains only that group’s
metrics.
Source: Coding guidelines
| hours := 24 | ||
| if rawHours := c.Query("hours"); rawHours != "" { | ||
| if parsed, err := strconv.Atoi(rawHours); err == nil { | ||
| hours = parsed | ||
| } | ||
| } | ||
| group := c.Query("group") | ||
|
|
||
| startTs := time.Now().Add(-time.Duration(hours) * time.Hour).Unix() | ||
| endTs := time.Now().Unix() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Normalize hours consistently before calculating timestamps.
Unlike perfmetrics.Query, these handlers do not default non-positive values to 24 hours or cap the window at 30 days. Values such as hours=0, negative values, or very large integers produce empty, future, or overflowed time ranges.
Also applies to: 205-214
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@controller/perf_metrics.go` around lines 120 - 129, Normalize the parsed
hours value in the affected handlers before calculating startTs and endTs,
matching perfmetrics.Query: use 24 hours for non-positive values and cap
positive values at 30 days. Apply the same normalization in both handler
sections, preserving the existing timestamp calculations after normalization.
| totals, err := model.GetPerfMetricChannelTotals(startTs, endTs, group) | ||
| if err != nil { | ||
| c.JSON(http.StatusInternalServerError, gin.H{ | ||
| "success": false, | ||
| "message": err.Error(), | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| modelDetails, err := model.GetPerfMetricChannelModelDetails(nil, "", group, startTs, endTs) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Include unflushed hot buckets in channel API results.
These handlers bypass pkg/perf_metrics, querying only persisted rows. Samples in the active bucket therefore disappear from both endpoints until the bucket flushes, unlike the existing metrics query path that merges database rows with hotBuckets.
Expose channel aggregates through pkg/perf_metrics and merge matching hot buckets before constructing either response.
Also applies to: 216-216
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@controller/perf_metrics.go` around lines 131 - 140, Update the channel
metrics handlers around GetPerfMetricChannelTotals and
GetPerfMetricChannelModelDetails to use a pkg/perf_metrics query path that
includes active hotBuckets, rather than querying persisted rows only. Expose the
required channel aggregates through pkg/perf_metrics, merge matching hot buckets
into the totals and model-detail data before constructing either response, and
preserve the existing error and response behavior.
| // Add channel_id column to perf_metrics and rebuild unique index | ||
| if err := migratePerfMetricAddChannelId(); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Make index reconciliation a hard migration invariant.
migratePerfMetricAddChannelId returns early when the column already exists and suppresses failures dropping the legacy index. This call can therefore succeed while idx_perf_model_group_bucket still excludes channel_id, causing different channels to collide. Always reconcile the index and propagate drop failures; add an interrupted-migration regression test.
As per coding guidelines, migrations must support SQLite, MySQL ≥ 5.7.8, and PostgreSQL ≥ 9.6 simultaneously.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@model/main.go` around lines 270 - 273, Update migratePerfMetricAddChannelId
so it always reconciles idx_perf_model_group_bucket, even when channel_id
already exists, and propagates failures from dropping the legacy index instead
of suppressing them. Preserve migration compatibility across SQLite, MySQL
5.7.8+, and PostgreSQL 9.6+, and add a regression test covering an interrupted
migration that verifies the corrected index includes channel_id.
Source: Coding guidelines
| if DB == nil { | ||
| t.Skip("DB not initialized") | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Do not silently skip these database regressions.
Initialize the test database explicitly or fail when unavailable; otherwise CI can report success without exercising either compatibility invariant.
As per coding guidelines, “initialize database, request, user, settings, and cache state explicitly in fixtures.”
Also applies to: 91-93
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@model/perf_metric_aggregation_test.go` around lines 15 - 17, Update the
database setup in the affected performance metric aggregation tests to
initialize DB explicitly through the test fixture, and fail the tests when
initialization is unavailable instead of calling t.Skip. Ensure both database
regression cases exercise their compatibility invariants with the required
database, request, user, settings, and cache state initialized.
Source: Coding guidelines
| // Register cleanup to delete seeded records. | ||
| fixture.cleanup = func() { | ||
| db.Where("id IN ?", []int{5, 6}).Delete(&model.Channel{}) | ||
| db.Where("channel_id IN ?", []int{5, 6}).Delete(&model.Log{}) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not seed fixed IDs into the shared model.DB.
The fixture commits channels 5/6 into whichever database is configured, then deletes every channel and log using those IDs. This can fail against—or erase—pre-existing developer/CI data. Use an isolated test database or an owning transaction that is rolled back.
As per coding guidelines, database state must be initialized explicitly in test fixtures.
Also applies to: 175-184
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/perf_metrics/fixture_test.go` around lines 138 - 142, Update the fixture
setup and cleanup around the seeded channels and logs to avoid writing fixed IDs
into shared model.DB. Use an isolated test database or an owning transaction
that is rolled back, initialize the required database state explicitly, and
ensure cleanup only removes records created by this fixture rather than deleting
channels/logs by hard-coded IDs.
Source: Coding guidelines
| // Note: Channel query functions were removed as controller layer directly uses | ||
| // model.GetPerfMetricChannelTotals and model.GetPerfMetricChannelModelDetails. | ||
| // If hot-bucket merging is needed in the future, implement it in the controller. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Merge hot buckets into channel API queries.
The channel handlers call database-only model queries, so newly recorded samples remain invisible until flushing, unlike the existing model endpoint. Keep channel query functions in this layer to merge persisted and hot data before the controller responds.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/perf_metrics/metrics.go` around lines 264 - 266, Restore the channel
query functions in the performance metrics layer and update the channel handlers
to use them instead of database-only model queries. Ensure these functions merge
persisted channel totals and details with currently buffered hot-bucket samples
before returning results, matching the existing model endpoint behavior; keep
the controller’s response contract unchanged.
| // Manually flush the bucket | ||
| key := bucketKey{ | ||
| model: "test-flush-model", | ||
| channel: channel, | ||
| group: "default", | ||
| bucketTs: bucket, | ||
| } | ||
| value, ok := hotBuckets.Load(key) | ||
| require.True(t, ok, "bucket should exist") | ||
|
|
||
| bucket1 := value.(*atomicBucket) | ||
| drained1 := bucket1.drain() | ||
| require.Equal(t, int64(1), drained1.requestCount) | ||
|
|
||
| err := model.UpsertPerfMetric(&model.PerfMetric{ | ||
| ModelName: key.model, | ||
| ChannelId: &channel, | ||
| Group: key.group, | ||
| BucketTs: key.bucketTs, | ||
| RequestCount: drained1.requestCount, | ||
| SuccessCount: drained1.successCount, | ||
| TotalLatencyMs: drained1.totalLatencyMs, | ||
| }) | ||
| require.NoError(t, err) | ||
|
|
||
| // Record another sample to the same bucket | ||
| Record(Sample{ | ||
| Model: "test-flush-model", | ||
| Group: "default", | ||
| Channel: channel, | ||
| LatencyMs: 200, | ||
| Success: true, | ||
| }) | ||
|
|
||
| // Flush again | ||
| value, ok = hotBuckets.Load(key) | ||
| require.True(t, ok, "bucket should exist again after new record") | ||
|
|
||
| bucket2 := value.(*atomicBucket) | ||
| drained2 := bucket2.drain() | ||
| require.Equal(t, int64(1), drained2.requestCount) | ||
|
|
||
| err = model.UpsertPerfMetric(&model.PerfMetric{ | ||
| ModelName: key.model, | ||
| ChannelId: &channel, | ||
| Group: key.group, | ||
| BucketTs: key.bucketTs, | ||
| RequestCount: drained2.requestCount, | ||
| SuccessCount: drained2.successCount, | ||
| TotalLatencyMs: drained2.totalLatencyMs, | ||
| }) | ||
| require.NoError(t, err) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Exercise the production flush path.
This test manually drains the bucket and calls UpsertPerfMetric, so it still passes if the real flush loses channel_id, counters, or retry state. Invoke the actual flush operation and assert its persisted result.
As per coding guidelines, backend tests must protect real behavior rather than implementation details.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/perf_metrics/pipeline_test.go` around lines 109 - 160, The test around
bucket flushing currently bypasses the production flush path by calling
atomicBucket.drain and model.UpsertPerfMetric directly. Replace those manual
operations with the actual flush operation used in production, then query the
persisted metric and assert the expected channel_id, counters, latency, and
retry-related behavior across both flushes while retaining the existing record
setup.
Source: Coding guidelines
| origBucketSeconds := perf_metrics_setting.GetBucketSeconds() | ||
| defer func() { | ||
| // Note: Cannot restore setting in tests as BucketTime is read-only via config | ||
| _ = origBucketSeconds | ||
| }() | ||
|
|
||
| // Test with 1-hour buckets (3600 seconds) - this is the default "hour" setting | ||
| ts := int64(1672531200) // 2023-01-01 00:00:00 UTC | ||
| assert.Equal(t, int64(1672531200), bucketStart(ts)) | ||
| assert.Equal(t, int64(1672531200), bucketStart(ts+1800)) // +30 minutes | ||
| assert.Equal(t, int64(1672531200), bucketStart(ts+3599)) // +59:59 | ||
| assert.Equal(t, int64(1672534800), bucketStart(ts+3600)) // +1 hour | ||
|
|
||
| // Note: Cannot test 5-minute buckets as BucketTime setting is read-only in tests | ||
| // The setting would need to be configured via environment/config file |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Derive expectations from the configured bucket interval.
The test reads GetBucketSeconds() but ignores it and hardcodes one-hour boundaries. It becomes nondeterministic under any non-hour test configuration. Calculate boundaries from the returned interval or explicitly initialize the setting.
As per coding guidelines, test settings must be initialized explicitly and expected outputs must be deterministic.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/perf_metrics/pipeline_test.go` around lines 419 - 433, The bucketStart
test currently ignores origBucketSeconds and hardcodes one-hour expectations,
making it configuration-dependent. Update the test around origBucketSeconds to
explicitly initialize the bucket interval or derive all timestamps and expected
boundaries from GetBucketSeconds(), while preserving deterministic assertions
for within-bucket and next-bucket cases.
Source: Coding guidelines
| result, err := Query(QueryParams{Model: "gpt-test", ChannelID: &channelID, Hours: 1}) | ||
| assert.NoError(t, err) | ||
| if assert.Len(t, result.Groups, 1) { | ||
| assert.Equal(t, 5, result.Groups[0].ChannelID) | ||
| assert.Equal(t, float64(100), result.Groups[0].SuccessRate) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Make query failures fatal.
Use require.NoError before inspecting result; continuing after a failed query produces misleading assertion output.
As per coding guidelines, “New or substantially rewritten Go backend tests must use testify/require for setup and fatal assertions.”
Proposed fix
import (
"testing"
"time"
"github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
)
...
- assert.NoError(t, err)
+ require.NoError(t, err)Also applies to: 52-64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/perf_metrics/relay_sample_test.go` around lines 28 - 33, Update the query
setup assertions in the relay sample test, including the analogous block around
lines 52–64, to use testify/require.NoError instead of assert.NoError before
inspecting result. Add or reuse the require import, while leaving the subsequent
result assertions unchanged.
Source: Coding guidelines
1 participant
Important
📝 变更描述 / Description
本次变更为 perf_metrics 增加了渠道维度的成功率统计能力，并保持原有模型级统计接口的兼容性。
后端侧做了三部分工作：
1.扩展统计存储维度
在 perf_metrics 聚合链路中加入 channel_id，覆盖采样、内存桶、Redis key、数据库表结构和查询入口，使同一模型在不同渠道下的成功/失败计数可以分别累计，而不会继续混在模型总指标里。
2.新增渠道级查询能力
增加渠道总览和渠道-模型明细所需的数据查询与控制器处理逻辑，用于返回：
3.保持旧接口语义不变
旧的模型级汇总接口继续按模型聚合，不因为引入 channel_id 而把同一模型拆成多条结果；历史数据也不会被错误归属到某个渠道。为此，历史聚合行保留 NULL channel_id，而渠道级查询只统计有真实渠道 ID 的新数据。
同时补充了回归测试和 API 契约测试。
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
📸 运行证明 / Proof of Work
关键验证结果示例：
Summary by CodeRabbit
New Features
Bug Fixes
Tests