feat(us4): drop project_settings subsystem (PR-v5-4)#184
Conversation
Removes the per-project adaptive threshold infrastructure: - Migration 097_drop_project_settings (DROP TABLE project_settings; per plan.md C3 rollback returns IRREVERSIBLE error — data was derived from learning signals) - Delete internal/db/gorm/project_settings_store.go (ProjectSettings model + ProjectSettingsStore type + GetThreshold/AdjustThreshold/ UpsertSettings methods) - internal/search/manager.go: remove projectSettingsStore field and SetProjectSettingsStore method; GetProjectThreshold becomes a thin wrapper returning globalDefault (kept as a wrapper so callers in internal/worker don't need to change — remove together in a later US if wanted) - internal/worker/service.go: remove projectSettingsStore field + init + SetProjectSettingsStore call (3 lines total) - internal/worker/handlers_scoring.go: remove the adaptive threshold adjustment block in the utility-signal handler (it was the only caller of ProjectSettingsStore.AdjustThreshold) - internal/worker/reaper/reaper.go: drop the project_settings entry from the ''tables related to projects.id'' comment Verified: go build ./... — clean go vet ./... — clean go test -short -count=1 ./... — green Per plan.md C7: CodeRabbit-only review. Per plan.md C8: single coherent PR, no atomic-commit split. Plan.md §Phase 4. Closes task #42.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (3)
ОбзорУдаляет функциональность адаптивной регулировки порогов для конкретных проектов путём удаления таблицы БД Изменения
Ожидаемый объём работы при проверке кода🎯 2 (Simple) | ⏱️ ~12 минут Возможно связанные PR
Стихотворение
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.4)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes per-project adaptive thresholds by dropping the project_settings database table and removing the associated ProjectSettingsStore and logic across the search manager and worker service. The changes include a new migration to drop the table and updates to the scoring handlers to remove threshold adjustment logic. Feedback focuses on improving Go idiomaticity by using blank identifiers for unused parameters in GetProjectThreshold and merging redundant lock/unlock blocks in the worker service initialization.
| func (m *Manager) GetProjectThreshold(ctx context.Context, project string, globalDefault float64) float64 { | ||
| if m.projectSettingsStore == nil { | ||
| return globalDefault | ||
| } | ||
| threshold, err := m.projectSettingsStore.GetThreshold(ctx, project) | ||
| if err != nil { | ||
| return globalDefault | ||
| } | ||
| // If the stored threshold equals the default (0.3), honor globalDefault | ||
| // in case the operator configured a higher global threshold. | ||
| if threshold == 0.3 { | ||
| return globalDefault | ||
| } | ||
| return threshold | ||
| _ = ctx | ||
| _ = project | ||
| return globalDefault | ||
| } |
There was a problem hiding this comment.
For unused parameters in a function signature, it is more idiomatic in Go to use the blank identifier _ directly in the parameter list rather than using _ = parameter in the function body. This clearly signals to both the compiler and other developers that the parameters are intentionally ignored while maintaining the required signature for compatibility.
func (m *Manager) GetProjectThreshold(_ context.Context, _ string, globalDefault float64) float64 {
return globalDefault
}| s.grpcServer = grpcSrv | ||
| s.initMu.Unlock() | ||
|
|
||
| // Wire project settings store for adaptive per-project thresholds. | ||
| projectSettingsStore := gorm.NewProjectSettingsStore(store.DB) | ||
| searchMgr.SetProjectSettingsStore(projectSettingsStore) | ||
| s.projectSettingsStore = projectSettingsStore | ||
|
|
||
| s.initMu.Lock() | ||
| s.searchMgr = searchMgr |
There was a problem hiding this comment.
After removing the projectSettingsStore initialization logic, this Unlock() and subsequent Lock() sequence has become redundant. Merging these two critical sections into one improves readability and slightly reduces overhead by avoiding an unnecessary lock release and re-acquisition during the initialization phase.
| s.grpcServer = grpcSrv | |
| s.initMu.Unlock() | |
| // Wire project settings store for adaptive per-project thresholds. | |
| projectSettingsStore := gorm.NewProjectSettingsStore(store.DB) | |
| searchMgr.SetProjectSettingsStore(projectSettingsStore) | |
| s.projectSettingsStore = projectSettingsStore | |
| s.initMu.Lock() | |
| s.searchMgr = searchMgr | |
| s.grpcServer = grpcSrv | |
| s.searchMgr = searchMgr |
1 participant
Scope: US4 per plan.md §Phase 4 — drop project_settings table + ProjectSettingsStore Go type + adaptive-threshold caller code.
Changes
097_drop_project_settings(per-C3 rollback returns error)internal/db/gorm/project_settings_store.gointernal/search/manager.go: remove projectSettingsStore field + Set method; GetProjectThreshold thin-wraps globalDefault (callers unchanged)internal/worker/service.go: remove field + init + Set callinternal/worker/handlers_scoring.go: remove AdjustThreshold block (was only caller)internal/worker/reaper/reaper.go: drop project_settings comment entryVerified
go build ./...greengo vet ./...cleango test -short -count=1 ./...greenConventions compliance (plan.md §Cross-Phase Conventions)
Non-atomic diff
6 files changed, +28/-130. Clean pure-delete PR per plan.md C8 ''larger PRs''.
Safety gates (NFR-2)
Summary by CodeRabbit
Изменения