feat: display total skills count on /skills page#76
feat: display total skills count on /skills page#76rknoche6 wants to merge 1 commit intoopenclaw:mainfrom
Conversation
|
@rknoche is attempting to deploy a commit to the Amantus Machina Team on Vercel. A member of the Team first needs to authorize it. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
convex/skills.ts
Outdated
| export const countPublicSkills = query({ | ||
| args: {}, | ||
| handler: async (ctx) => { | ||
| const skills = await ctx.db | ||
| .query('skills') | ||
| .withIndex('by_active_updated', (q) => q.eq('softDeletedAt', undefined)) | ||
| .collect() | ||
| return skills.length |
There was a problem hiding this comment.
[P1] countPublicSkills does a full .collect() and then returns skills.length, which will scan and materialize every active skill doc. On larger datasets this will be slow and can hit Convex limits, especially since /skills mounts it unconditionally via useQuery. Prefer a count/index-driven approach (e.g., maintain an activeSkillsCount in a singleton doc updated on create/soft-delete) rather than fetching all docs just to count them.
Prompt To Fix With AI
This is a comment left during a code review.
Path: convex/skills.ts
Line: 744:751
Comment:
[P1] `countPublicSkills` does a full `.collect()` and then returns `skills.length`, which will scan and materialize *every* active skill doc. On larger datasets this will be slow and can hit Convex limits, especially since `/skills` mounts it unconditionally via `useQuery`. Prefer a count/index-driven approach (e.g., maintain an `activeSkillsCount` in a singleton doc updated on create/soft-delete) rather than fetching all docs just to count them.
How can I resolve this? If you propose a fix, please make it concise.| <h1 className="section-title" style={{ marginBottom: 8 }}> | ||
| Skills | ||
| Skills{' '} | ||
| {totalSkills !== undefined && <span style={{ opacity: 0.5 }}>{totalSkills}</span>} | ||
| </h1> |
There was a problem hiding this comment.
[P3] The header currently renders Skills{' '}{totalSkills...} which shows a bare number with no delimiter/label. This can read as part of the title and may be confusing for screen readers. Consider formatting as e.g. Skills (123) or adding an aria-label/visually-hidden label for the count.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/routes/skills/index.tsx
Line: 228:231
Comment:
[P3] The header currently renders `Skills{' '}{totalSkills...}` which shows a bare number with no delimiter/label. This can read as part of the title and may be confusing for screen readers. Consider formatting as e.g. `Skills (123)` or adding an `aria-label`/visually-hidden label for the count.
How can I resolve this? If you propose a fix, please make it concise.
rknoche6
force-pushed
the
add-skills-count
branch
from
2b81bfc to
eb1963b
Compare
2 participants
Greptile Overview
Greptile Summary
This PR adds a new Convex query (
countPublicSkills) and wires it into the/skillsroute to display a total count in the page header. It also addsundicias a dev dependency (and updatesbun.lock) to fix missing dependency issues in e2e tests, and applies a small Biome-driven import reordering insrc/lib/og.ts.Main integration point is
src/routes/skills/index.tsx, which now callsuseQuery(api.skills.countPublicSkills)alongside the existing paginated public skills query to show the total.Confidence Score: 3/5
countPublicSkillscurrently loads all active skills into memory to count them, which can degrade performance and potentially hit Convex query limits on larger datasets since it’s fetched on every/skillspage load.(2/5) Greptile learns from your feedback when you react with thumbs up/down!
Context used:
dashboard- AGENTS.md (source)