-
Notifications
You must be signed in to change notification settings - Fork 0
Review 5704
PR: #5704 · author cixzhang
Posted review: https://github.com/facebook/astryx/pull/5704#pullrequestreview-5061900063
#5704 docs: define knowledge-contract architecture by cixzhang (bucket: the maintainer)
c39368fb3968284a8686500d15edd5882c35ce02
LOOP VERSION: 1.9.1 AUDIT RUBRIC: 1.15
LANE: full WHY: the PR records a current repository architecture. The diff is prose-only, but it establishes durable ownership and decision rules, so it is not eligible for the fast lane.
WHY 1: Reviewers otherwise have to reconstruct which decisions are current from old pull requests and documents. WHY 2: That makes them re-ask settled questions or invent precedence when current records disagree. WHY 3: Repeated or invented decisions make reviews slower and let repository policy drift without an owner.
USER-FACING PROBLEM: A future Astryx maintainer reviewing a change can apply the wrong rule or interrupt an owner because the repository does not identify one approved, reusable knowledge contract.
PROBLEM SEVERITY: harmful friction — work can continue, but reviews repeat decisions and can route conflicts inconsistently.
NEW FEATURE CASE: owner-approved repository architecture; the author marked the record current, named herself as approver, and posted an exact-head /approve-spec command before this maintenance rebase.
EARLY STOP: clear — the need and owner decision are explicit.
VERDICT: clear
The repository gets one current architecture record that tells reviewers where knowledge lives, which records may be trusted, and when a conflict must stop for its canonical owner. It points to the bootstrap checks that validate structure and bind approval to a commit. This PR owns only the approved architecture record; the bootstrap remains in its existing parent PR.
SOLUTION (1 decision · 174 prose lines · 0 runtime lines)
- Record the already-approved knowledge-contract architecture and its conflict rule in one current architecture document.
BURDEN: low in this PR — one immutable review record, no runtime state, Effect, listener, dependency, or workflow change. BURDEN MATCH: proportionate — a repository-wide decision needs one durable source; this PR adds exactly one.
VERDICT: clear
OWNER: architecture:knowledge-contracts, approved by the named repository owner.
TIER 1: repository knowledge records and the validator/gate supplied by the parent bootstrap.
TIER 2: none.
SEAMS: current vs draft authority; component/family/design/system records; canonical-owner conflict resolution; exact-head approval.
BEHAVIOR UNIT: prose contract — no runtime behavior in this PR.
COMPLEXITY BUDGET: one architecture owner, one current writable record, zero new runtime modules/workflows, and one focused validator path.
ACTUAL BURDEN: 1 changed file; 174 prose lines; 1 owner; 1 current record; 0 schemas/maps/workflows added by this PR; 50 focused parent-bootstrap tests cover validation and approval wiring.
BURDEN TREND: e92c2364 → c39368fb: shrank by the stale two-line claim plus its blank line; the architecture and decision content are otherwise unchanged.
RESET TRIGGER: not triggered — no second owner, policy representation, or synchronization guard was added.
| domain fact | one authoritative writable source | generated / immutable projections | other writable copies |
|---|---|---|---|
| knowledge-record authority and conflict handling | docs/architecture/knowledge-contracts.md |
templates/schema/checks referenced from the parent bootstrap | none introduced here |
The record lives with its named owner and links outward instead of copying other records. It is the smallest durable representation of the approved decision.
VERDICT: clear
Future maintainers can distinguish reusable decisions from new human judgment and stop on genuine current-record conflicts instead of choosing an implicit precedence. No shipped component user, package consumer, or rendered surface changes.
VERDICT: clear
No package or component API changes. The repository-governance concept is already owner-approved; this maintenance pass does not add or choose policy.
OSSIFICATION: the current record is intentionally durable. Its one owner and explicit decision reference make future changes visible instead of silently changing meaning.
VERDICT: clear
Not applicable — the only PR file is Markdown under docs/architecture/; no styles, theme target, token, rendered class, or package source changes.
VERDICT: clear
Behavior: no product runtime behavior changes. API: no package call site changes. Visual: no rendered component or docsite output changes. Theme: no target, token, or override changes.
VERDICT: clear
Effects: zero. Render, listeners/observers, layout, bundle, dependencies: no reachable change; the PR contains one Markdown record.
VERDICT: clear
VISUAL CHECK: not applicable WHY: the complete PR delta is one non-rendered architecture Markdown file; no component, story, docsite page, DOM, style, or visual asset changes.
VERDICT: clear
REMEDY SEARCH: not triggered — no proven visual defect
No user- or AT-facing runtime string, role, ARIA attribute, focus path, direction rule, or locale behavior changes. No local a11y audit was run, as required.
VERDICT: clear
| slot | verdict |
|---|---|
| PROBLEM | clear |
| SOLUTION | clear |
| ARCHITECTURE | clear |
| IMPACT | clear |
| API | clear |
| THEMING | clear |
| BREAKING | clear |
| PERFORMANCE | clear |
| VISUAL | clear |
| A11Y & I18N | clear |
GOAL: met — node scripts/check-knowledge.mjs accepts the current record, 50 focused tests pass, and the record no longer contradicts authority: current by calling itself a draft example.
DISPOSITION: the stale draft-example sentence was a correctness defect with a single answer and was removed before this gate; no unresolved findings remain.
ADVICE: omitted — maintenance preserves the approved architecture.
AUTHOR CAN PROCEED: yes — keep the PR draft until the stack owner chooses to undraft it; no implementation or design decision remains from this maintenance pass.
WORST OUTCOME: none found.
JUDGEMENT NEEDED: none — the owner approved the architecture before this rebase, and the only content correction removes a direct contradiction without changing policy.
ENGINEERING VERDICT: approve. MECHANICAL POSTED VERDICT: comment — the PR is draft and authored by the reviewing account.
Rebase review is clean. This is an approve once the stack is ready to undraft; the only content fix removes a stale sentence that called the current contract a draft.
[Reviewed by Robohands]
None.
- Rebase used rerere disabled. The architecture commit range-diff was identical before the stale sentence was removed.
- A merge-tree preview against current
origin/mainwas clean; the four newer main commits touch unrelated product code. - There were no prior human reviews or inline comments to reconcile. The prior owner command was tied to the pre-rebase head and must be renewed after push.
-
node scripts/check-knowledge.mjs— passed. - Focused Vitest: 4 files, 50 tests — passed.
-
pnpm check:repofrom the verified commit hook — passed. - Prettier and
git diff --check— passed. - Public-repo guard over commit messages and the full PR delta — passed.
- Typecheck: not applicable to this PR delta; it adds one Markdown record and no typed source.
- Local a11y audit: not run, as required.
TIME total 9m
- setup 3m — fetched current main/base, read the required kit, cut one dedicated worktree, cloned dependencies
- reading 3m — PR history, exact diff, parent contract, review rules
- measuring 2m — range-diff, merge-tree, validator, 50 focused tests, guard
- writing 1m — correction, presentation, critic pass
- waste 1m — initially rebased the whole stack onto main, then corrected to the PR's existing bootstrap base before any push
- Exact-head GitHub Actions are blocked by the stack bootstrap: the
Spec owner gateworkflow runs from the parent branch but intentionally checks out default-branchmain, where its helper has not landed yet. Thereconcilejob therefore fails withMODULE_NOT_FOUND; Vercel and CLA pass. This is a parent-order dependency, not a defect in the one-file PR delta.
VERDICT CHECK: correct — approve, posted mechanically as a comment because this is the maintainer's draft. LENGTH: 30 words before attribution → at the approve cap. VIOLATIONS: none. WOULD SHE HAVE POSTED IT: yes — it states the maintenance result, preserves draft state, and names the only content correction without reopening policy. REWRITE: not required.
Checks: required sections and version pair are present; architecture budget/source/reset fields are literal; prior review state is reconciled; no visual claim lacks evidence; no human decision is delegated; no internal residue or private control text appears in the public REVIEW block; the attribution is exact.
The only defect was stale status prose inside a current record. The smallest fix was deletion; no architecture wording or owner decision was changed.
GATE 1: CLEAN.
Nothing; the review was posted exactly as drafted.