docs: complete review-core-and-redis checkpoint

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

@@ -1,7 +1,7 @@
 ---
 id: review-core-and-redis
 name: Review core module tests and Redis adapter
-status: pending
+status: completed
 depends_on: [core-pubsub-tests, core-operators-tests, redis-adapter-tests]
 scope: narrow
 risk: low
@@ -38,8 +38,123 @@ This is a quality gate before implementing new adapters. Mistakes in the core ty
 
 ## Notes
 
-> To be filled by implementation agent
+### 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
 
-> To be filled on completion
+**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.