Skip to content

refactor(api): take the confidence-badge thresholds from pkg/exchange - #1453

Merged
IAmJSD merged 1 commit into
LeanerCloud:mainfrom
IAmJSD:fix/issue-343-rest
Oct 26, 2026
Merged

IAmJSD merged 1 commit into
LeanerCloud:mainfrom
IAmJSD:fix/issue-343-rest

Conversation

@IAmJSD

@IAmJSD IAmJSD commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator

Closes #343

Why

The confidence badge in the recommendation drawer (confidenceBucketFor) kept its own copies of the 200 / 3 / 50 cut-offs, while pkg/exchange scores the reshape ranking that the badge accompanies. A comment said to alias them once the cloud-commitments-go/pkg pin exported the constants. The pin on main (ff6225d5ff04) now exports exchange.ConfidenceHighSavingsUSD, ConfidenceHighMinCount and ConfidenceMediumSavingsUSD.

What

  • internal/api/handler_recommendations.go: deletes the three local constants. confidenceBucketFor now compares against exchange.ConfidenceHighSavingsUSD, exchange.ConfidenceHighMinCount and exchange.ConfidenceMediumSavingsUSD. The doc comment is updated to match. Behaviour is unchanged.

The other two acceptance items on #343 were already on main:

With this PR, nothing is left on the issue.

Verification

  • No fail-before test: this is a pure refactor with no behaviour change. The existing boundary table in handler_recommendations_test.go pins the literal values (200 / 199.99, count 3 / 2, 50 / 49.99, count 0 and negative). It passes unchanged, which proves the exchange constants equal the old literals.
  • go build ./... && go vet ./... && go vet -tags integration ./... && (cd tests/e2e && go vet -tags e2e ./...): pass.
  • golangci-lint v2.10.1 on ./internal/api/..., with and without --build-tags integration: 0 issues. gocyclo -over 10 internal/api/handler_recommendations.go: nothing reported.
  • go test -race -short -count=1 ./internal/api/: ok.
  • go test -race -tags integration -count=1 ./internal/api/: everything passed except TestRIExchangeTokenRoutes_Integration_MissingAnswersLikeWrongToken and TestApproveRIExchange_Integration_SessionMissingIs404. Both failed while starting their own Postgres container (wait until ready ... context deadline exceeded) on a heavily loaded shared host (load average about 35). Neither test touches the recommendation drawer. On rerun, the first passed. The second hit the same container-start timeout twice, so this run has no local result for it; CI covers it.
  • Secrets scan: clean (planted key flagged). scripts/test-git-secrets-allowlist.sh: 183 PASS, 0 FAIL.

🤖 Generated with Claude Code

confidenceBucketFor kept its own copies of the 200/3/50 cut-offs because
the pinned pkg did not export them yet. The pin (ff6225d5ff04) now exports
exchange.ConfidenceHighSavingsUSD, ConfidenceHighMinCount and
ConfidenceMediumSavingsUSD, so use them directly. The drawer badge and the
reshape ranking it accompanies can no longer drift apart. Behaviour is
unchanged: the boundary table in handler_recommendations_test.go still
pins the literal values.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

  • Run on-demand review

This review includes 1 billable file and costs up to $0.25.

Or wait 59 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 2a11540 and 26fc39a.

Files selected for processing: 1
  • internal/api/handler_recommendations.go
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@IAmJSD IAmJSD added impact/internal Team-internal only type/chore Maintenance / non-user-visible priority/p3 Polish / idea / may never ship severity/low Minor harm urgency/eventually No deadline effort/xs Trivial / one-liner triaged Item has been triaged labels Oct 26, 2026
@IAmJSD

IAmJSD commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE 26fc39abcf506b7d3637c438dfb913b61ceb08ff

Blocking:

  • none

Non-blocking:

  • Closes #343, third acceptance item ("count-0 handling is reconciled"): the two sides still differ (the badge clamps count to 1, confidenceComponent returns a neutral 0.5). Main already documents this as deliberate and pins it with the count-0 and negative-count rows, so "reconciled" here means "documented and tested", not "made equal". I'm fine with that reading. Noting it so the acting owner knows Closes rests on it.
  • handler_recommendations.go:390 and types.go:491-494 (both outside this diff) still say the heuristic "previously lived client-side in frontend/src/recommendations.ts". chore(server/api): duplicated Cache-Control switch and confidence thresholds [platform part] #343 says that copy doesn't exist. This is pre-existing comment drift, so it could be fixed here or later.

Attacked: Read the pinned cloud-commitments-go/pkg@v0.0.0-20261020053130-ff6225d5ff04 exchange/reshape.go. ConfidenceHighSavingsUSD=200.0, ConfidenceHighMinCount=3 and ConfidenceMediumSavingsUSD=50.0 are exported untyped constants equal to the deleted literals, so float/int comparisons are unchanged. Grepped for leftover uses of the deleted lower-case constants: none. Confirmed the other two ACs are on main (setCacheHeaders delegates at internal/server/static.go:42, and the boundary table at handler_recommendations_test.go:798-831 pins literals, so a future change to the exchange constants fails this test). No fail-before applies to a no-behaviour-change refactor. Ran go build ./..., go vet (plain and -tags integration) on ./internal/api/, golangci-lint v2.10.1 on ./internal/api/... (0 issues), go test -race -short -count=1 ./internal/api/ (ok), and go test -race -tags integration -count=1 ./internal/api/ (ok, 594s, every test passed, including the two the PR body could not get a local result for).

Body-stale: none

Reviewer: rD2. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.

@IAmJSD

IAmJSD commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

Local verification at 26fc39abcf506b7d3637c438dfb913b61ceb08ff merged onto main (01fb35a6), together with #1447, #1453, #1455 in that order (combined tree 92b7f1db, Linux, Go 1.26.10): go build ./..., go vet with and without -tags integration, the full go test -race -short -count=1 ./..., and go test -race -tags integration -count=1 on every changed package, against Docker Postgres. All pass. CI Success and pre-commit are green on this head, with 1 approving independent review(s) on it. Nothing was run against any cloud.

@IAmJSD
IAmJSD marked this pull request as ready for review October 26, 2026 10:44
@IAmJSD
IAmJSD requested a review from cristim as a code owner October 26, 2026 10:44
@IAmJSD
IAmJSD merged commit 6a1f4ab into LeanerCloud:main Oct 26, 2026
41 checks passed
@IAmJSD

IAmJSD commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

Merged by IAmJSD's agent at IAmJSD's request (IAmJSD is acting owner while the maintainers are away), under the bar IAmJSD set: 1 approving independent review(s) on 26fc39ab, green CI, and the combined-tree verification above. Squash-merged at exactly 26fc39abcf506b7d3637c438dfb913b61ceb08ff.

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

Labels

effort/xs Trivial / one-liner impact/internal Team-internal only priority/p3 Polish / idea / may never ship severity/low Minor harm triaged Item has been triaged type/chore Maintenance / non-user-visible urgency/eventually No deadline

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore(server/api): duplicated Cache-Control switch and confidence thresholds [platform part]

1 participant