-
Notifications
You must be signed in to change notification settings - Fork 692
[deprecated][fix][infra]change CheckAndDeductOptimizationBenefit struct to fit special task #249
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…imizationBenefit Co-Authored-By: Coda <[email protected]>
…imizationBenefit Co-Authored-By: Coda <[email protected]>
…/coze-loop into feat/optimize_task_prompt_pilot
…ontrol Generated with Coda Co-Authored-By: Coda <[email protected]>
…t execution trace control
…imizationBenefit Co-Authored-By: Coda <[email protected]>
…ontrol Generated with Coda Co-Authored-By: Coda <[email protected]>
…t execution trace control
…/coze-loop into feat/optimize_task_prompt_pilot
This reverts commit ea002c3.
…imizationBenefit Co-Authored-By: Coda <[email protected]>
…ontrol Generated with Coda Co-Authored-By: Coda <[email protected]>
…t execution trace control
…ontrol Generated with Coda Co-Authored-By: Coda <[email protected]>
This reverts commit ea002c3.
…/coze-loop into feat/optimize_task_prompt_pilot
…imizationBenefit Co-Authored-By: Coda <[email protected]>
…ontrol Generated with Coda Co-Authored-By: Coda <[email protected]>
…t execution trace control
…ontrol Generated with Coda Co-Authored-By: Coda <[email protected]>
This reverts commit ea002c3.
# Conflicts: # backend/infra/external/benefit/benifit.go # backend/infra/external/benefit/noop.go
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #249 +/- ##
==========================================
+ Coverage 65.66% 65.69% +0.02%
==========================================
Files 504 507 +3
Lines 57307 57718 +411
==========================================
+ Hits 37629 37915 +286
- Misses 17103 17197 +94
- Partials 2575 2606 +31
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 18 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary: Requesting changes due to numeric type inconsistency (float64 vs int64) for resource points and enum robustness/naming for When. See inline comments.
- Must-Fix: Use integer/decimal type for resource points consistently across check/deduct paths to avoid precision drift.
- Should-Fix: Rename and validate enum
When(suggestOptPhasewithValid()), avoid ambiguous zero value. - Suggestion: Mark deprecated API with
Deprecated:for tooling support.
Thanks for the clear scope and minimal diff. Once addressed, the API surface will be more robust and consistent.
What type of PR is this?
feat: change a benefit struct to fit special task
Check the PR title.
(Optional) Translate the PR title into Chinese.
(Optional) More detailed description for this PR(en: English/zh: Chinese).
en:
zh(optional):
(Optional) Which issue(s) this PR fixes: