refactor: extract readCanonicalStat and add structural guards for stat field migration

[momothemage]

Summary

This PR introduces readCanonicalStat() as the single canonical way to read migrated stat fields from a skills document, and adds schema-level @deprecated annotations to discourage direct access to the legacy nested fields.

Background

The skills table is mid-migration: four stat fields (downloads, stars, installsCurrent, installsAllTime) are being promoted from the nested stats.* object to top-level indexable fields (statsDownloads, statsStars, etc.). During the transition, both sets of fields coexist, and reads must prefer the top-level field while falling back to the nested field for pre-migration documents.

Previously, this fallback logic was duplicated inline in applySkillStatDeltas() and scattered across callers. This PR centralizes it.

Changes

convex/lib/skillStats.ts

• Extracts readCanonicalStat(skill, field): a typed helper that reads the top-level field when present, and falls back to skill.stats[field] for pre-migration documents.
• Refactors applySkillStatDeltas() to use readCanonicalStat() internally, removing ~12 lines of duplicated fallback logic.

convex/schema.ts

• Adds a JSDoc comment to statsValidator explaining the migration context.
• Marks the four legacy nested fields (downloads, stars, installsCurrent, installsAllTime) as @deprecated with pointers to their top-level replacements, so IDE tooling surfaces a strikethrough warning on direct access.

AGENTS.md

• Documents the Stat Field Migration Rules for future contributors and AI agents: always use readCanonicalStat() to read, always use applySkillStatDeltas() to write, and never access the deprecated nested fields directly.

Testing

No behavior change — this is a pure refactor. The fallback logic in readCanonicalStat() is equivalent to the inline logic it replaces.

[vercel]

@momothemage is attempting to deploy a commit to the Amantus Machina Team on Vercel.

A member of the Team first needs to authorize it.

[momothemage]

@codex review

[greptile-apps]

Greptile Summary

This PR extracts readCanonicalStat() as a typed helper that centralizes the top-level-field-with-nested-fallback logic for the four migrated stat fields, refactors applySkillStatDeltas() to use it, and adds @deprecated JSDoc annotations plus AGENTS.md documentation. The implementation is correct and the refactor is behavior-preserving.

Two P2 follow-up items: the @deprecated annotations live on v.object() validator properties rather than TypeScript type declarations, so whether they propagate as IDE strikethroughs depends on Convex codegen behavior that should be verified; and two existing callers (statsMaintenance.ts:287, search.ts:253,258) still use the legacy pattern now documented as forbidden in AGENTS.md.

Confidence Score: 5/5

Safe to merge — pure refactor with no behavior change and correct implementation.

All findings are P2: one about the uncertain effectiveness of @deprecated JSDoc in the Convex codegen pipeline, and one about pre-existing callers not yet migrated to readCanonicalStat(). Neither blocks merge. The core extraction logic is correct and the refactor faithfully preserves existing behavior.

No files require special attention for merge; follow-up migration of statsMaintenance.ts and search.ts is recommended post-merge.

Comments Outside Diff (1)

1. convex/statsMaintenance.ts, line 286-287 (link)

   Pre-existing duplicate of readCanonicalStat fallback logic

   statsMaintenance.ts:287 still uses the inline ternary pattern (typeof skill.statsStars === "number" ? skill.statsStars : skill.stats.stars) that readCanonicalStat() was introduced to replace, and search.ts:253,258 reads entry.skill.stats.downloads directly without any top-level fallback. These are pre-existing callers that now violate the AGENTS.md rule added in this PR. A follow-up to migrate them would make the pattern fully consistent.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: convex/statsMaintenance.ts
   Line: 286-287
   
   Comment:
   **Pre-existing duplicate of `readCanonicalStat` fallback logic**
   
   `statsMaintenance.ts:287` still uses the inline ternary pattern (`typeof skill.statsStars === "number" ? skill.statsStars : skill.stats.stars`) that `readCanonicalStat()` was introduced to replace, and `search.ts:253,258` reads `entry.skill.stats.downloads` directly without any top-level fallback. These are pre-existing callers that now violate the AGENTS.md rule added in this PR. A follow-up to migrate them would make the pattern fully consistent.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: convex/schema.ts
Line: 104-112

Comment:
**`@deprecated` annotations may not surface in IDE tooling**

The `@deprecated` JSDoc tags are placed on object literal properties passed to `v.object()`. TypeScript/IDE strikethrough warnings for `@deprecated` only activate on accesses to properties whose *type declarations* carry the tag — for example, an `interface` or `type` field. Convex codegen generates `Doc<"skills">` from the schema validators at build time, and is unlikely to carry these JSDoc comments through to the generated types in `_generated/dataModel.ts`.

If the generated types don't include `@deprecated`, developers accessing `skill.stats.downloads` in other files will see no strikethrough, and the AGENTS.md claim ("Any IDE access to `skill.stats.downloads` etc. will show a strikethrough warning") would be inaccurate. It's worth verifying against the actual generated output, and if the strikethrough doesn't propagate, updating AGENTS.md accordingly so contributors aren't confused when they don't see it.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: convex/statsMaintenance.ts
Line: 286-287

Comment:
**Pre-existing duplicate of `readCanonicalStat` fallback logic**

`statsMaintenance.ts:287` still uses the inline ternary pattern (`typeof skill.statsStars === "number" ? skill.statsStars : skill.stats.stars`) that `readCanonicalStat()` was introduced to replace, and `search.ts:253,258` reads `entry.skill.stats.downloads` directly without any top-level fallback. These are pre-existing callers that now violate the AGENTS.md rule added in this PR. A follow-up to migrate them would make the pattern fully consistent.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "refactor: extract readCanonicalStat and ..." | Re-trigger Greptile}

[greptile-apps]

@deprecated annotations may not surface in IDE tooling

The @deprecated JSDoc tags are placed on object literal properties passed to v.object(). TypeScript/IDE strikethrough warnings for @deprecated only activate on accesses to properties whose type declarations carry the tag — for example, an interface or type field. Convex codegen generates Doc<"skills"> from the schema validators at build time, and is unlikely to carry these JSDoc comments through to the generated types in _generated/dataModel.ts.

If the generated types don't include @deprecated, developers accessing skill.stats.downloads in other files will see no strikethrough, and the AGENTS.md claim ("Any IDE access to skill.stats.downloads etc. will show a strikethrough warning") would be inaccurate. It's worth verifying against the actual generated output, and if the strikethrough doesn't propagate, updating AGENTS.md accordingly so contributors aren't confused when they don't see it.

Prompt To Fix With AI

This is a comment left during a code review.
Path: convex/schema.ts
Line: 104-112

Comment:
**`@deprecated` annotations may not surface in IDE tooling**

The `@deprecated` JSDoc tags are placed on object literal properties passed to `v.object()`. TypeScript/IDE strikethrough warnings for `@deprecated` only activate on accesses to properties whose *type declarations* carry the tag — for example, an `interface` or `type` field. Convex codegen generates `Doc<"skills">` from the schema validators at build time, and is unlikely to carry these JSDoc comments through to the generated types in `_generated/dataModel.ts`.

If the generated types don't include `@deprecated`, developers accessing `skill.stats.downloads` in other files will see no strikethrough, and the AGENTS.md claim ("Any IDE access to `skill.stats.downloads` etc. will show a strikethrough warning") would be inaccurate. It's worth verifying against the actual generated output, and if the strikethrough doesn't propagate, updating AGENTS.md accordingly so contributors aren't confused when they don't see it.

How can I resolve this? If you propose a fix, please make it concise.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. 👍

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[hxy91819]

Thanks for the follow-up PR — the readCanonicalStat extraction, @deprecated annotations, and AGENTS.md rules are exactly the structural guards we discussed. Clean work.

On second look, this PR's scope is clear: extract the helper, add deprecation annotations, document the rules. Migrating callers like statsMaintenance.ts is a natural follow-up, not a blocker here.

LGTM.

[hxy91819]

Thanks for the follow-up PR — the readCanonicalStat extraction, @deprecated annotations, and AGENTS.md rules are exactly the structural guards we discussed. Clean work.

On second look, this PR's scope is clear: extract the helper, add deprecation annotations, document the rules. Migrating callers like statsMaintenance.ts is a natural follow-up, not a blocker here.

LGTM.

[momothemage]

Merged via squash.

• Prepared head SHA: e92817f66f44254f78a27df12022bf7a1b1af5b9
• Merge commit: 530e39eedc70ef81a68279d1adb799f221267a39

Thanks @momothemage!