🔧 Remove refresh option and always force refresh in contract report #2174
Conversation
📝 docs(contract_report): Update comment for forceRefresh clarification 🔧 fix: remove trailing whitespace in contract report comments 🔧 fix: remove trailing whitespace in contract report comments
There was a problem hiding this comment.
Pull request overview
This PR removes the /contract-report refresh slash-command option and changes contract report generation to always force-refresh contract evaluations when fetching archives, updating related user messaging and inline documentation accordingly.
Changes:
- Removed the
refreshboolean option from the/contract-reportcommand definition. - Updated error messaging and function documentation to no longer reference
refresh=true. - Set
forceRefreshto always betrueinContractReport.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return "No contract found in this channel. Please provide a contract-id." | ||
| case errors.Is(err, ErrEvaluationNotFound): | ||
| return "Evaluation not found, if you just completed the contract please wait a few minutes and try again with refresh=true." | ||
| return "Evaluation not found, if you just completed the contract please wait a few minutes and try again." |
There was a problem hiding this comment.
This user-facing string is a run-on sentence and reads awkwardly. Consider splitting it into two sentences (e.g., end after "not found") to improve clarity.
| return "Evaluation not found, if you just completed the contract please wait a few minutes and try again." | |
| return "Evaluation not found. If you just completed the contract, please wait a few minutes and try again." |
src/boost/contract_report.go
Outdated
| // Always want forceRefresh true to ensure we have the latest contract evaluations, | ||
| // especially for the caller. Caching is handled at the individual API fetch level | ||
| // in processContributors. | ||
| forceRefresh := true | ||
|
|
There was a problem hiding this comment.
The new comment and forceRefresh := true line include trailing whitespace; CI enforces gofmt (diff must be empty), so this will fail formatting checks. Run gofmt on this file or remove trailing spaces.
src/boost/contract_report.go
Outdated
| // Always want forceRefresh true to ensure we have the latest contract evaluations, | ||
| // especially for the caller. Caching is handled at the individual API fetch level | ||
| // in processContributors. | ||
| forceRefresh := true | ||
|
|
There was a problem hiding this comment.
Setting forceRefresh := true unconditionally disables the 1-hour on-disk cache in ei.GetContractArchiveFromAPI, so every /contract-report run will re-fetch archives for the caller and each contributor. This will deterministically increase external API traffic and latency (and may hit rate limits) compared to the previous default-cached behavior. If the intent is only to refresh the caller’s view, consider using forceRefresh=true for the caller fetch but forceRefresh=false when calling processContributors, or make refresh conditional (e.g., only when the evaluation is missing).
mkmccarty
force-pushed
the
mm-branch-1
branch
from
16ea09f to
b21cb31
Compare
1 participant
No description provided.