feat(#72): SPI v1 slice 1c-vi.a — Redux Toolkit substrate#114
Merged
Conversation
Adds Redux Toolkit substrate detection. Five new SpiFrameworkRole variants under the redux_ prefix:
* redux_slice — createSlice({...}) factory result
* redux_store — configureStore({...}) or legacy createStore() factory result
* redux_selector — createSelector(...) or createDraftSafeSelector(...) factory result
* redux_async_thunk — createAsyncThunk(...) factory result
* redux_rtk_query_api — createApi(...) factory result
Detector (src/pipeline/spi/framework-redux.ts):
* Imports are gated to the canonical module specifiers: @reduxjs/toolkit, @reduxjs/toolkit/query, @reduxjs/toolkit/query/react, redux (for legacy createStore), and reselect (createSelector is routinely re-exported from RTK via reselect).
* Walks top-level VariableStatements. For each initialiser that is a CallExpression whose callee identifier matches an imported factory name (including aliased imports like createSlice as makeSlice), the variable's symbol is tagged with the matching framework_role.
* Property-access factory variants (api.injectEndpoints) are intentionally NOT tagged in this slice — they're derivations off already-tagged factory results, not new factory calls. A future slice may extend.
Projector wiring (src/pipeline/spi/projector.ts):
* frameworkForRole maps redux_* prefix → 'redux'.
* nodeKindForRole: redux_slice → 'slice', redux_store → 'store', other redux_* → 'function' (the existing ExtractionNode node_kind taxonomy already supports 'slice' and 'store' from the legacy redux extractor).
Tests: 13 new in spi-framework-redux.test.ts. Cover every factory function (createSlice, configureStore, legacy createStore, createSelector, createDraftSafeSelector, createAsyncThunk, createApi), reselect package support, aliased imports, negative cases (local same-named function not tagged, unrelated initialisers not tagged, property-access derivations not tagged), end-to-end projection through to ExtractionNode shape (framework=redux, correct node_kind).
Out of scope (deferred):
* Slice metadata extraction: slice name, reducer keys, auto-generated action creators. Slice 1c-vi.b candidate.
* createReducer (functional reducer composition) — non-essential, add when a real codebase asks for it.
* useSelector / useDispatch hook detection — structural noise; consumers can locate selectors via framework_role instead.
📝 WalkthroughWalkthroughThis PR adds Redux Toolkit framework detection to the SPI pipeline. It extends the type system with five Redux-specific roles, implements a new detection module that identifies Redux factory API calls in TypeScript source files, integrates detection into the per-file type-checker pass, updates the projection layer to map Redux roles to extraction nodes, and validates the feature with comprehensive unit tests. ChangesRedux Framework Detection Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 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. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/pipeline/spi/framework-redux.ts`:
- Around line 89-100: collectBindings() currently treats type-only imports as
runtime factories; add guards to skip them by checking importClause.isTypeOnly
before proceeding with a Redux module import and checking element.isTypeOnly
inside the named.elements loop to avoid adding type-only specifiers to
bindings.factories; specifically, when you detect an import declaration for
REDUX_MODULE_SPECIFIERS, return early if stmt.importClause.isTypeOnly, and
inside the loop skip any element where element.isTypeOnly before mapping
element.name.text via FACTORY_TO_ROLE into bindings.factories.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f14ba2ea-dfed-4555-ab3f-07eb73765bd1
📒 Files selected for processing (6)
src/pipeline/spi/build.tssrc/pipeline/spi/framework-redux.tssrc/pipeline/spi/index.tssrc/pipeline/spi/projector.tssrc/pipeline/spi/types.tstests/unit/spi-framework-redux.test.ts
Comment on lines
+89
to
+100
| if (!ts.isImportDeclaration(stmt) || !stmt.importClause) continue | ||
| if (!ts.isStringLiteral(stmt.moduleSpecifier)) continue | ||
| if (!REDUX_MODULE_SPECIFIERS.has(stmt.moduleSpecifier.text)) continue | ||
|
|
||
| bindings.hasReduxImport = true | ||
| const named = stmt.importClause.namedBindings | ||
| if (!named || !ts.isNamedImports(named)) continue | ||
| for (const element of named.elements) { | ||
| const importedName = element.propertyName?.text ?? element.name.text | ||
| const role = FACTORY_TO_ROLE.get(importedName) | ||
| if (role) bindings.factories.set(element.name.text, role) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Confirm whether type-only guards exist in the detector.
rg -n "importClause\\.isTypeOnly|element\\.isTypeOnly" src/pipeline/spi/framework-redux.ts
# 2) Check whether tests already cover `import type` for Redux factories.
rg -n "import type|type createSlice|type configureStore|type createSelector" tests/unit/spi-framework-redux.test.tsRepository: mohanagy/graphify-ts
Length of output: 46
🏁 Script executed:
# Check if the file exists and get repository structure
git ls-files | grep -E "(framework-redux|spi)" | head -20Repository: mohanagy/graphify-ts
Length of output: 761
🏁 Script executed:
# Verify the file path and read the collectBindings function
if [ -f "src/pipeline/spi/framework-redux.ts" ]; then
echo "=== File exists ==="
wc -l src/pipeline/spi/framework-redux.ts
echo "=== Lines 80-110 (context around lines 89-100) ==="
sed -n '80,110p' src/pipeline/spi/framework-redux.ts
else
echo "File not found at src/pipeline/spi/framework-redux.ts"
find . -name "*framework-redux*" -type f 2>/dev/null || echo "No matching files found"
fiRepository: mohanagy/graphify-ts
Length of output: 1246
🏁 Script executed:
# Check test file for type-only import coverage
if [ -f "tests/unit/spi-framework-redux.test.ts" ]; then
echo "=== Test file exists ==="
wc -l tests/unit/spi-framework-redux.test.ts
echo "=== Searching for type import tests ==="
rg -n "import type|type.*createSlice|type.*configureStore" tests/unit/spi-framework-redux.test.ts --max-count=20
else
echo "Test file check:"
find . -name "*spi*framework*redux*test*" -type f 2>/dev/null || echo "No matching test files found"
fiRepository: mohanagy/graphify-ts
Length of output: 261
Add type-only import guards in Redux factory detection.
The collectBindings() function lacks guards for type-only imports, risking false positives when import type { createSlice } coexists with a local const createSlice = .... Add checks on both importClause.isTypeOnly (line 89) and element.isTypeOnly (within the loop at line 96) to exclude type-only specifiers from the factory bindings map.
🔧 Proposed fix
function collectBindings(sourceFile: ts.SourceFile): ReduxBindings {
const bindings: ReduxBindings = {
factories: new Map<string, SpiFrameworkRole>(),
hasReduxImport: false,
}
for (const stmt of sourceFile.statements) {
if (!ts.isImportDeclaration(stmt) || !stmt.importClause) continue
+ if (stmt.importClause.isTypeOnly) continue
if (!ts.isStringLiteral(stmt.moduleSpecifier)) continue
if (!REDUX_MODULE_SPECIFIERS.has(stmt.moduleSpecifier.text)) continue
bindings.hasReduxImport = true
const named = stmt.importClause.namedBindings
if (!named || !ts.isNamedImports(named)) continue
for (const element of named.elements) {
+ if (element.isTypeOnly) continue
const importedName = element.propertyName?.text ?? element.name.text
const role = FACTORY_TO_ROLE.get(importedName)
if (role) bindings.factories.set(element.name.text, role)
}
}
return bindings
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (!ts.isImportDeclaration(stmt) || !stmt.importClause) continue | |
| if (!ts.isStringLiteral(stmt.moduleSpecifier)) continue | |
| if (!REDUX_MODULE_SPECIFIERS.has(stmt.moduleSpecifier.text)) continue | |
| bindings.hasReduxImport = true | |
| const named = stmt.importClause.namedBindings | |
| if (!named || !ts.isNamedImports(named)) continue | |
| for (const element of named.elements) { | |
| const importedName = element.propertyName?.text ?? element.name.text | |
| const role = FACTORY_TO_ROLE.get(importedName) | |
| if (role) bindings.factories.set(element.name.text, role) | |
| } | |
| function collectBindings(sourceFile: ts.SourceFile): ReduxBindings { | |
| const bindings: ReduxBindings = { | |
| factories: new Map<string, SpiFrameworkRole>(), | |
| hasReduxImport: false, | |
| } | |
| for (const stmt of sourceFile.statements) { | |
| if (!ts.isImportDeclaration(stmt) || !stmt.importClause) continue | |
| if (stmt.importClause.isTypeOnly) continue | |
| if (!ts.isStringLiteral(stmt.moduleSpecifier)) continue | |
| if (!REDUX_MODULE_SPECIFIERS.has(stmt.moduleSpecifier.text)) continue | |
| bindings.hasReduxImport = true | |
| const named = stmt.importClause.namedBindings | |
| if (!named || !ts.isNamedImports(named)) continue | |
| for (const element of named.elements) { | |
| if (element.isTypeOnly) continue | |
| const importedName = element.propertyName?.text ?? element.name.text | |
| const role = FACTORY_TO_ROLE.get(importedName) | |
| if (role) bindings.factories.set(element.name.text, role) | |
| } | |
| } | |
| return bindings | |
| } |
🤖 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 `@src/pipeline/spi/framework-redux.ts` around lines 89 - 100, collectBindings()
currently treats type-only imports as runtime factories; add guards to skip them
by checking importClause.isTypeOnly before proceeding with a Redux module import
and checking element.isTypeOnly inside the named.elements loop to avoid adding
type-only specifiers to bindings.factories; specifically, when you detect an
import declaration for REDUX_MODULE_SPECIFIERS, return early if
stmt.importClause.isTypeOnly, and inside the loop skip any element where
element.isTypeOnly before mapping element.name.text via FACTORY_TO_ROLE into
bindings.factories.
5 tasks
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
1 participant
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.
Fifth and final framework substrate. Completes the substrate-level coverage for the framework set the user's roadmap calls out (Express, Next.js, React Router, Redux, plus the pre-existing NestJS).
Patterns detected
Import-gated to: `@reduxjs/toolkit`, `@reduxjs/toolkit/query`, `@reduxjs/toolkit/query/react`, `redux`, `reselect` (selectors are routinely re-exported from RTK via reselect).
Aliased imports work — `import { createSlice as makeSlice } from '@reduxjs/toolkit'` then `makeSlice({...})` tags correctly.
Projector wiring
The `slice` and `store` `node_kind` values already exist in the legacy extractor's taxonomy.
Out of scope (deferred)
Test plan
Framework substrate progress
After this slice merges:
Refs #72.
Summary by CodeRabbit
Release Notes