lilleman
576b713af4
Test / lint (pull_request) Successful in 23s
Test / test (18) (pull_request) Successful in 30s
Test / test (20) (pull_request) Successful in 31s
Test / test (22) (pull_request) Successful in 31s
Test / test (24) (pull_request) Successful in 30s
Test / test (26) (pull_request) Successful in 30s
Mirror / push (push) Successful in 5s
60 lines
5.3 KiB
Markdown
60 lines
5.3 KiB
Markdown
# Lessons from three redesign rounds
|
||
|
||
A four-seat comprehension panel reads the whole project: a junior, a mid, a maintainability senior and an inherited-system architect. It scores on an absolute 1–10 scale, where 7 = "Predictable: the layout answers where things live; the hard parts are hard because the problem is hard, few, localized and marked". There are four dimensions: Navigation, Locality, Shape and Self-sufficiency. The overall may not exceed the lowest dimension plus one. The target is a mean overall at least one full point above main.
|
||
|
||
| | Overall per seat | Mean | Locality |
|
||
| --- | --- | --- | --- |
|
||
| Main today | 6, 6, 7, 6 | 6.25 | 5, 5, 6, 6 |
|
||
| A: internals only; a `Link` object per socket | 6, 6, 6, 6 | 6.0 | 5, 5, 6, 6 |
|
||
| B: internals only; the lifecycle as one state machine (reducer returns effects, `Session` runs them) | 6, 6, 6, 6 | 6.0 | 6, 6, 5, 6 |
|
||
| C: contract change; `onSms` handler option, every message answered `ESME_ROK` on arrival, `sendResp()` removed, `src/` grouped into session/, messages/, wire/, defs/ | 6, 6, 6, 6 | 6.0 | 6, 6, 6, 6 |
|
||
| D: contract change; `onSms` handler, message held while its promise runs, `sendResp()` kept, no handler means refuse with the retry status | 5, 6, 7, 6 | 6.0 | 5, 6, 6, 6 |
|
||
|
||
Their full diffs are `drafts/draft-a.patch` to `drafts/draft-d.patch`; each carries the draft's own DESIGN.md.
|
||
|
||
What the panels taught:
|
||
1. **Restructuring internals under the old contract does not move the scores (A, B).** The held-message timing contract capped every seat: six exits, a `setImmediate` turn, listener counts, `captureRejections` routed through a `WeakMap`.
|
||
2. **Changing the receiving contract to an `onSms` handler removed that cap (C, D).** In C no reader named the held-message flow; D's junior still did, because "answered" lived in three places (a closure flag, a store field, and `answeredOnArrival`).
|
||
3. **The new ceiling is the session lifecycle.** Seven of eight round-two seats named the same unit they would least want to modify:
|
||
- `LinkLife`: a 4-value phase plus a separate `stopped` flag, whose initial `'up'` is an exception to its own rule.
|
||
- Seven predicates over it (`isUp`, `isAttached`, `isOver`, `isStopped`, `retrying`, `awaitsNextLink`, `refusal`), read by `Session`, `OutgoingRequests` and `IncomingRequests`.
|
||
- `Session.linkLost`/`end`/`dropSocket`/`comeBackUp`, whose correctness hangs on call order.
|
||
- Listeners of `disconnected`/`close` re-entering `close()` synchronously.
|
||
- `ReconnectLoop`'s own stopped flag duplicating `LinkLife`'s.
|
||
- client.ts's `bindOn` relying on `close()` reaching `stop()` before its first await, stated in another file.
|
||
|
||
B tried a single state machine, but under the old contract, where the held-message cap hid any gain.
|
||
4. **Also still cited:**
|
||
- `IncomingRequests`/`HeldMessages` call back into `Session` (`emit`, `sendReturn`, `close`, `listenerCount`).
|
||
- `OutgoingRequests`' several entry points, or lanes, and its retry loop, which depends on link state at each await.
|
||
- `ExpiringGroups` leaves enforcement to its three owners.
|
||
- The GSM 03.38 codec is still named `ascii` somewhere.
|
||
- `DlrMerger.close` really means "spend".
|
||
- There is no glossary for the SMPP terms (ESME, SMSC/MC, esm_class, data_coding, UDH, sar_*, TLV).
|
||
- SMPP section citations with no summary.
|
||
- Defaults are spread over several files.
|
||
5. **Goal checks the drafts raised:**
|
||
- C answers `ESME_ROK` before the application has taken the message, so a crash loses it. That is a goal 2 risk: "work the peer has no reason to send again is not dropped".
|
||
- D's "no handler, so refuse every inbound message with the retry status" is a judgement call. If you keep something like it, record it in docs/decisions.md with the goal it rests on.
|
||
|
||
## Round three
|
||
|
||
| | Overall per seat | Mean | Locality |
|
||
| --- | --- | --- | --- |
|
||
| E: an `onSms` handler whose message is answered when the handler returns, plus the lifecycle as one state machine (`connected, bound, closing, down, ended`) | 5, 7, 6, 6 | 6.0 | 5, 6, 6, 6 |
|
||
| F: a Session is one socket's life, bound once and ended once; reconnect is an `SmppClient` composed above it; `onSms` answered on return | 6, 6, 7, 6 | 6.25 | 5, 5, 6, 6 |
|
||
|
||
Diffs: `drafts/draft-e.patch` and `drafts/draft-f.patch`.
|
||
|
||
What round three taught:
|
||
- **The hardest unit moves every round.**
|
||
- Round 1: the held-message flow.
|
||
- Round 2: the handler contract removed it, and the lifecycle took its place.
|
||
- Round 3: F's one-socket session removed the lifecycle, and readers now name other units:
|
||
- `ExpiringGroups` with `Reassembler.trim`, named by 4 of 8 seats: `set()` or `weigh()` may evict the caller's own entry, reads mutate and fire callbacks, and three owners depend on drop order;
|
||
- the invariant "every inbound PDU gets exactly one answer", enforced jointly by `IncomingRequests` and `sms.ts`.
|
||
- E's state machine was still hard, because each transition's meaning is split across five callbacks in another file.
|
||
- **Juniors score Self-sufficiency 5** even with a README glossary. The cost is SMPP knowledge: spec section numbers with no summary, and the `data_coding` bit masks. That caps a junior's overall at 6.
|
||
- **A fix sometimes adds a smaller hard spot of its own**, as D's "answered" in three places and E's callbacks show.
|
||
- **The scale is coarse**: four integer seats, so one seat moving one point shifts the mean by 0.25.
|