-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
perf(templates): improve templates loading performance #3099
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: develop
Are you sure you want to change the base?
Conversation
98fb55e
to
c8cbe35
Compare
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.
Pull Request Overview
This PR improves templates loading performance by adding pagination and deferring vault data retrieval.
- Limit the number of templates displayed per page in the UI to 100 and restore footer pagination controls.
- Introduce a
loadVaults
boolean flag inGetTemplates
to skip loading vaults by default and update all call sites accordingly. - Enhance the SQL implementation of
GetTemplates
withLIMIT
/OFFSET
support and optimize handling of emptytaskIDs
.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
web/src/views/project/Templates.vue | Limit items per page to 100 and remove hide-default-footer . |
services/tasks/TaskRunner.go | Updated import ordering and GetTemplates call to include loadVaults=true . |
services/project/backup.go | Pass loadVaults=true for backup loading of templates. |
db/sql/template.go | Extended GetTemplates signature with loadVaults , added LIMIT /OFFSET , and conditional vault loading. |
db/sql/task.go | Added early return for empty taskIDs (pointer assignment may be incorrect). |
db/sql/project.go | Removed obsolete commented-out template-loading code. |
db/bolt/template.go | Updated Bolt store GetTemplates signature with loadVaults . |
db/Store.go | Updated TemplateManager interface to include loadVaults . |
api/projects/views.go | Pass loadVaults=false in GetViewTemplates . |
api/projects/templates.go | Use QueryParams variable and pass loadVaults=false . |
Comments suppressed due to low confidence (1)
db/sql/template.go:233
- New pagination logic (
LIMIT
/OFFSET
) has been added; consider adding unit tests to verify behavior whenparams.Count
andparams.Offset
are set.
if params.Count > 0 {
class="mt-4 templates-table" | ||
single-expand | ||
show-expand | ||
:headers="filteredHeaders" | ||
:items="items" | ||
:items-per-page="Number.MAX_VALUE" | ||
:items-per-page="100" |
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.
[nitpick] Consider extracting the magic number 100
into a constant or prop to make the page size configurable and improve readability.
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <[email protected]>
@cursor review |
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.
Bug: Missing Data Population and Error Handling
The tsk.Fill(d)
call and its error handling were removed from the GetTemplates
function. This method was responsible for populating additional data for the TaskWithTpl
object. As a result, the template.LastTask
field will now contain incomplete information, potentially leading to missing data for API consumers and masking underlying data loading issues due to the removed error handling.
db/sql/template.go#L275-L280
Lines 275 to 280 in 510e68a
for _, tsk := range tasks { | |
if tsk.ID == *tpl.LastTaskID { | |
template.LastTask = &tsk | |
break | |
} | |
} |
Bugbot free trial expires on August 10, 2025
Learn more in the Cursor dashboard.
No description provided.