chore: clear pre-existing CI backlogs + first/last util + no-index-zero rule#2538
Open
andrew-bierman wants to merge 5 commits into
Open
chore: clear pre-existing CI backlogs + first/last util + no-index-zero rule#2538andrew-bierman wants to merge 5 commits into
andrew-bierman wants to merge 5 commits into
Conversation
Cleaner + safer than raw arr[0]/arr[len-1] (which are T|undefined under noUncheckedIndexedAccess). first(arr, default?)/last(arr, default?). Manifest 54=54, 100% cov. Basis for the no-index-zero nudge rule. Follow-up branch.
client.call() takes a single object arg ({ promise, action, ... }) — all prod
call sites use it, but the tests still passed positionally, so args.promise was
undefined and every assertion failed on the wrong error. Updated the 18 calls to
the object form; no production changes. 63/63 green. (Pre-existing on development.)
Exempt framework-mandated signatures (Workflow run(event,step) via FRAMEWORK_METHOD_NAMES; the cloudflare-workers test-stub ctor + env-positional sweepInvalidItemLogs via EXCLUDED_FILES, each with rationale). Refactor the two genuinely-ours functions (fetchHeaderRow, ChunkBoundaryError) to object args + update their call sites. Analyzer tests + api tests pass. (Pre-existing on dev.)
…first()/last()/.at() Warning-level ast-grep rule (+ tsx twin + tests) flagging bare `ident[0]`, `ident[1]`, … numeric-literal index access on identifiers (not calls/chains/ computed). Steers to first()/last() (@packrat/utils) or native .at(n) — all T|undefined under noUncheckedIndexedAccess, so style not safety; burn-down, not CI-gating (154 warnings repo-wide). Migrate 4 `const first = arr[0]` sites to first(). Tuple/match-group access stays valid (// allow-index escape).
packages/ui declared ^2.0.6 but the root override forces 2.0.3-2 (the pinned fork), so it always resolved to 2.0.3-2 anyway — the ^2.0.6 was a dishonest declaration tripping check:catalog (VERSION MISMATCH vs apps/expo). Align the declaration; zero resolution change. Clears check:catalog. (expo-doctor's expo-* patch drift is deferred — fixing it surfaces cross-workspace native-module dup that needs proper monorepo dedup via catalog/overrides — dependency-policy work.)
Contributor
|
Warning Review limit reached
More reviews will be available in 40 minutes and 58 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (19)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Contributor
Coverage Report for packages/mcp (./packages/mcp)
File CoverageNo changed files found. |
Contributor
Coverage Report for packages/utils (./packages/utils)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
Contributor
Coverage Report for packages/units (./packages/units)
File CoverageNo changed files found. |
Contributor
Coverage Report for packages/api (./packages/api)
File Coverage
|
||||||||||||||||||||||||||||||||||||||
Contributor
Coverage Report for packages/overpass (./packages/overpass)
File CoverageNo changed files found. |
Contributor
Coverage Report for packages/analytics (./packages/analytics)
File CoverageNo changed files found. |
github-actions
Bot
added
documentation
Contributor
There was a problem hiding this comment.
Pull request overview
Clears several pre-existing CI failures on development while also adding two small enforcement additions: a first/last array-end utility in @packrat/utils and a new (warning-level) ast-grep rule discouraging raw numeric-literal indexing on array identifiers.
Changes:
- Fixes
packages/mcptests by updatingcall()usage to the current object-argument signature. - Extends utils surface/provenance to include
first/last, and migrates a fewarr[0]call sites tofirst(arr). - Adds
no-index-zero/no-index-zero-tsxast-grep rules + tests/snapshots, and resolves acheck:catalogmismatch by aligning@packrat-ai/nativewindui’s declared version.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/lint/no-owned-max-params.ts | Exempts additional framework-mandated positional signatures. |
| packages/utils/src/surface.test.ts | Adds surface tests for array.first / array.last. |
| packages/utils/src/provenance.ts | Records provenance entries for first / last. |
| packages/utils/src/array.ts | Re-exports first / last from radashi. |
| packages/ui/package.json | Aligns @packrat-ai/nativewindui declared version to the pinned override. |
| packages/mcp/src/tests/client.test.ts | Updates call() tests to pass { promise, ...options }. |
| packages/api/src/workflows/shared/chunkCsvForR2.ts | Refactors ChunkBoundaryError to object args and updates usage. |
| packages/api/src/workflows/catalog-etl-workflow.ts | Refactors fetchHeaderRow to object args and updates call site. |
| packages/api/src/routes/weather.ts | Replaces matches[0] with first(matches) in /by-name handler. |
| packages/api/src/routes/admin/index.ts | Replaces updated[0] with first(updated) in catalog update route. |
| apps/expo/features/trips/screens/TripWeatherDetailsScreen.tsx | Replaces locations[0] with first(locations). |
| apps/expo/features/ai/lib/tools.ts | Replaces results[0] with first(results) in weather tool. |
| ast-grep-rules/no-index-zero.yml | Adds warning rule to flag numeric-literal indexing on identifiers (TS). |
| ast-grep-rules/no-index-zero-tsx.yml | TSX twin of no-index-zero rule. |
| ast-grep-rules/PARITY.md | Documents the new rule (but currently contains a behavior mismatch). |
| ast-grep-tests/no-index-zero-test.yml | Adds rule test cases (currently inconsistent with snapshot). |
| ast-grep-tests/no-index-zero-tsx-test.yml | Adds TSX rule test cases. |
| ast-grep-tests/snapshots/no-index-zero-snapshot.yml | Snapshot output for TS rule. |
| ast-grep-tests/snapshots/no-index-zero-tsx-snapshot.yml | Snapshot output for TSX rule. |
| bun.lock | Updates lockfile (includes Expo-related bumps beyond the nativewindui alignment). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+15
to
+20
| invalid: | ||
| # Bare identifier indexed at a numeric literal — the style-nudge target. | ||
| - "const a = arr[0];" | ||
| - "const b = results[0];" | ||
| - "const c = locations[1];" | ||
| - "const d = items[2];" |
Comment on lines
+69
to
+81
| Rule shape: `pattern: $ARR[0]` with `has: {field: object, kind: identifier}`. The | ||
| `kind: identifier` constraint on the `object` field of the `subscript_expression` keeps the | ||
| rule deliberately NARROW — it flags only a bare identifier indexed at `0`: | ||
|
|
||
| | Shape | Flagged? | Why | | ||
| |---|---|---| | ||
| | `arr[0]` | yes | object is an `identifier` | | ||
| | `getThing()[0]` | no | object is a `call_expression` | | ||
| | `a.b.c[0]` | no | object is a `member_expression` | | ||
| | `re.match(s)[0]` | no | object is a `call_expression` | | ||
| | `Object.entries(x)[0]` | no | object is a `call_expression` | | ||
| | `arr[1]` / `arr[2]` / `arr[i]` | no | literal index `0` only | | ||
|
|
Comment on lines
+15
to
+20
| invalid: | ||
| # Bare identifier indexed at a numeric literal — the style-nudge target. | ||
| - "const a = arr[0];" | ||
| - "const b = results[0];" | ||
| - "const c = locations[1];" | ||
| - "const d = items[2];" |
Comment on lines
+69
to
+81
| Rule shape: `pattern: $ARR[0]` with `has: {field: object, kind: identifier}`. The | ||
| `kind: identifier` constraint on the `object` field of the `subscript_expression` keeps the | ||
| rule deliberately NARROW — it flags only a bare identifier indexed at `0`: | ||
|
|
||
| | Shape | Flagged? | Why | | ||
| |---|---|---| | ||
| | `arr[0]` | yes | object is an `identifier` | | ||
| | `getThing()[0]` | no | object is a `call_expression` | | ||
| | `a.b.c[0]` | no | object is a `member_expression` | | ||
| | `re.match(s)[0]` | no | object is a `call_expression` | | ||
| | `Object.entries(x)[0]` | no | object is a `call_expression` | | ||
| | `arr[1]` / `arr[2]` / `arr[i]` | no | literal index `0` only | | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #2535 (re-target to
developmentonce that merges). Clears the pre-existing CI backlogs that were red ondevelopmentindependent of the utils work, plus two small enforcement additions requested during review.Backlogs cleared
client.test.tswas callingcall()with the old positional signature; the prod API takes a single object ({ promise, action, … }). Fixed the 18 calls to the object form — no production changes. 63/63 green.no-owned-max-params(6 → 0): exempted framework-mandated signatures (Workflowrun(event, step), the cloudflare-workers test-stub ctor,sweepInvalidItemLogs's positionalenv) viaFRAMEWORK_METHOD_NAMES/EXCLUDED_FILESwith rationale; refactored the two genuinely-ours functions (fetchHeaderRow,ChunkBoundaryError) to object args. Analyzer + api tests pass.check:catalog(nativewindui):packages/uideclared^2.0.6but the root override forces2.0.3-2(the pinned fork), so it always resolved to2.0.3-2— the declaration was dishonest and tripped the version-mismatch check. Aligned the declaration; zero resolution change.New enforcement (requested during review)
first/lastutil (@packrat/utils, radashi) — safeT | undefinedaccess for array ends. Migrated 4const first = arr[0]sites.no-index-zerorule (ast-grep, warning / burn-down) — nudges raw numeric-literal index access (arr[0],arr[1], …) on identifiers towardfirst()/last()/native.at(n). Style, not safety (alreadyT|undefinedundernoUncheckedIndexedAccess), so non-gating. 154 warnings repo-wide; tuple/match-group access stays valid via// allow-index:.Deferred (not safe to fix blindly here)
expo install --fixaligns the 8 expo-* patch drifts but then surfaces cross-workspace duplicate native modules (expo-file-system/font/image/symbols at two versions). Properly deduping needs catalog/overrides for the expo-* set — a dependency-policy task, risky to do blindly. Reverted the partial bump; flagging for a focused follow-up. Pre-existing ondevelopment.Verify
bun scripts/lint/no-owned-max-params.tsexit 0 ·bun run --cwd packages/mcp test63/0 ·check:catalogclean ·bunx ast-grep testgreen ·bun run check:ast-grepexit 0 · biome clean · provenance 54=54.