Skip to content

refactor: extract storage record utils#259

Closed
ndycode wants to merge 1 commit intorefactor/pr3-storage-account-match-utilsfrom
refactor/pr3-storage-record-guards
Closed

refactor: extract storage record utils#259
ndycode wants to merge 1 commit intorefactor/pr3-storage-account-match-utilsfrom
refactor/pr3-storage-record-guards

Conversation

@ndycode
Copy link
Copy Markdown
Owner

@ndycode ndycode commented Mar 21, 2026

Summary

  • extract simple record/index utility helpers out of lib/storage.ts
  • keep the existing normalization behavior intact while continuing the storage split in tiny slices

What Changed

  • added lib/storage/record-utils.ts
  • moved isRecord(...) and clampIndex(...) into the new helper module
  • updated lib/storage.ts to use the extracted helper without changing storage behavior

Validation

  • npm run test -- test/storage.test.ts
  • npm run lint
  • npm run typecheck
  • npm run build

Risk and Rollback

  • Risk level: low
  • Rollback plan: revert 2ede713 to restore the inline record/index helpers in lib/storage.ts

Additional Notes

  • this keeps the storage split moving with another tiny, reviewer-friendly helper extraction

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

pure mechanical extraction: isRecord and clampIndex are moved verbatim from lib/storage.ts into the new lib/storage/record-utils.ts helper module. the import is wired up correctly and no storage behavior changes. this continues the ongoing lib/storage.ts split into reviewable slices.

changes:

  • lib/storage/record-utils.ts — new file exporting isRecord and clampIndex (8 lines, direct copy)
  • lib/storage.ts — removes the two private definitions, adds the import; all 11 call sites are untouched

concern:

  • no test/storage-record-utils.test.ts added; isRecord and clampIndex are indirectly exercised by test/storage.test.ts but have no direct unit-level edge case coverage — follow the pattern set by test/storage-flagged.test.ts and test/named-backup-export.test.ts

Confidence Score: 5/5

  • safe to merge — code is a verbatim copy with no behavioral change and no token/filesystem risk surface
  • both functions are copied exactly from storage.ts, the import resolves correctly, and all 11 call sites in storage.ts remain untouched. the only gap is missing dedicated unit tests, which is a follow-up concern rather than a merge blocker given the 80% threshold is still met via indirect coverage through storage.test.ts.
  • no files require special attention

Important Files Changed

Filename Overview
lib/storage/record-utils.ts new helper module exporting isRecord and clampIndex, copied verbatim from lib/storage.ts — logic is correct but no dedicated vitest unit test file added
lib/storage.ts cleanly replaces the two private function definitions with a single import from ./storage/record-utils.js; no behavioral change, all call sites preserved

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    ST["lib/storage.ts"]
    RU["lib/storage/record-utils.ts\n─────────────────────\nisRecord(value)\nclampIndex(index, length)"]

    ST -->|"import { isRecord, clampIndex }"| RU
    ST -->|"11 call sites unchanged"| ST
Loading

Fix All in Codex

Prompt To Fix All With AI
This is a comment left during a code review.
Path: lib/storage/record-utils.ts
Line: 1-8

Comment:
**missing dedicated vitest unit tests**

`isRecord` and `clampIndex` are now public exports with no test file for `lib/storage/record-utils.ts`. they're exercised indirectly through `test/storage.test.ts` integration paths, but direct edge-case unit coverage is absent — e.g. `isRecord(null)`, `isRecord([])`, `clampIndex(-1, 5)`, `clampIndex(99, 0)`, `clampIndex(2, 3)`. the project enforces an 80% coverage threshold across all modules; as more of `lib/storage.ts` is extracted into standalone helpers, each new file needs its own vitest suite to keep that bar green. similar extractions like `storage/flagged-storage-file.ts` and `storage/named-backups.ts` already have dedicated `test/storage-flagged.test.ts` and `test/named-backup-export.test.ts` — follow the same pattern here with a `test/storage-record-utils.test.ts`.

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

Last reviewed commit: "refactor: extract st..."

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 21, 2026

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)
  • skip-review

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4dab14c8-b1b5-4c14-abd5-eda491a9f5f8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/pr3-storage-record-guards
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch refactor/pr3-storage-record-guards

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ndycode ndycode added the passed label Mar 21, 2026
ndycode added a commit that referenced this pull request Mar 23, 2026
@ndycode
Copy link
Copy Markdown
Owner Author

ndycode commented Mar 23, 2026

Closing because this work is now included in main via #318 and #319.

@ndycode ndycode closed this Mar 23, 2026
@ndycode ndycode deleted the refactor/pr3-storage-record-guards branch March 24, 2026 18:33
ndycode added a commit that referenced this pull request Apr 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant