mirror of
https://github.com/olehomelchenko/astrolabe.git
synced 2026-08-08 02:02:33 +00:00
Skills: fold in four external-skills-review borrows; record the comparison memo
This commit is contained in:
@@ -54,6 +54,16 @@ Review all changes in scope. If changes span multiple patterns below, apply all
|
|||||||
only within their module (including `as const` arrays that exist to derive a type) stay
|
only within their module (including `as const` arrays that exist to derive a type) stay
|
||||||
unexported — `export type` the type, not its source array. Verify with Grep before
|
unexported — `export type` the type, not its source array. Verify with Grep before
|
||||||
exporting "for future use"; the future caller can add the export.
|
exporting "for future use"; the future caller can add the export.
|
||||||
|
- **Smell baseline** (Fowler, _Refactoring_ ch. 3 — judgement calls, never hard
|
||||||
|
violations; a documented project rule overrides, and skip anything eslint/Prettier
|
||||||
|
already enforces): mysterious name (rename — if no honest name comes, the design is
|
||||||
|
murky); data clumps (the same few params traveling together → one type); primitive
|
||||||
|
obsession (a string/number standing in for a domain concept); feature envy (a function
|
||||||
|
reaching into another module's data more than its own); repeated switches (the same
|
||||||
|
discriminant cascade at multiple sites → one shared map); message chains
|
||||||
|
(`a.b().c().d()` → hide the walk behind the first object); middle man (a layer that
|
||||||
|
only delegates → call the target directly). Duplication and speculative generality are
|
||||||
|
covered by the cleanup rules above.
|
||||||
|
|
||||||
5. **Styles and UI**: When altering CSS or layout, follow or generalize existing patterns
|
5. **Styles and UI**: When altering CSS or layout, follow or generalize existing patterns
|
||||||
(CSS Modules + design tokens in `styles/tokens.css`) rather than writing from scratch. Don't
|
(CSS Modules + design tokens in `styles/tokens.css`) rather than writing from scratch. Don't
|
||||||
|
|||||||
@@ -126,7 +126,9 @@ that prevents data loss, or accessibility.
|
|||||||
**If it must be built — what shape?** Line up the existing instances of the kind (modal,
|
**If it must be built — what shape?** Line up the existing instances of the kind (modal,
|
||||||
store, service, hook, persistence path), name the canonical shape, list what to reuse, and
|
store, service, hook, persistence path), name the canonical shape, list what to reuse, and
|
||||||
flag what the new work might make deletable. An abstraction is earned only by ≥ 2 call
|
flag what the new work might make deletable. An abstraction is earned only by ≥ 2 call
|
||||||
sites that would use it today (deletion rule 3).
|
sites that would use it today (deletion rule 3). The deletion test settles suspected
|
||||||
|
pass-throughs: imagine the module deleted — if the complexity just vanishes, it was a
|
||||||
|
shallow wrapper; only if it reappears across its callers was it earning its keep.
|
||||||
|
|
||||||
No report scaffolding — these two answers are the output.
|
No report scaffolding — these two answers are the output.
|
||||||
|
|
||||||
|
|||||||
@@ -123,6 +123,12 @@ npm run format # Prettier
|
|||||||
leads. When in doubt about existing behavior, read the spec. When you ship user-facing
|
leads. When in doubt about existing behavior, read the spec. When you ship user-facing
|
||||||
behavior, update the matching spec section in the same session; if spec and app
|
behavior, update the matching spec section in the same session; if spec and app
|
||||||
disagree, the spec is stale — rewrite it deliberately, never drift silently.
|
disagree, the spec is stale — rewrite it deliberately, never drift silently.
|
||||||
|
- **Reproduce before theorizing** — on a nontrivial bug, first build a command that goes
|
||||||
|
red on the exact symptom (failing test, script, headless-browser driver) and is fast,
|
||||||
|
deterministic, and runnable unattended; minimize the repro, then hypothesize against it.
|
||||||
|
Reading code to build a theory before that command exists is the failure mode. Write the
|
||||||
|
regression test before the fix; tag temporary debug logs with a unique prefix
|
||||||
|
(e.g. `[DEBUG-x7]`) so cleanup is one grep.
|
||||||
- **Core-first** — for each feature, build the pure `src/core/` logic with tests before UI.
|
- **Core-first** — for each feature, build the pure `src/core/` logic with tests before UI.
|
||||||
- **Session wrap-up** — when the user signals the session is wrapping, run the review
|
- **Session wrap-up** — when the user signals the session is wrapping, run the review
|
||||||
pass before any commit: `/doc-update` in-session first (flush unrecorded rationale),
|
pass before any commit: `/doc-update` in-session first (flush unrecorded rationale),
|
||||||
@@ -172,6 +178,11 @@ carries — platform branches, state transitions, config-path writes, render ser
|
|||||||
not the strings it renders; if that logic is worth guarding, lift it into core/stores and
|
not the strings it renders; if that logic is worth guarding, lift it into core/stores and
|
||||||
test it there.
|
test it there.
|
||||||
|
|
||||||
|
Expected values come from an independent source of truth — a known-good literal, a worked
|
||||||
|
example, the spec — never recomputed the way the implementation computes them. A
|
||||||
|
tautological assertion (`expect(add(a, b)).toBe(a + b)`) passes by construction and can
|
||||||
|
never disagree with the code.
|
||||||
|
|
||||||
Component tests (happy-dom) share a harness shape: `createRoot` + `act` with
|
Component tests (happy-dom) share a harness shape: `createRoot` + `act` with
|
||||||
`IS_REACT_ACT_ENVIRONMENT = true` set at module level, stores reset in `beforeEach`, and
|
`IS_REACT_ACT_ENVIRONMENT = true` set at module level, stores reset in `beforeEach`, and
|
||||||
`vi.mock('../services/chart-renderer', …)` for anything that embeds a chart (vega-embed is
|
`vi.mock('../services/chart-renderer', …)` for anything that embeds a chart (vega-embed is
|
||||||
|
|||||||
@@ -0,0 +1,73 @@
|
|||||||
|
# External skills repos review — mattpocock/skills and github/spec-kit
|
||||||
|
|
||||||
|
_Point-in-time record, 2026-07-04. Both repos are shallow-cloned under
|
||||||
|
`/Users/oleh/code/reference/` (`mattpocock-skills/`, `spec-kit/`) for grepping._
|
||||||
|
|
||||||
|
## Question
|
||||||
|
|
||||||
|
Do the general-purpose engineering-robustness skill sets — mattpocock/skills and
|
||||||
|
github/spec-kit — contain anything our project skills (`/alignment`, `/eng-council`,
|
||||||
|
`/council`, `/doc-update`) should adopt? Constraint: fold single additive ideas into
|
||||||
|
existing skills; never install a parallel framework.
|
||||||
|
|
||||||
|
## Verdict
|
||||||
|
|
||||||
|
Neither repo is worth adopting wholesale. Our set covers the same ground with more rigor
|
||||||
|
because it is project-specific and evidence-grounded where theirs is generic. Four
|
||||||
|
discrete ideas were folded in (all from mattpocock/skills or shared with spec-kit);
|
||||||
|
everything else was already covered or rejected.
|
||||||
|
|
||||||
|
## The three approaches
|
||||||
|
|
||||||
|
- **Ours** — a closed-loop system: `/alignment` enforces numbered project-specific checks
|
||||||
|
per diff; `/eng-council` and `/council` review from altitude under a hard evidence
|
||||||
|
requirement (file:line, tool output, cited canon); recurring findings promote into
|
||||||
|
architecture rules and new alignment checks. Built around the code-leads /
|
||||||
|
descriptive-spec regime and a deletion bias (LOC delta per finding).
|
||||||
|
- **mattpocock/skills** — small composable per-task disciplines: grilling (relentless
|
||||||
|
one-question-at-a-time plan interviews), TDD, a bug-diagnosis loop, two-axis code review
|
||||||
|
(repo standards + Fowler smell baseline vs. originating spec), domain modeling
|
||||||
|
(`CONTEXT.md` glossary + ADRs), deep-module design vocabulary (Ousterhout/Feathers).
|
||||||
|
Same anti-framework philosophy as ours; its README positions against spec-kit explicitly.
|
||||||
|
- **github/spec-kit** — a heavyweight spec-first pipeline (constitution → specify →
|
||||||
|
clarify → plan → tasks → analyze → implement → converge), seven-plus artifacts per
|
||||||
|
feature, gates between phases. Its thesis — the spec is the primary artifact, code its
|
||||||
|
expression — is the inverse of our regime, and it assumes greenfield feature branches,
|
||||||
|
team role separation, and business-stakeholder specs. Even `converge`, its only
|
||||||
|
code-vs-artifacts mode, treats undocumented code behavior as scope creep to justify or
|
||||||
|
remove, where our regime says the code is right and the spec gets rewritten.
|
||||||
|
|
||||||
|
## Folded in (2026-07-04)
|
||||||
|
|
||||||
|
| Idea | Source | Landed in |
|
||||||
|
| --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------- | --------------------------------- |
|
||||||
|
| Reproduce-before-theorizing debugging discipline: red-capable repro command before any hypothesis; minimize; regression test before fix; prefix-tagged debug logs | `diagnosing-bugs` | AGENTS.md → AI Developer Protocol |
|
||||||
|
| Fowler smell baseline as judgement-call heuristics (mysterious name, data clumps, primitive obsession, feature envy, repeated switches, message chains, middle man) | `code-review` | `/alignment` rule 4 |
|
||||||
|
| Tautological-test rule: expected values from an independent source of truth, never recomputed the implementation's way | `tdd` | AGENTS.md → Testing Philosophy |
|
||||||
|
| The deletion test for suspected pass-throughs: delete the module mentally — complexity vanishing means shallow wrapper, reappearing across callers means it earned its keep | `codebase-design` | `/eng-council` consult mode |
|
||||||
|
|
||||||
|
## Considered and rejected
|
||||||
|
|
||||||
|
- **Grilling as a skill** — sessions here are already interactively driven, and the
|
||||||
|
harness's question tool plus explore-instead-of-ask covers the discipline. No standing
|
||||||
|
gap.
|
||||||
|
- **`CONTEXT.md` glossary + ADRs (domain modeling)** — `docs/architecture/` +
|
||||||
|
`/doc-update` fill the same role with a stricter altitude bar; a second
|
||||||
|
decision-record home would split the record.
|
||||||
|
- **spec-kit's constitution** — SOUL.md + the architecture playbook already are the
|
||||||
|
constitution, and ours is enforced mechanically (alignment checks), not re-read per
|
||||||
|
phase.
|
||||||
|
- **spec-kit's "unit tests for English" checklists** (items test requirement quality:
|
||||||
|
completeness/clarity/measurability, banned Verify/Test verbs) — the standout idea of
|
||||||
|
the repo, but it targets prescriptive specs. Our spec is descriptive; its quality bar
|
||||||
|
is "matches the code", which alignment's spec-tracking check already enforces.
|
||||||
|
- **spec-kit's bidirectional coverage / gap-type taxonomy** (`missing`/`partial`/
|
||||||
|
`contradicts`/`unrequested`) — both directions of spec↔code drift are already covered
|
||||||
|
by alignment's spec-tracking check and the eng-council Documentation seat.
|
||||||
|
- **spec-kit's clarify mechanics** (fixed ambiguity taxonomy, Impact×Uncertainty question
|
||||||
|
cap, recommend-before-asking) — recommend-before-asking is already harness convention;
|
||||||
|
the rest is ceremony sized for teams, not a solo interactive loop.
|
||||||
|
- **`improve-codebase-architecture` / HTML report** — `/eng-council` sweep covers it with
|
||||||
|
real evidence tooling (madge/knip/jscpd) and the metrics trend.
|
||||||
|
- **`research`, `prototype`, `handoff`** — already covered by reference clones, the
|
||||||
|
one-off HTML showcase habit, and harness context management respectively.
|
||||||
Reference in New Issue
Block a user