Deduplicate item-record casts and tighten docs/package hygiene#65
Conversation
Two coordinated, zero-behavior-change hygiene workstreams tracked under the
pm-rnpb remediation feature, delivered by specialized sub-agents.
Code dedup (pm-p5if, slice of pm-mbdu):
- Add src/core/item/item-record.ts exporting toItemRecord(item: ItemMetadata):
Record<string, unknown> — a single documented home for the item/metadata ->
generic-record TYPE widening that was scattered as `as unknown as
Record<string, unknown>` across the codebase. This is distinct from the
runtime asRecord* guards consolidated earlier in pm-why9 (those validate
typeof at runtime; this is a compile-time-only widening returning the same
reference).
- Replace 16 homogeneous widening casts across 13 files: append, claim, close,
delete, get, list, normalize, search, update-many, update (commands) plus
history and replay (core). Heterogeneous casts (frontMatter/normalized/event/
definition/frame/plan/hit/settings) intentionally left as-is.
- Add tests/unit/item-record.spec.ts and register the module in vitest `include`
(100% statements/branches/functions/lines).
Docs/packaging hygiene (pm-rjgh):
- Drop PRD.md (~117KB) from package.json `files[]` so it no longer ships to
every npm install (still tracked in the repo). No publish-files test required
it.
- Reconcile the drifted root marketplace.json to the canonical
.claude-plugin/marketplace.json schema (metadata.{description,version} +
plugin `category`); the two files are now byte-identical. claude-plugin
contract + smoke assertions unchanged and green.
- AGENTS.md left as-is (already a low-context entrypoint deferring to
docs/AGENT_GUIDE.md; its quickstart block is injector-managed).
- The "slim CHANGELOG" sub-goal is superseded: CHANGELOG.md is now a fully
pm-changelog-generated artifact (PR #62); regenerated here so pm-p5if/pm-rjgh
appear under [Unreleased].
Verification: full release gate green — build, typecheck, docs-skills,
static-quality, coverage (1740 tests / 115 files, 100%), version-policy,
secret-scan, npx-smoke, package-first-dogfood, npm-pack-dry-run, compatibility,
and sentry-telemetry (Sentry critical=0/high=4 under thresholds; telemetry
finish_error_rate 2.49%, required mode). Manual temp-dir dogfood exercised every
toItemRecord path (get --fields/claim/update/append/list/search/history/
normalize/close/delete) with no broken functionality.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Warning Review limit reached
Your plan includes 1 review of capacity. Refill in 35 minutes and 10 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR consolidates homogeneous ChangesType-widening cast centralization
Publishing and marketplace updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request centralizes TypeScript type-widening casts by introducing a toItemRecord helper, replacing numerous inline 'as unknown as Record' casts across the CLI and core modules. It also performs documentation and packaging hygiene, including the reconciliation of marketplace.json and the removal of PRD.md from the published npm package. Feedback suggests reusing an existing variable in the update command to avoid redundant helper calls.
| try { | ||
| applyRegisteredItemFieldDefaultsAndValidation( | ||
| document.metadata as unknown as Record<string, unknown>, | ||
| toItemRecord(document.metadata), |
There was a problem hiding this comment.
The metadataRecord variable (defined at line 1552) already holds the widened reference to document.metadata. You can use it here instead of calling toItemRecord again, which is more efficient and consistent with the rest of the mutate callback.
| toItemRecord(document.metadata), | |
| metadataRecord, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
marketplace.json (1)
7-10: ⚡ Quick winAdd contract assertions for the newly introduced manifest fields.
Current contract/smoke checks (from
tests/integration/claude-plugin-contract.spec.tsandscripts/smoke-claude-plugin.mjs) validatename/plugin wiring but do not assertmetadata.version,metadata.description, or plugincategory. Please add explicit assertions so this schema change can’t silently drift again.Also applies to: 35-35
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@marketplace.json` around lines 7 - 10, Update the Claude plugin contract/smoke tests to assert the new manifest fields: add explicit checks for metadata.version and metadata.description and for the plugin's category value; e.g., in tests/integration/claude-plugin-contract.spec.ts add assertions that manifest.metadata.version equals the expected version and manifest.metadata.description is non-empty (or matches the expected string) and that manifest.category equals the expected category, and mirror the same assertions in scripts/smoke-claude-plugin.mjs so both integration and smoke checks fail if those fields drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@marketplace.json`:
- Around line 7-10: Update the Claude plugin contract/smoke tests to assert the
new manifest fields: add explicit checks for metadata.version and
metadata.description and for the plugin's category value; e.g., in
tests/integration/claude-plugin-contract.spec.ts add assertions that
manifest.metadata.version equals the expected version and
manifest.metadata.description is non-empty (or matches the expected string) and
that manifest.category equals the expected category, and mirror the same
assertions in scripts/smoke-claude-plugin.mjs so both integration and smoke
checks fail if those fields drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: a455f8a3-0d02-42ff-ae41-606febdc3856
📒 Files selected for processing (24)
.agents/pm/chores/pm-mbdu.toon.agents/pm/chores/pm-p5if.toon.agents/pm/chores/pm-rjgh.toon.agents/pm/history/pm-mbdu.jsonl.agents/pm/history/pm-p5if.jsonl.agents/pm/history/pm-rjgh.jsonlCHANGELOG.mdmarketplace.jsonpackage.jsonsrc/cli/commands/append.tssrc/cli/commands/claim.tssrc/cli/commands/close.tssrc/cli/commands/delete.tssrc/cli/commands/get.tssrc/cli/commands/list.tssrc/cli/commands/normalize.tssrc/cli/commands/search.tssrc/cli/commands/update-many.tssrc/cli/commands/update.tssrc/core/history/history.tssrc/core/history/replay.tssrc/core/item/item-record.tstests/unit/item-record.spec.tsvitest.config.ts
💤 Files with no reviewable changes (1)
- package.json
- update.ts (Gemini): reuse the in-scope `metadataRecord` (the widened `document.metadata` reference held since the start of the mutate callback) instead of re-calling `toItemRecord(document.metadata)` at the registered- field validation site, and at the 4 sibling `(document.metadata as Record)` index casts. Removes 5 more inline casts; behavior-identical (same reference, never reassigned). The out-of-callback cast at the result-projection site is left as-is. - claude-plugin-contract test (CodeRabbit): assert the reconciled marketplace.json manifest fields (metadata block + description + version === plugin version, plugin category) and add an invariant test that root marketplace.json deep-equals .claude-plugin/marketplace.json so the two manifests can never drift again. Local gate green: build, typecheck, docs-skills, static-quality, coverage (1741 tests / 115 files, 100%), version-policy, secret-scan, npx-smoke, package-first-dogfood, npm-pack-dry-run, compatibility. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 participant
Summary
Two coordinated, zero-behavior-change hygiene workstreams (continuation of active work under the
pm-rnpbremediation feature), each implemented by a specialized sub-agent and tracked end-to-end in the pm CLI.1. Item-record cast dedup —
pm-p5if(slice ofpm-mbdu)The repeated
<x> as unknown as Record<string, unknown>type-widening casts that bridge a typedItemMetadatainto the generic record shape were scattered across the codebase. These are distinct from the runtimeasRecord*guards already consolidated inpm-why9(those validatetypeofat runtime; this is a compile-time-only widening that returns the same reference).src/core/item/item-record.ts→toItemRecord(item: ItemMetadata): Record<string, unknown>— one documented home for the widening.append,claim,close,delete,get,list,normalize,search,update-many,update(commands) +history,replay(core).frontMatter/normalized/event/definition/frame/plan/hit/settings) intentionally left as-is.tests/unit/item-record.spec.ts; module added to vitestincludeat 100% coverage.2. Docs / packaging hygiene —
pm-rjghPRD.md(~117KB) frompackage.jsonfiles[]— it shipped to everynpm installand duplicateddocs/. Still tracked in the repo; no publish-files test required it.marketplace.jsonto the canonical.claude-plugin/marketplace.jsonschema (metadata.{description,version}+ plugincategory). The two files are now byte-identical; the claude-plugin contract + smoke assertions are unchanged and green.AGENTS.mdleft as-is (already a low-context entrypoint deferring todocs/AGENT_GUIDE.md; its quickstart block is injector-managed).CHANGELOG.mdis now a fullypm-changelog-generated artifact (Use pm-changelog for full historical CHANGELOG #62), regenerated here so the two new items land under[Unreleased].Verification
toItemRecordpath (get --fields/claim/update/append/list/search/history/normalize/close/delete) — no broken functionality.npm pack --dry-runconfirmsPRD.mdgone and both marketplace files still shipped.pm tracking
pm-p5if(closed),pm-rjgh(closed) with full resolution/expected/actual.pm-mbdukept open for the remaining work (barrel-split the 3 files still >2000 LOC:extension.ts,loader.ts,sdk/cli-contracts.ts).🤖 Generated with Claude Code