pubsub/tasks/005-review-core-and-redis.md

---
id: review-core-and-redis
name: Review core module tests and Redis adapter
status: completed
depends_on: [core-pubsub-tests, core-operators-tests, redis-adapter-tests]
scope: narrow
risk: low
impact: phase
level: review
---

## Description

Review checkpoint before moving to new adapter implementations. Verify that:
- Core tests cover the `createPubSub` contract thoroughly (publish, subscribe, topic scoping, `__` rejection, cleanup)
- Operator tests cover all 13 operators with edge cases
- Redis adapter tests pass reliably with real Redis or mock
- Code follows architecture conventions (no unnecessary comments, MIT headers on forked files)
- Build passes (`npm run build`)
- Type-check passes (`npm run lint / tsc --noEmit`)
- Test suite passes (`npm test`)

This is a quality gate before implementing new adapters. Mistakes in the core types or contract will cascade to every adapter.

## Acceptance Criteria

- [ ] `npm run build` passes cleanly
- [ ] `npm run lint` passes (tsc --noEmit)
- [ ] `npm test` passes with all core + Redis tests
- [ ] No regressions in existing functionality
- [ ] Core tests align with architecture spec (api-surface.md)
- [ ] Code follows project conventions (no comments in source, license headers on forked files)

## References

- docs/architecture/api-surface.md
- docs/architecture/event-targets/redis.md

## Notes

### Build / Lint / Test

- **Build**: `npm run build` passes cleanly. ESM + CJS + declarations all produced.
- **Lint**: `npm run lint` (tsc --noEmit) passes with no errors.
- **Tests**: 99 tests pass across 4 files (core: 11, operators: 53, redis: 25, integration: 10).

### Core Tests (`test/create_pubsub.test.ts` — 11 tests)

Aligned with `api-surface.md`. Coverage:

| Contract area | Covered? | Details |
|---|---|---|
| `publish(type, id, payload)` dispatches with `type:id` topic | Yes | "dispatches event with correct type:id topic" |
| EventEnvelope `{ type, id, payload }` in CustomEvent detail | Yes | "dispatches EventEnvelope with type, id, and payload as CustomEvent detail" |
| `__`-prefixed event types rejected | Yes | "throws on __-prefixed event types" with exact error message |
| `subscribe(type, id)` returns async iterable | Yes | "returns async iterable that yields EventEnvelope objects" |
| Topic scoping — only matching `type:id` received | Yes | "receives events only for the subscribed topic" |
| Multiple subscribers on same topic | Yes | "multiple subscribers on the same topic all receive events" |
| Cleanup — listener removed on break | Yes | "cleanup: breaking out of for await loop removes the listener" |
| Custom eventTarget config | Yes | 3 tests: spy on dispatch, in-process default, subscribe via custom target |

All acceptance criteria from `api-surface.md` are tested. No gaps found.

### Operator Tests (`test/operators.test.ts` — 53 tests for 13 operators)

| Operator | Tests | Edge cases |
|---|---|---|
| `filter` | 5 | Sync/async predicate, type-narrowing overload, all-filtered, all-pass |
| `map` | 3 | Sync/async mapper, type transformation |
| `pipe` | 6 | 0–4 functions, composition with subscribe+filter+map |
| `take` | 4 | Normal, excess, 0, single |
| `reduce` | 4 | Normal, empty source, async reducer, string concat |
| `toArray` | 3 | Normal, empty, preserves order |
| `batch` | 5 | Exact, remainder, single-item, size=1, empty |
| `dedupe` | 4 | Mixed, no dups, empty, all same |
| `window` | 5 | Default step, custom step, too short, exact size, empty |
| `flat` | 3 | Normal, empty inner arrays, empty source |
| `groupBy` | 3 | String key, empty, numeric key |
| `chain` | 4 | Multiple, single, with empties, all empty |
| `join` | 4 | Matching keys, no match, empty, duplicate keys (last wins) |

All 13 operators tested with edge cases. Comprehensive coverage.

### Redis Adapter Tests (`test/event-target-redis.test.ts` — 25 tests)

Covers all acceptance criteria from `docs/architecture/event-targets/redis.md`:

| Criterion | Covered? | Tests |
|---|---|---|
| Publish path — dispatchEvent sends to Redis | Yes | Correct channel, JSON serialization, returns true |
| Subscribe path — addEventListener dispatches to local listeners | Yes | Subscribes to Redis, deserializes messages, ignores wrong channels |
| Unsubscribe — removes callback; unsubscribes when empty | Yes | Last listener unsubscribes, partial remove keeps subscription |
| Topic scoping — type:id strings as channels | Yes | "uses type:id strings as Redis channel names" |
| Envelope serialization — full { type, id, payload } round-trip | Yes | JSON round-trip, null payload |
| Multiple listeners on same topic | Yes | Single subscribe, delivers to all listeners |
| Error propagation / handling | Yes | Parse error logs warning + continues, invalid JSON skipped |
| Channel prefix | Yes | Prefix on publish, subscribe, unsubscribe, delivery, ignores non-prefixed |
| Custom serializer | Yes | Custom stringify/parse called, non-JSON serializer round-trips |
| EventListenerObject support | Yes | addEventListener + removeEventListener with handleEvent |

The `redis.md` spec noted "No tests yet. Need: ..." — all 7 required test areas now covered, plus prefix, custom serializer, EventListenerObject, and error handling.

### Integration Tests (`test/integration-pubsub-redis.test.ts` — 10 tests)

End-to-end `createPubSub` + `createRedisEventTarget` with linked mock (publish triggers message on subscribe client):

- Publish through Redis, subscriber receives
- Correct channel name (type:id)
- Subscribe triggers Redis SUBSCRIBE
- Topic scoping (filtered events)
- Multiple subscribers broadcast
- Full envelope round-trip (complex + simple payloads)
- Scoped topic isolation (no cross-topic leakage)
- Channel prefix integration with pubsub

### Code Conventions

| File | License header | Unnecessary comments | Stray debug code |
|---|---|---|---|
| `types.ts` | MIT (graphql-yoga) ✅ | None ✅ | None ✅ |
| `create_pubsub.ts` | MIT (graphql-yoga) ✅ | None ✅ | None ✅ |
| `operators.ts` | MIT (graphql-yoga) short form ✅ | None ✅ | None ✅ |
| `repeater.ts` | MIT (repeaterjs) ✅ | None ✅ | None ✅ |
| `event-target-redis.ts` | MIT (graphql-yoga) + changelog ✅ | None ✅ | None ✅ |
| `index.ts` | No header (barrel, original code) ✅ | None ✅ | None ✅ |

All forked files have MIT attribution headers. No unnecessary comments in source. No stray debug code.

### Architecture Alignment

- Source layout matches `AGENTS.md` spec: `types.ts`, `create_pubsub.ts`, `operators.ts`, `repeater.ts`, `event-target-redis.ts`, `index.ts` barrel.
- Sub-path exports: `@alkdev/pubsub/event-target-redis` configured in `package.json` exports map and `tsup.config.ts` entry array.
- Peer dep isolation: `ioredis` is optional peer dep with `peerDependenciesMeta.optional: true`.
- Barrel `index.ts` re-exports core API + operators + Redis adapter.
- `EventEnvelope` format `{ type, id, payload }` matches spec (read-only fields).
- `createPubSub` rejects `__`-prefixed types per ADR-003.
- `TypedEventTarget` contract: `addEventListener`/`dispatchEvent`/`removeEventListener` implemented by Redis adapter.
- `prefix` feature implemented per `redis.md` channel naming section.

### Deficiencies / Observations

1. **redis.md status**: The doc still says "Needs tests" and status "draft" — could be updated to reflect tests now exist.
2. **redis.md limitations**: The listed limitations (no error handling, no channel prefix) are partially addressed — prefix is now implemented and error handling for parse errors exists, but connection failure/reconnection handling is still absent. The limitations section could be updated.
3. **Test count**: Task description says 81 tests, actual count is now 99 (8 new Redis tests for prefix/error handling + 10 integration tests). No issue, just noting the delta.

## Summary

**Review PASSED.** All acceptance criteria met:

- ✅ `npm run build` passes cleanly
- ✅ `npm run lint` passes (tsc --noEmit)
- ✅ 99 tests pass (core: 11, operators: 53, redis: 25, integration: 10)
- ✅ No regressions
- ✅ Core tests align with api-surface.md (publish, subscribe, topic scoping, `__` rejection, cleanup, custom eventTarget)
- ✅ All 13 operators tested with edge cases
- ✅ Redis adapter tests cover all 7 spec requirements plus prefix, custom serializer, error handling, EventListenerObject
- ✅ Code follows project conventions (no unnecessary comments, MIT headers on forked files, no debug code)
- ✅ Architecture alignment verified (source layout, exports, peer dep isolation, EventEnvelope contract)

Minor follow-up: consider updating `redis.md` status from "draft" to "stable" and updating its "No tests yet" and limitations sections to reflect current state.