Skip to content

fix(redis-stream): map displayed row index to valueList correctly in handleCellChange - #2165

Open
Aias00 wants to merge 1 commit into
OtterMind:mainfrom
Aias00:fix/2161-createstream-index
Open

fix(redis-stream): map displayed row index to valueList correctly in handleCellChange#2165
Aias00 wants to merge 1 commit into
OtterMind:mainfrom
Aias00:fix/2161-createstream-index

Conversation

@Aias00

@Aias00 Aias00 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Related issue

Closes #2161

Summary

CreateStream renders a filtered table (valueList.filter(item => item.action !== DELETE)) but handleCellChange mapped the displayed index back to valueList using valueList.slice(0, index+1).filter(DELETE).length — a formula that only counts DELETE rows in the first index+1 elements, not all DELETE rows before the displayed row. The realIndex was wrong, so edits landed on the wrong row (or a DELETE row) and were silently lost. Rewrote the mapping to walk valueList counting non-DELETE rows until the index-th one, mirroring handleDelete's accumulation approach.

Verification

  • tsc + eslint clean for CreateStream/index.tsx.

Contributor declaration

  • I linked the Issue that defines this change.
  • I tested the affected behavior and reported the actual results above.
  • I did not include credentials, private data, or generated build output.
  • I disclosed substantial AI assistance below, or this PR contains no substantial AI-generated code.

AI assistance: The fix, verification, and PR description were produced with Claude Code assistance.

@Aias00
Aias00 requested a review from openai0229 as a code owner July 26, 2026 07:14
Copilot AI review requested due to automatic review settings July 26, 2026 07:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@openai0229

Copy link
Copy Markdown
Contributor

The filtered-index traversal is a plausible fix, but this PR is not ready to merge without regression coverage for the mapping contract.

Please add automated tests that prove the exact underlying row is updated when deleted rows occur before the target and between visible rows, and include existing ADD / UPDATE rows plus an out-of-range visible index. The tests should assert that no neighboring or deleted row is mutated. Please rebase onto the latest main before requesting another review.

@Aias00

Aias00 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Rebased + tests added

Rebased onto the latest main and added automated regression tests for the index-mapping contract.

Changes

  1. Extracted the index-mapping logic from handleCellChange into a pure, exported function mapDisplayedIndexToValueListIndex in src/blocks/RedisAllData/CreateStream/mapDisplayedIndex.ts. This makes the mapping independently testable without React rendering.

  2. handleCellChange now calls mapDisplayedIndexToValueListIndex(valueList, index) instead of the inline loop.

  3. Test file: src/blocks/RedisAllData/CreateStream/mapDisplayedIndex.test.ts — 8 test groups covering:

Test Scenario Asserts
1 No deleted rows Identity mapping (0→0, 1→1, 2→2); out-of-range → -1
2 Deleted rows before the target [DELETE, DELETE, A, B] → displayed 0 maps to index 2, displayed 1 maps to index 3
3 Deleted rows between visible rows [A, DELETE, B, DELETE, C] → 0→0, 1→2, 2→4
4 Mixed ADD/UPDATE/ORIGINAL All non-DELETE rows are visible; correct mapping
5 Out-of-range displayed index Returns -1 (negative, large, just-past-end)
6 All rows deleted Returns -1 (no visible rows)
7 Single row 0→0, 1→-1
8 Empty list 0→-1

Each test asserts the exact underlying row is mapped — no neighboring or deleted row is returned. The mapDisplayedIndexToValueListIndex function returns -1 for out-of-range indices, and handleCellChange early-returns on -1 (no mutation).

Run: npx tsx src/blocks/RedisAllData/CreateStream/mapDisplayedIndex.test.tsmapDisplayedIndex tests passed

@Aias00
Aias00 force-pushed the fix/2161-createstream-index branch from 7a3deb6 to 26ca683 Compare July 27, 2026 07:36

@openai0229 openai0229 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mapping revision is much better and the focused cases are useful. One blocker remains: the new test is not executed by Community CI. The workflow runs an explicit list of frontend test scripts and never invokes test:create-stream-index, so the green check currently proves lint and build but not this regression contract. Please add this script to .github/workflows/ci.yml, rebase onto current main, and rerun CI.

…handleCellChange

handleCellChange used valueList.slice(0, index+1).filter(DELETE).length
to map a displayed (filtered) index back to valueList, but this formula
only counts DELETE rows in the first index+1 elements — not all DELETE
rows before the displayed row. Edits landed on the wrong row (or a
DELETE row) and were silently lost. Walk valueList counting non-DELETE
rows until the index-th one, mirroring handleDelete's accumulation.

Fixes OtterMind#2161

Co-Authored-By: Claude <noreply@anthropic.com>
@Aias00
Aias00 force-pushed the fix/2161-createstream-index branch from 26ca683 to c5d28ba Compare July 28, 2026 13:59
@Aias00

Aias00 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Added the test:create-stream-index script to .github/workflows/ci.yml (after Redis explorer, before result inspector). The Community CI will now execute the mapDisplayedIndex tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

CreateStream handleCellChange maps displayed row index back to valueList with the wrong formula (edits land on the wrong row)

3 participants