From ebd336efe6e36321e2574fe127cd5eb485d6f3a7 Mon Sep 17 00:00:00 2001 From: "glm-5.1" Date: Fri, 8 May 2026 06:51:48 +0000 Subject: [PATCH] docs: complete review-core-and-redis checkpoint --- tasks/005-review-core-and-redis.md | 121 ++++++++++++++++++++++++++++- 1 file changed, 118 insertions(+), 3 deletions(-) diff --git a/tasks/005-review-core-and-redis.md b/tasks/005-review-core-and-redis.md index 0a43e23..e6f0355 100644 --- a/tasks/005-review-core-and-redis.md +++ b/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 \ No newline at end of file +**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. \ No newline at end of file