From 6efba896f4e52e46da51628ed71f0c07d4522636 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Mon, 27 Jul 2026 11:58:54 -0400 Subject: [PATCH] Add Chisel quality calibration --- .chisel/calibration/calibration.md | 144 +++++ .chisel/calibration/work/adjudication.md | 249 +++++++++ .../calibration/work/consistency-review.md | 112 ++++ .chisel/calibration/work/reviewer-1.md | 182 +++++++ .chisel/calibration/work/reviewer-2.md | 176 +++++++ .chisel/calibration/work/reviewer-3.md | 490 ++++++++++++++++++ .chisel/calibration/work/validation-1.md | 424 +++++++++++++++ .chisel/calibration/work/validation-2.md | 450 ++++++++++++++++ .chisel/calibration/work/validation-3.md | 447 ++++++++++++++++ 9 files changed, 2674 insertions(+) create mode 100644 .chisel/calibration/calibration.md create mode 100644 .chisel/calibration/work/adjudication.md create mode 100644 .chisel/calibration/work/consistency-review.md create mode 100644 .chisel/calibration/work/reviewer-1.md create mode 100644 .chisel/calibration/work/reviewer-2.md create mode 100644 .chisel/calibration/work/reviewer-3.md create mode 100644 .chisel/calibration/work/validation-1.md create mode 100644 .chisel/calibration/work/validation-2.md create mode 100644 .chisel/calibration/work/validation-3.md diff --git a/.chisel/calibration/calibration.md b/.chisel/calibration/calibration.md new file mode 100644 index 000000000..a5777e0c6 --- /dev/null +++ b/.chisel/calibration/calibration.md @@ -0,0 +1,144 @@ +# Chisel Quality Calibration + +Calibration v1 · cartography v1 · revision `6bb6b5efcc0e36b52e3c097f532d9f2c00914c6c` · 2026-07-27T15:55:07Z +Sample: `fabro-workflow`, `fabro-http`, `fabro-web-app`, `repository-ci` · Control: `fabro-checkpoint` at `6bb6b5efcc0e36b52e3c097f532d9f2c00914c6c` +Evaluators: GPT-5 (Codex primary and independent reviewers) + +## How to Use This Calibration + +Judge each mapped component against its purpose and direct repository evidence. +Do not grade on a curve. Apply one score per lens, and count a finding under +only its primary lens. + +**Isolated** means contained at an edge; normal callers and routine changes do +not encounter it. **Central** means part of a mapped entry point, common path, +or recurring change. A **routine change** is an ordinary extension or +maintenance task implied by the component's mapped purpose. + +Infer routine work from the mapped purpose and traced common paths; a public +method alone does not establish frequency. A directly evidenced central concern +caps the component's lens score rather than being averaged against healthier +sub-responsibilities. Necessary delegation inside a clear owner is not pressure, +and size or internal busyness alone does not lower ownership. + +Use **N/E** when evidence is insufficient. Never convert missing evidence into +a numeric score, and do not penalize a missing lifecycle path without evidence +that the mapped purpose requires it. Score 4 requires a positive production +mechanism and no material friction; tests may corroborate that mechanism but +cannot create it or become a second authority merely by asserting its contract. + +## Lenses + +### `ownership-boundaries` — Ownership and boundaries + +**Does each responsibility and lifecycle have a clear home, with dependencies +pointing in the intended direction?** Includes responsibility, state, resource, +dependency, and lifecycle placement; excludes local control flow, naming, +types, API meaning, and repeated policy alone. + +### `simplicity` — Simplicity + +**Is the implementation no more complex, indirect, or general than necessary?** +Includes common-path traceability, control flow, indirection, abstraction, and +configuration burden; excludes placement, domain meaning, and independently +repeated knowledge. + +### `domain-model` — Domain model + +**Does each domain concept have one clear meaning and valid shape?** Includes +types, terminology, legal states, conversions, validation, and API semantics; +excludes module placement, lifecycle ownership, and repetition preserving one +meaning. + +### `duplication-knowledge` — Duplication of knowledge + +**Are policies, invariants, decisions, and transformations authoritative rather +than repeated?** Includes semantic repetition and manual synchronization; +excludes harmless syntax, coincidental similarity, and unification that would +create a parameterized mega-abstraction. + +## Observable Anchors + +| Score | Ownership and boundaries | Simplicity | Domain model | Duplication of knowledge | +|---:|---|---|---|---| +| 4 | One owner contains the mapped responsibility's state and complete lifecycle. | A production mechanism makes the necessary common path directly traceable. | Canonical types reject invalid states before every common-path interpretation. | One authoritative mechanism enforces each recurring policy, invariant, or transformation. | +| 3 | Ownership friction is isolated outside routine changes. | Unnecessary indirection is isolated outside routine changes. | Meaning or validation friction is isolated outside routine changes. | Repeated knowledge is isolated outside routine changes. | +| 2 | Routine changes coordinate competing owners or reverse the mapped dependency direction. | Routine changes repeatedly navigate competing paths, avoidable layers, or configuration machinery. | Routine changes reconcile recurring meanings, conversions, or invalid intermediate states. | Routine changes manually synchronize the same policy, invariant, or transformation across recurring locations. | +| 1 | No stable owner or dependency direction can be identified for the responsibility. | No stable common path can be traced through the implementation. | No stable meaning or legal shape can be identified for a core concept. | No stable authority can be identified for recurring domain knowledge. | + +## Decision Rules + +1. A directly evidenced central concern caps the component's lens score; do not average it against healthier sub-responsibilities. +2. Judge ownership against the map, not type names; when routine callers reconstruct a mapped lifecycle from low-level primitives, ownership fits 2. +3. A check owns trigger coverage for every path it scans; non-triggering routine targets are ownership pressure, while nonexistent selector values are domain-model pressure. +4. An unused production dependency or parallel entry layer is isolated simplicity friction, capping 4 at 3 when the common path remains direct. +5. Caller validation or a typed destination does not isolate an invalid-capable mapped entry; routine common-path use of that shape fits 2. +6. Concrete second semantic representations cap 4 at 3; score 2 only when an ordinary mapped change must synchronize them, not merely because call sites repeat. + +## Confidence + +Confidence describes evidence quality, not severity. **High** requires direct +evidence across relevant common and boundary paths; final High also requires +independent readings to converge. **Medium** has a material ambiguity or +coverage gap. **Low** is partial or substantially inferential. + +## Classifying a Finding + +- Where should this responsibility or lifecycle live? → `ownership-boundaries` +- Why is this much machinery necessary? → `simplicity` +- What does this name, type, state, or API value mean? → `domain-model` +- Why is this knowledge authoritative in several places? → `duplication-knowledge` + +Tags are diagnostic metadata, not additional scores: + +```text +abstraction-burden boundary-leakage configuration-sprawl +control-flow conversion-sprawl dependency-direction +generality indirection invalid-states +lifecycle misplaced-responsibility +ownership repeated-invariant repeated-policy +repeated-test-knowledge repeated-transformation +state-coupling type-sprawl vocabulary-drift +``` + +## Repository Examples + +### `ownership-boundaries` + +- `lib/components/fabro-workflow/src/lifecycle/mod.rs:WorkflowLifecycle` shows a central orchestrator can own callback order through focused delegates; reviewers must still inspect terminal paths before calling lifecycle ownership contained. +- `apps/fabro-web/app/lib/api-client.ts:apiData` and `apps/fabro-web/app/lib/queries.ts:useRun` keep shared transport and read lifecycles out of route composition; a busy route alone is not boundary leakage. + +### `simplicity` + +- `lib/foundation/fabro-http/src/lib.rs:define_builder!` makes async and blocking construction traceable through one necessary mechanism; local macro indirection can reinforce simplicity. +- `lib/components/fabro-workflow/src/operations/start.rs:RunSession::run` exposes a linear phase sequence, while service reshaping across phase inputs shows that a stable path can still carry recurring machinery. + +### `domain-model` + +- `lib/components/fabro-workflow/src/event/events.rs:Event::StageCompleted` uses string status before `lib/components/fabro-workflow/src/event/convert.rs:stage_status_from_string` reparses it; a typed durable result does not isolate this common-path intermediate. +- `lib/foundation/fabro-http/src/lib.rs:ProxyPolicy` and `ProxyPolicy::resolve_with_env_value` demonstrate a closed policy vocabulary whose invalid boundary values are rejected. + +### `duplication-knowledge` + +- `lib/components/fabro-workflow/src/event/names.rs:event_name` and `lib/components/fabro-workflow/src/event/convert.rs:event_body_from_event` show manual mappings that a routine event extension must synchronize, even when exhaustive matches detect omissions. +- `.github/workflows/rust.yml:on.push.paths` and `.github/workflows/rust.yml:on.pull_request.paths` demonstrate duplicated trigger knowledge: one source-area change requires two manual policy edits. + +## Control Baseline + +`fabro-checkpoint` at `6bb6b5efcc0e36b52e3c097f532d9f2c00914c6c`: + +| Lens | Score | Confidence | +|---|---:|---| +| Ownership and boundaries | 2 | High | +| Simplicity | 3 | High | +| Domain model | 2 | Medium | +| Duplication of knowledge | 3 | Medium | + +## Recalibration Triggers + +Recalibrate only for a rubric change, a material cartography change, a model +change with demonstrated drift, or inconsistent scores on the control sample. + +## Open Questions + +None. diff --git a/.chisel/calibration/work/adjudication.md b/.chisel/calibration/work/adjudication.md new file mode 100644 index 000000000..8f3f7bb28 --- /dev/null +++ b/.chisel/calibration/work/adjudication.md @@ -0,0 +1,249 @@ +# Calibration Adjudication + +Revision: `6bb6b5efcc0e36b52e3c097f532d9f2c00914c6c` + +Cartography: v1 at `2bcf94fed8a9b429f18d9196fa824711d6f4cb0a`. +The only later commit adds cartography artifacts, so the mapped code paths are +unchanged at the assessed revision. + +Sample: `fabro-workflow`, `fabro-http`, `fabro-web-app`, `repository-ci`. +Control: `fabro-checkpoint`. + +## Independent Score Matrix + +Cells list reviewer 1 / reviewer 2 / reviewer 3. + +| Component | Ownership and boundaries | Simplicity | Domain model | Duplication of knowledge | +|---|---:|---:|---:|---:| +| `fabro-workflow` | 2 / 3 / 4 | 2 / 2 / 2 | 2 / 3 / 2 | 2 / 2 / 2 | +| `fabro-http` | 4 / 4 / 4 | 4 / 4 / 4 | 4 / 3 / 4 | 3 / 4 / 4 | +| `fabro-web-app` | 4 / 4 / 3 | 2 / 2 / 2 | 2 / 2 / 2 | 2 / 2 / 2 | +| `repository-ci` | 4 / 4 / 3 | 3 / 3 / 3 | 2 / 3 / 2 | 2 / 2 / 2 | + +Unanimous pairs establish that central machinery may still have a stable path: +`fabro-workflow` is 2 for simplicity and duplication; `fabro-web-app` is 2 for +simplicity, domain model, and duplication; and `repository-ci` is 3 for +simplicity and 2 for duplication. `fabro-http` is unanimously 4 for ownership +and simplicity. + +## Material Disagreements + +### `fabro-workflow` × ownership and boundaries — 2 / 3 / 4 + +- **Evidence:** `pipeline/mod.rs` and `pipeline/types.rs` give the normal run + explicit phase owners; `lifecycle/mod.rs:WorkflowLifecycle` owns callback + ordering through focused delegates. +- **Counterevidence:** terminal completion and failure are also constructed in + `pipeline/finalize.rs:build_terminal_event`, + `operations/start.rs:emit_workflow_run_failed`, + `operations/start.rs:persist_terminal_engine_failure`, completion/drop + guards, retry, and archive operations. +- **Ambiguous rule:** two reviewers judged the clear normal path; one judged + whether the same lifecycle has one home across normal and exceptional paths. +- **Discriminator:** inspect every recurring terminal path. A routine + terminal-contract change crossing several operation owners is score-2 + ownership pressure even when the success path is well partitioned. +- **Draft adjudication:** 2. + +### `fabro-workflow` × domain model — 2 / 3 / 2 + +- **Evidence:** `pipeline/types.rs` encodes phase states and canonical product + records are reused. +- **Counterevidence:** `event/events.rs:Event::StageCompleted` carries a string + status; `lifecycle/event.rs:EventLifecycle::after_node` serializes a typed + outcome and `event/convert.rs:stage_status_from_string` reparses it with an + unknown-value fallback. +- **Ambiguous rule:** whether a typed durable event isolates an invalid + intermediate representation on the common producer path. +- **Discriminator:** common-path invalid intermediate states are central even + when the durable result is typed. +- **Draft adjudication:** 2. + +### `fabro-http` × domain model — 4 / 3 / 4 + +- **Evidence:** `ProxyPolicy`, `resolve_with_env_value`, and + `HttpClientBuildError` form a closed policy with explicit precedence and + rejection. +- **Counterevidence:** public builders expose both + `proxy_policy(ProxyPolicy::Disabled)` and lower-level `no_proxy()`. +- **Ambiguous rule:** whether a lower-level transport control creates a second + meaning for the repository policy. +- **Discriminator:** an escape hatch does not split the canonical concept when + the typed policy remains closed and its precedence is enforced. +- **Draft adjudication:** 4. + +### `fabro-http` × duplication of knowledge — 3 / 4 / 4 + +- **Evidence:** `define_builder!` is the shared async/blocking authority and + `ProxyPolicy::resolve` owns precedence. +- **Counterevidence:** adding a policy variant synchronizes the enum, parser, + expected-value error text, behavior match, and tests. +- **Ambiguous rule:** whether co-location and exhaustive matching make all + policy vocabulary authoritative. +- **Discriminator:** hypothetical variants do not establish routine + recurrence; exhaustive compiler-checked behavior remains one authority + unless direct evidence shows recurring manual synchronization. +- **Draft adjudication:** 4. + +### `fabro-web-app` × ownership and boundaries — 4 / 4 / 3 + +- **Evidence:** `entry.tsx`, route graphs, `lib/api-client.ts`, queries, + mutations, effect hooks, and the build script give shared responsibilities + visible homes. +- **Counterevidence:** `install-app.tsx` and `routes/run-stages.tsx` contain + several central transformations and presentation concerns. +- **Ambiguous rule:** whether a busy but clearly identified route owner is + boundary pressure or simplicity pressure. +- **Discriminator:** do not lower ownership for internal complexity unless + routine changes cross another owner or reverse the mapped dependency + direction. +- **Draft adjudication:** 4. + +### `repository-ci` × ownership and boundaries — 4 / 4 / 3 + +- **Evidence:** Rust and TypeScript workflows have distinct validation jobs, + narrow permissions, and delegate build procedures to repository commands. +- **Counterevidence:** the Rust clippy job embeds the repository's legacy-auth + vocabulary check. +- **Ambiguous rule:** whether enforcement of a product migration invariant is + misplaced when CI owns validation but not the underlying vocabulary. +- **Discriminator:** a named invariant check may live in CI, but its product + vocabulary must remain authoritative elsewhere; this isolated boundary + friction fits 3. +- **Draft adjudication:** 3. + +### `repository-ci` × domain model — 2 / 3 / 2 + +- **Evidence:** job, runner, permission, and test-mode vocabulary is otherwise + coherent. +- **Counterevidence:** `rust.yml:on.*.paths` names nonexistent `openapi/**` + rather than `docs/public/api-reference/fabro-api.yaml`, and + `zizmor.yml:rules.stale-action-refs.ignore` identifies exceptions by stale + line positions. +- **Ambiguous rule:** whether configuration references are domain vocabulary + or only duplicated operational data. +- **Discriminator:** identifiers that control central behavior are domain + vocabulary; missing or stale referents create score-2 pressure. +- **Draft adjudication:** 2. + +## Draft Anchor Decisions + +- Anchor score 4 on a positive enforcing mechanism, never absence of a defect. +- Separate owner clarity from the amount of machinery inside that owner. +- Treat invalid common-path intermediate states as domain-model pressure. +- Treat repeated semantic decisions as duplication only when routine changes + require manual synchronization. +- Treat mapped configuration identifiers as domain vocabulary. +- Reserve N/E for a lens without direct evidence; no sampled pair required it. + +## Consistency Review + +The fresh reviewer applied only the written draft to `fabro-checkpoint` and +reported: + +| Lens | Score | Evidence confidence | +|---|---:|---| +| Ownership and boundaries | 2 | Medium | +| Simplicity | 3 | High | +| Domain model | 2 | High | +| Duplication of knowledge | 2 | High | + +The control exposed four material wording problems: + +1. The draft did not say how a component-level score combines several + responsibilities, or whether positive mechanisms and friction can coexist + at score 4. +2. Necessary layered delegation could satisfy the original ownership and + simplicity score-2 wording. +3. The domain rules did not say when a public low-level API is an escape hatch + or what score a common invalid intermediate implies. +4. Decision rule 5 contradicted the duplication anchor by assigning routine + string synchronization to score 3. + +The revision now says that a central concern caps rather than averages, score 4 +requires a positive production mechanism without material friction, public +surface alone does not establish routine work, and missing paths are not +negative without mapped-purpose evidence. The anchors now distinguish competing +owners from necessary delegation and maintainer navigation from runtime +layering. Decision rules 2–6 resolve scoped lifecycle handoff, necessary +delegation, common-path invalid states, direct evidence of recurring +synchronization, and configuration identifiers. Tests corroborate production +authorities but are not second authorities merely because they restate a +contract. + +All 16 wording observations in `consistency-review.md` are covered by those +changes or by the existing primary-lens and confidence sections. No consistency +objection remains open before validation. + +## Validation + +### Round 1 + +| Assignment | Validator 1 | Validator 2 | Validator 3 | Result | +|---|---:|---:|---:|---| +| `fabro-workflow` × ownership | 2 | 2 | 2 | Resolved | +| `fabro-workflow` × domain | 2 | 2 | 2 | Resolved | +| `fabro-http` × domain | 4 | 4 | 4 | Resolved | +| `fabro-http` × duplication | 4 | 4 | 3 | Repeated adjacent split | +| `fabro-web-app` × ownership | 4 | 4 | 4 | Resolved | +| `repository-ci` × ownership | 4 | 2 | 4 | Non-adjacent split | +| `repository-ci` × domain | 2 | 2 | 2 | Resolved | +| Control × ownership | 2 | 4 | 2 | Non-adjacent split | +| Control × simplicity | 4 | 4 | 3 | Adjacent split | +| Control × domain | 2 | 3 | 2 | Adjacent split | +| Control × duplication | 3 | 2 | 3 | Adjacent split | + +The sample's workflow lifecycle, event status, HTTP policy model, web +composition, and CI identifier anchors now converge. Six assignments require +the permitted final simplification: + +- HTTP diagnostic allowed-value text is a concrete second semantic + representation, even though the macro is the behavioral authority. +- A CI check owns trigger coverage for every path its embedded policy scans; + this is distinct from the domain meaning of a nonexistent selector. +- Control ownership is judged against the mapped metadata-branch purpose, not + against narrower names on `Store` and `BranchStore`. +- The control's unused dependency and unused parallel entry layer are isolated + simplicity friction rather than evidence-free public breadth. +- Validation in an external caller does not make an invalid-capable mapped + entry type enforce its own legal shape. +- Repeated fixed Git protocol syntax is a concrete second representation, but + multiple current call sites alone do not make changing that protocol an + ordinary mapped change. + +Decision rules 2–6 now state those discriminators directly. Round 2 will +re-score only the six unresolved assignments. + +### Round 2 + +| Assignment | Validator 1 | Validator 2 | Validator 3 | Result | +|---|---:|---:|---:|---| +| `fabro-http` × duplication | 3 | 3 | 3 | Resolved | +| `repository-ci` × ownership | 2 | 2 | 2 | Resolved | +| Control × ownership | 2 | 2 | 2 | Resolved | +| Control × simplicity | 3 | 3 | 3 | Resolved | +| Control × domain | 2 | 2 | 2 | Resolved | +| Control × duplication | 3 | 3 | 3 | Resolved | + +All round-2 scores converge. The final control baseline is ownership 2 +(High), simplicity 3 (High), domain model 2 (Medium), and duplication of +knowledge 3 (Medium). Domain confidence remains Medium because one validator +found a material ambiguity over whether low-level Git path validation belongs +inside the component. Duplication confidence remains Medium because stable +protocol syntax is concrete repetition but has limited demonstrated change +burden. + +Across both validation rounds, the final disputed sample scores are: + +| Component | Ownership and boundaries | Domain model | Duplication of knowledge | +|---|---:|---:|---:| +| `fabro-workflow` | 2 | 2 | — | +| `fabro-http` | — | 4 | 3 | +| `fabro-web-app` | 4 | — | — | +| `repository-ci` | 2 | 2 | — | + +No non-adjacent or repeated adjacent split remains. + +## Open Questions + +None. diff --git a/.chisel/calibration/work/consistency-review.md b/.chisel/calibration/work/consistency-review.md new file mode 100644 index 000000000..e41ff2c26 --- /dev/null +++ b/.chisel/calibration/work/consistency-review.md @@ -0,0 +1,112 @@ +# Chisel Consistency Review: `fabro-checkpoint` + +Revision: `6bb6b5efcc0e36b52e3c097f532d9f2c00914c6c` + +Scope: `lib/components/fabro-checkpoint/**` only. The scored evidence is the manifest, production source, and unit tests at the pinned revision. I did not inspect callers, sample reviews, adjudication, or any other file under `.chisel/calibration/work/`. + +## Scores + +| Lens | Score | Confidence | +|---|---:|---| +| `ownership-boundaries` | 2 | Medium | +| `simplicity` | 3 | High | +| `domain-model` | 2 | High | +| `duplication-knowledge` | 2 | High | + +Confidence here describes this reading's evidence quality. The rubric's additional requirement that a final High confidence needs independent convergence can only be decided during adjudication. + +## `ownership-boundaries`: 2 + +The central branch lifecycle crosses two public owners. `BranchStore` stores the branch name and owns bootstrap plus normal branch reads and writes (`branch.rs:20-209`), but branch cleanup is exposed only as `Store::delete_ref(branch)` (`git.rs:215-226`). `BranchStore` keeps both its `Store` reference and branch name private and has no cleanup/archive operation. A caller therefore has to retain the same raw branch identity and leave the branch-scoped interface for cleanup. Bootstrap sequencing is also caller-owned: `BranchStore::new` does not establish the branch, writes fail when it is absent, and every writable test explicitly calls `ensure_branch` first (`branch.rs:26-81, 282-343`). This is recurring lifecycle work rather than an isolated edge, especially under decision rule 1. Primary tags: `lifecycle`, `ownership`. + +Strongest counterevidence: once initialized, `BranchStore::write_with` keeps the read-modify-write sequence together and delegates only Git object/ref primitives to `Store` (`branch.rs:56-82`). The dependency direction is stable: branch storage depends on the lower-level Git store, not vice versa. + +Why adjacent scores do not fit: + +- **1 does not fit:** `BranchStore` is a stable, identifiable owner for the common branch-scoped read/write responsibility, and `Store` is a coherent lower-level Git owner. +- **3 does not fit:** the split includes explicit bootstrap and cleanup paths. Decision rule 1 says recurring terminal ownership cannot be treated as isolated merely because the success path is clear. + +Confidence is Medium because the split is direct, but the scoped evidence cannot show whether archive, retry, and cleanup are deliberately owned by a higher-level caller. + +## `simplicity`: 3 + +The common write path is directly traceable: `write_entry`/`write_entries` prepare blobs, `write_with` reads the tip tree, applies one mutation, writes one commit, and advances one ref (`branch.rs:56-109`). `Store::read_tree` and `Store::write_tree` use a single flat `TreeEntries` representation with private recursive helpers (`git.rs:39-99, 141-159, 229-310`). These are positive reinforcing mechanisms, not just an absence of complexity. + +The remaining simplicity pressure is isolated configuration burden. The manifest declares `fabro-store`, `serde`, and the dev dependency `chrono` (`Cargo.toml:16-28`), but none is referenced anywhere in the component source or tests at this revision. The public `Store::repo` escape hatch (`git.rs:112-114`) and the lower-level object API also add surface area, but normal branch writes do not have to choose among competing implementations. Primary tag: `configuration-sprawl`. + +Strongest counterevidence to lowering the score: the component has one linear common mutation path, and its indirection corresponds directly to Git's blob/tree/commit/ref structure. + +Why adjacent scores do not fit: + +- **2 does not fit:** ordinary reads and writes do not repeatedly traverse competing orchestration paths or configuration machinery; the `BranchStore` to `Store` layering is stable and direct. +- **4 does not fit:** the centralized mutation path is a qualifying positive mechanism, but the unused manifest dependencies are concrete unnecessary configuration rather than necessary machinery. + +Confidence is High because all component files are in scope, so the dependency non-use and the full common write path are directly observable. + +## `domain-model`: 2 + +The common tree-entry producer accepts invalid intermediate path states. `TreeEntries` hides its map, but its public `set` accepts any `Into` without validating a relative Git path (`git.rs:46-61`). Both `BranchStore::write_entry` and `write_entries` feed caller-provided `&str` paths directly into it (`branch.rs:84-109`), and `build_dir_node` later assigns meaning by splitting the strings on `/` (`git.rs:270-294`). Empty components, leading/trailing separators, and file/directory prefix collisions are therefore representable in the canonical intermediate type and reach late Git-tree construction rather than being rejected at the common boundary. Branch identity is likewise an arbitrary `String` until `git2` receives the synthesized ref name (`branch.rs:20-38`, `git.rs:182-197`). This is central invalid-state pressure under decision rule 3, not an isolated low-level escape hatch. Primary tag: `invalid-states`. + +The small helper `sharded_path` is corroborating boundary evidence: its contract says the input is a hex ID, but its public signature accepts any `&str` and slices at a caller-provided byte offset (`branch.rs:211-220`), so a non-ASCII input can panic rather than be rejected as invalid input. + +Strongest counterevidence: `FileMode` is a closed enum and `TreeEntries` keeps ordering and representation private (`git.rs:13-99`). `Error` also distinguishes a missing branch from generic Git failures (`error.rs:5-18`). The component therefore has stable concepts even though common constructors do not preserve all their invariants. + +Why adjacent scores do not fit: + +- **1 does not fit:** branch storage, tree entries, file modes, authors, and trailers all have recognizable, stable meanings. +- **3 does not fit:** raw paths and branch names enter the common public read/write boundary, so validation friction is not isolated outside routine use. + +Confidence is High because the accepting producers and their downstream interpretation are both visible within the scoped common path. + +## `duplication-knowledge`: 2 + +The transformation “find a path in a commit tree, treat only `NotFound` as absence, load the entry as a blob, and copy its bytes” is independently implemented by `BranchStore::read_entry`, `BranchStore::read_entries`, and `Store::read_blob_at` (`branch.rs:119-158`, `git.rs:200-213`). An ordinary maintenance change to missing-entry or entry-kind behavior must synchronize all three common read locations. Ref qualification is also repeated in `update_ref`, `resolve_ref`, and `delete_ref` (`git.rs:182-226`). + +Trailer grammar supplies independent corroboration at the commit-message edge: `": "` formatting/detection is separately encoded by `append`, `parse`, `format_message`, and `has_trailing_trailer_block` (`trailer.rs:9-25, 28-42, 45-65, 68-87`). Primary tags: `repeated-transformation`, `repeated-policy`. + +Strongest counterevidence: important write knowledge is authoritative. `BranchStore::write_with` centralizes tip loading, parent linkage, commit creation, and ref advancement, while `GitAuthor::default` centralizes the fallback identity (`branch.rs:56-82`, `author.rs:13-35`). + +Why adjacent scores do not fit: + +- **1 does not fit:** the repeated implementations currently agree, and stable authorities exist for branch mutation, author defaults, and file-mode conversion. +- **3 does not fit:** the repeated blob-read transformation appears on the public latest-entry and multi-entry common paths, so a routine storage-policy change encounters it centrally rather than only at an edge. + +Confidence is High because the repeated transformations and the mechanisms that are already centralized can both be enumerated completely inside the scoped component. + +## Rubric wording audit + +The following rules or anchors were ambiguous or non-discriminating in this application. I resolved each explicitly rather than silently choosing an interpretation. + +1. **One component score across several responsibilities.** The instruction says to judge “each mapped component,” while the anchors use singular phrases such as “a mapped responsibility” and “a core concept.” It does not say whether to average sub-responsibilities, take the worst concern, or weight by centrality. I scored the mapped checkpoint-storage responsibility and let a directly evidenced central concern cap the lens; isolated author/trailer helpers could affect a score only at 3 versus 4. + +2. **How to establish “routine” and “central” with component-only evidence.** A public method may be a mapped entry point without being frequent, and scoped evidence cannot establish caller frequency. I treated bootstrap, latest reads/writes, and cleanup as routine because they are ordinary lifecycle operations implied by branch storage. I did not infer frequency for unrelated external call sites. + +3. **N/E threshold versus an absent lifecycle path.** “Use N/E when evidence is insufficient” does not say whether a missing archive/retry API is negative evidence, out of scope, or grounds for N/E. I scored paths that are directly present (bootstrap, normal operation, cleanup), did not penalize an unobserved archive/retry design, and lowered ownership confidence for the coverage gap. + +4. **Score 3 and score 4 overlap in every lens.** A positive reinforcing mechanism can coexist with isolated friction, so the score-4 requirement and score-3 anchor can both be true. I treated any evidenced unnecessary/frictional mechanism as a cap at 3; score 4 requires both a positive mechanism and no material friction in the mapped responsibility. This is why the unused manifest dependencies keep simplicity at 3 despite `write_with`. + +5. **What qualifies as a “positive reinforcing mechanism.”** The rubric does not say whether tests, encapsulation alone, or a production authority qualifies. I required an operative production mechanism that funnels behavior or rejects invalid construction. Tests alone did not qualify. + +6. **Ownership score 2 versus ordinary delegation.** “Cross recurring owners or dependency boundaries” could penalize every layered implementation. Decision rule 2 partly resolves this, but “same responsibility” remains subjective. I treated `BranchStore` calling `Store` during a write as ordinary delegation; I counted cleanup only because the caller must leave the branch-scoped owner and supply its identity again. + +7. **Decision rule 1 when terminal operations live at a lower abstraction.** The rule says not to isolate recurring terminal owners but does not define whether a lower-level deletion primitive is a second owner or a delegate. Because `BranchStore` offers no cleanup interface and keeps the needed state private, I treated `Store::delete_ref` as a lifecycle-owner crossing, not merely internal machinery. + +8. **Simplicity score 2’s “repeatedly traverse.”** It is unclear whether this means runtime calls passing through multiple necessary layers, or maintainers choosing among competing paths repeatedly. I used the latter interpretation, consistent with the lens question and decision rule 2; necessary Git layers did not lower the score. + +9. **Decision rule 2’s “simplicity pressure.”** The rule labels machinery inside an owner as pressure even though the lens expressly permits necessary complexity and gives no score consequence for “pressure.” I treated machinery as evidence to test for necessity, not as an automatic deduction. + +10. **Domain score 4 versus decision rule 4’s escape hatch.** “Every common boundary” is not defined, and a public low-level API can be called common or an escape hatch depending on external usage. I treated `TreeEntries::set` as common because `BranchStore::write_with`, `write_entry`, and `write_entries` use it directly; `Store::repo` was treated as an escape hatch. + +11. **Decision rule 3 does not identify a score boundary.** It says a typed durable value does not “repair domain pressure,” but does not say whether a common invalid intermediate means 2 or merely prevents 4. I mapped common-path invalid intermediates to the score-2 anchor (“routine changes reconcile ... invalid intermediate states”); isolated invalid intermediates would map to 3. + +12. **Duplication score 2 versus decision rule 5.** Rule 5 says to score 3 when a routine vocabulary change requires synchronization, while the score-2 anchor says routine synchronization of the same policy/invariant/transformation is score 2. Those statements conflict unless “vocabulary” is an unstated special case. I treated rule 5 narrowly as an exception for localized, string-only vocabulary at an edge. The score-2 finding here rests instead on repeated behavioral blob-read transformations on common paths. + +13. **What test repetition counts as knowledge duplication.** The `repeated-test-knowledge` tag suggests tests can count, but the anchors do not distinguish duplicated policy from assertions that intentionally restate expected behavior. I did not count an assertion of a production contract as a second authority. Repeated test fixture setup was only isolated counterevidence and did not drive a numeric score. + +14. **Decision rule 6 lacks a lens and defines neither “current referent” nor “line selector.”** Its opening phrase points toward `domain-model`, while duplicated CI selectors could point toward `duplication-knowledge`; its mandatory score 2 also bypasses centrality analysis. It had no referent in this component, so I did not apply it. If applicable, I would classify a single invalid identifier under domain model and synchronized copies under duplication. + +15. **The “primary lens only” rule does not explain multi-causal facts.** Raw strings can simultaneously expose invalid states, repeat vocabulary, and force lifecycle handoffs. I assigned each negative fact once by its primary question: lifecycle handoff to ownership, unused dependencies to simplicity, raw path legality to domain, and repeated lookup/ref/trailer behavior to duplication. + +16. **Confidence High cannot be finalized by one reviewer.** “Final High also requires independent readings to converge” is not decidable during an independent review. I reported evidence-quality confidence now and left final convergence to adjudication. + +All other score-1 versus score-2 distinctions were discriminating here: the component consistently has identifiable owners, paths, concepts, and intended policies, so none of the “no stable ... can be identified” anchors fit. diff --git a/.chisel/calibration/work/reviewer-1.md b/.chisel/calibration/work/reviewer-1.md new file mode 100644 index 000000000..d6c0140da --- /dev/null +++ b/.chisel/calibration/work/reviewer-1.md @@ -0,0 +1,182 @@ +# Calibration review — reviewer 1 + +Revision reviewed: `6bb6b5efcc0e36b52e3c097f532d9f2c00914c6c` + +Scope: `fabro-workflow`, `fabro-http`, `fabro-web-app`, and `repository-ci` as routed by `.chisel/cartography/codebase-map.md`. I excluded `apps/fabro-web/app/components/playground/**` from `fabro-web-app`, and limited `repository-ci` to `.github/workflows/rust.yml`, `.github/workflows/typescript.yml`, and `.github/zizmor.yml`. The routed paths have no changes between the map revision and the reviewed revision. + +## Provisional ratings + +| Component | Ownership boundaries | Simplicity | Domain model | Duplication of knowledge | +| --- | --- | --- | --- | --- | +| `fabro-workflow` | **2 — High** | **2 — High** | **2 — High** | **2 — High** | +| `fabro-http` | **4 — High** | **4 — High** | **4 — High** | **3 — High** | +| `fabro-web-app` | **4 — High** | **2 — High** | **2 — High** | **2 — High** | +| `repository-ci` | **4 — High** | **3 — High** | **2 — High** | **2 — High** | + +## `fabro-workflow` + +### Ownership boundaries — 2, High confidence + +The component has a clear top-level phase boundary: `pipeline/mod.rs` orders parse, transform, validate, initialize, execute, finalize, and pull-request processing; `pipeline/types.rs` gives those phases distinct result types. `pipeline/execute.rs:execute`, `graph.rs:WorkflowGraph`, and `node_handler.rs:WorkflowNodeHandler` also make the boundary with the generic `fabro-core` executor explicit. `lifecycle/mod.rs:WorkflowLifecycle` composes named lifecycle owners instead of placing every callback in the executor. + +The pressure appears in terminal-run ownership. The normal path is owned by `pipeline/finalize.rs:finalize` and `pipeline/finalize.rs:build_terminal_event`, while engine/bootstrap failures are handled by `operations/start.rs:emit_workflow_run_failed`, `operations/start.rs:persist_terminal_engine_failure`, and the completion/drop guards in `operations/start.rs`. Retry and archive operations also synthesize terminal events in `operations/retry.rs` and `operations/archive.rs`. These paths are understandable individually, but terminal state, persistence, and event emission do not have one stable lifecycle home. + +A representative routine change is adding terminal metadata that must be present for every failed or concluded run. It would require checking or changing `pipeline/finalize.rs:build_terminal_event`, `pipeline/finalize.rs:finalize`, `operations/start.rs:emit_workflow_run_failed`, `operations/start.rs:persist_terminal_engine_failure`, the start-operation guards, and the corresponding terminal paths in `operations/retry.rs` and `operations/archive.rs`. + +Strongest counterevidence: the main successful-run path is explicit and strongly partitioned, and `WorkflowLifecycle` plus `RunServices` give many responsibilities named owners. + +Why adjacent scores do not fit: 3 understates the issue because terminal completion is a central lifecycle concern, not an edge-only exception; an ordinary terminal-contract change must inspect several authorities. 1 does not fit because the normal path and the exceptional paths are still traceable and deliberately named. + +### Simplicity — 2, High confidence + +The top-level flow is readable, but routine run startup crosses a large amount of central wiring. `operations/start.rs:start` enters `execute_persisted_run`, constructs `RunSession`, and then `RunSession::run` coordinates logging, SHA listeners, initialization, cleanup/drain guards, execution, finalization, and pull-request handling. `pipeline/types.rs:InitOptions` carries a large set of run inputs, and `operations/start.rs:RunSession::run` assembles them before handing control to `pipeline/initialize.rs`. The resulting services are then repartitioned through `services.rs:RunServices`, `services.rs:EngineServices`, and `pipeline/execute.rs:execute`. + +A representative routine change is adding a run-scoped service needed by node handlers. It would pass through `operations/start.rs:StartServices` or `RunSession`, `pipeline/types.rs:InitOptions`, `pipeline/initialize.rs:initialize`, `pipeline/types.rs:Initialized`, `services.rs:RunServices`, `services.rs:EngineServices`, and the destructuring/building in `pipeline/execute.rs:execute`. + +Strongest counterevidence: the phase result types in `pipeline/types.rs` and the extracted executor/lifecycle adapters make the long path navigable; the complexity is structured rather than accidental. + +Why adjacent scores do not fit: 3 does not fit because the pressure is on the common startup and execution path, and a small run-scoped dependency change propagates through several central handoff types. 1 does not fit because the ordered pipeline and named handoffs still provide a stable path through the component. + +### Domain model — 2, High confidence + +The strongest positive mechanism is the phase model in `pipeline/types.rs`: `Parsed`, `Transformed`, `Validated`, `Persisted`, `Initialized`, `Executed`, `Concluded`, and `Finalized` constrain which data exists at each stage. Canonical run records are reused from `fabro-types`, and `services.rs:RunServices` documents cancellation ownership. + +However, the core event path weakens those guarantees. `event/events.rs:Event::StageCompleted` carries `status: String`; lifecycle code such as `lifecycle/event.rs` converts `StageOutcome` to a string, and `event/convert.rs:stage_status_from_string` parses it back when creating the durable event. An unknown value is not rejected: it is warned about and converted to `StageOutcome::Failed`. The durable model in `fabro-types` is typed, but the internal central event model permits invalid status values and gives them a lossy fallback meaning. `WorkflowRunCompleted` similarly carries a string status internally. + +Strongest counterevidence: the durable event body and most run/pipeline records use named enums and phase-specific types, so this is not a component with generally unmodeled state. + +Why adjacent scores do not fit: 3 does not fit because stage and run outcomes are central workflow vocabulary used on every execution, and the internal-to-durable boundary permits and silently reinterprets invalid values. 1 does not fit because canonical typed outcomes exist and dominate downstream storage; the break is concentrated at the internal event boundary. + +### Duplication of knowledge — 2, High confidence + +Adding an event requires coordinated knowledge in several central authorities. The internal variant lives in `event/events.rs:Event`; its wire name is separately selected by `event/names.rs:event_name`; durable fields are declared in `fabro-types::EventBody`; conversion is implemented in `event/convert.rs:event_body_from_event`; stored-field behavior is selected in `event/stored_fields.rs:stored_event_fields_for_variant`; and tracing behavior is implemented on `Event`. `docs/internal/events-strategy.md` documents this multi-site procedure, confirming that this is the expected recurring event-evolution path rather than a one-off remnant. + +A representative routine change is adding a persisted workflow event. It touches `event/events.rs:Event`, `event/names.rs:event_name`, the `Event` tracing method, `fabro_types::EventBody`, `event/convert.rs:event_body_from_event`, `event/stored_fields.rs:stored_event_fields_for_variant`, emitters, and any event consumers. + +Strongest counterevidence: `event/emitter.rs:Emitter::emit_with_scope` constructs the canonical run event once before dispatch, exhaustive matches make omissions visible to the compiler, and the strategy document gives maintainers one checklist. + +Why adjacent scores do not fit: 3 does not fit because event evolution is frequent, central workflow work and requires synchronized changes across representations and crates. 1 does not fit because each representation has a stated role and there is a single canonicalization point before dispatch. + +Lens-boundary note: the internal `Event`/durable `EventBody` split could be described as a domain-model issue or duplication. I treated the repeated declarations and conversion sites as duplication of knowledge; the separate `String`-to-`StageOutcome` loss of meaning is the domain-model issue. Likewise, repeated terminal constructors are secondary duplication, but I classified the primary problem as ownership because the key question is which operation owns terminal lifecycle completion. + +## `fabro-http` + +### Ownership boundaries — 4, High confidence + +`lib/foundation/fabro-http/src/lib.rs` is a small, focused owner for HTTP client construction and proxy policy. Callers get approved async or blocking builders and convenience clients from this crate. Repository lint policy in `clippy.toml` disallows direct `reqwest` constructors and points callers to `fabro-http`, so the boundary is reinforced rather than merely conventional. `ProxyPolicy::resolve` also owns the environment-variable authority through `fabro_static::EnvVars::FABRO_HTTP_PROXY_POLICY`. + +Strongest counterevidence: the crate deliberately re-exports several `reqwest` types and carries lint exceptions for those facade exports, so callers are not isolated from every transport detail. + +Why adjacent scores do not fit: 3 does not fit because construction policy, environment precedence, test defaults, and transport facade all have one enforced home with no observed competing builder authority. + +### Simplicity — 4, High confidence + +The common path is short: choose `HttpClientBuilder` or `BlockingHttpClientBuilder`, optionally configure it, resolve `ProxyPolicy`, and build the underlying client. `define_builder!` generates the shared async/blocking surface once, while the async-only `read_timeout` extension remains plainly visible next to the macro invocation. Convenience functions such as `http_client`, `blocking_http_client`, `test_http_client`, and `blocking_test_http_client` expose the common cases directly. + +Strongest counterevidence: macro generation means the two concrete builder implementations are not visible as ordinary source, and async-only options must be added outside the shared definition. + +Why adjacent scores do not fit: 3 does not fit because the macro removes rather than creates routine common-option work: a shared builder option is added in one readable location, while the generated types remain thin wrappers. + +### Domain model — 4, High confidence + +`ProxyPolicy` names the only supported policies, `ProxyPolicy::parse` rejects unknown values, and `ProxyPolicy::resolve_with_env_value` makes precedence explicit: a caller override wins, then the environment value, then the system default. Test helpers force `Disabled`, making local test semantics deliberate. `HttpClientBuildError` distinguishes policy configuration failure from transport construction failure. + +Strongest counterevidence: callers can express no-proxy behavior through both `proxy_policy(ProxyPolicy::Disabled)` and the lower-level `no_proxy()` builder method, and the facade re-exports lower-level proxy types. + +Why adjacent scores do not fit: 3 does not fit because the overlapping entry points do not introduce an ambiguous stored state or silent fallback: the policy values and their precedence are explicit, and invalid environment vocabulary fails closed. + +### Duplication of knowledge — 3, High confidence + +The builder macro is a strong anti-duplication mechanism for async and blocking clients. The remaining policy vocabulary is manually repeated: `ProxyPolicy` variants, `ProxyPolicy::parse`, the expected-value text in `HttpClientBuildError::InvalidProxyPolicy`, and the policy match in the generated `build` method must agree. + +A representative routine change is adding another supported proxy policy. It would touch `ProxyPolicy`, `ProxyPolicy::parse`, the expected-value message on `HttpClientBuildError::InvalidProxyPolicy`, the `define_builder!` build-time match, and policy tests in the same source file. + +Strongest counterevidence: every repeated policy decision is co-located in one small file, and the exhaustive build match makes a missing behavioral branch a compile error. + +Why adjacent scores do not fit: 4 does not fit because the accepted vocabulary and error vocabulary are independently maintained strings. 2 does not fit because the synchronization is confined to one authority and does not force routine callers or neighboring components to change. + +Lens-boundary note: macro use could be counted as simplicity indirection, but its primary effect here is eliminating async/blocking duplication. The generated control flow is small enough that I did not lower simplicity for it. + +## `fabro-web-app` + +### Ownership boundaries — 4, High confidence + +The app has explicit composition points. `app/entry.tsx` selects normal or install mode and installs shared providers; `app/router.tsx` and `app/install-router.tsx` own the two route trees. `app/lib/api-client.ts` owns generated-client construction and uniform API errors, `app/lib/query-keys.ts` owns cache keys, and `app/lib/queries.ts` owns shared reads. The React effects policy is embodied by approved wrappers in `app/hooks/effects.ts`; direct effect usage is concentrated in hooks and live-event libraries rather than route/component bodies. `scripts/build.ts` separately owns deterministic asset building and atomic publication. + +Strongest counterevidence: some cache mutation and API-write coordination remains in route handlers, particularly in the large run and installation screens, so not every server interaction passes through a single application-service layer. + +Why adjacent scores do not fit: 3 does not fit because routing, reads, client configuration, effects, and build publication each have a visible and consistently used owner; route-local writes are appropriate UI orchestration rather than a competing global authority. + +### Simplicity — 2, High confidence + +The normal routing shell is simple, but two central screens concentrate substantial policy and presentation. `app/routes/run-stages.tsx` combines event-to-turn reduction, event filtering, grouping, stage/activity interpretation, row and panel rendering, stage renderer selection, and the route page. `app/install-app.tsx` similarly combines installation state transitions, controller behavior, forms, and view composition. Cross-tab stream coordination in `app/lib/cross-tab-sse.ts` is another large central mechanism. + +A representative routine change is showing a new kind of stage activity in the run timeline. It requires following `app/lib/run-events.ts:STAGE_ACTIVITY_EVENT_TYPES`, `app/routes/run-stages.tsx:STAGE_ACTIVITY_EVENT_SET`, `app/routes/run-stages.tsx:buildStageActivity`, the route's turn/activity types, and the corresponding render helpers in the same large route module. + +Strongest counterevidence: shared event lists, query keys, generated API types, and route helpers provide landmarks, and the activity reducer is deterministic rather than dispersed among many components. + +Why adjacent scores do not fit: 3 does not fit because run-stage interpretation is a common product path and small presentation changes require navigating large modules that mix reduction and rendering concerns. 1 does not fit because the route and install flows remain typed, testable, and traceable from explicit entry points. + +### Domain model — 2, High confidence + +Generated API types provide a strong canonical model for ordinary request/response queries, and several local models use discriminated unions. The live-event boundary is weaker. `app/lib/sse.ts:EventPayload` permits an optional event name plus arbitrary fields. `app/lib/run-events.ts:RunEventPayload` and `app/lib/live-events.ts:LiveEventPayload` repeat mostly optional envelope fields with `properties: unknown`. `app/lib/sse.ts:subscribeToSharedEventSource` parses JSON and casts it to the requested payload type without runtime validation. Common live UI behavior therefore accepts payloads that lack the fields implied by their event names. + +There is additional vocabulary translation in `app/data/runs.ts:RunStatus`, which locally reproduces API run-state kinds and adds presentation state, and compatibility shape probing in `app/lib/run-sandbox-lifecycle.ts:sandboxLifecycleKind` and `sandboxInstance`. + +Strongest counterevidence: generated types remain the authority for normal API calls, `session-stream.ts` and query paths use generated event-envelope types where possible, and the local run status adds a genuine presentation concept rather than merely renaming every API state. + +Why adjacent scores do not fit: 3 does not fit because SSE drives common live run behavior and its central payload model makes invalid event/field combinations representable and unchecked. 1 does not fit because static generated models are sound and the weak representation is concentrated at live and compatibility boundaries. + +### Duplication of knowledge — 2, High confidence + +Live refresh policy is repeated in separate manually curated authorities. `app/lib/run-events.ts:RUN_SUMMARY_EVENTS` lists events that invalidate run summaries, while `app/lib/board-events.ts:BOARD_STATUS_EVENTS` independently lists many of the same run, interview, and pull-request lifecycle events for board refresh. The duplicated payload interfaces in `run-events.ts` and `live-events.ts` add another synchronization surface. + +A representative routine change is adding a lifecycle event that changes both a run summary and its board status. It requires updating `app/lib/run-events.ts:RUN_SUMMARY_EVENTS` and `app/lib/board-events.ts:BOARD_STATUS_EVENTS`, then checking phase derivation in `app/lib/run-phases.ts:deriveRunPhases` and live consumers if the event also changes the visible run phase. + +Strongest counterevidence: stage activity vocabulary is centralized in `app/lib/run-events.ts:STAGE_ACTIVITY_EVENT_TYPES` and imported by the run-stages route; query keys and server contract types are also centralized or generated. + +Why adjacent scores do not fit: 3 does not fit because the repeated invalidation lists govern common live behavior, and a missing update produces stale UI rather than a compile-time failure. 1 does not fit because each list has a clear local purpose and several other high-change vocabularies already have a single authority. + +Lens-boundary note: the repeated loose live-event interfaces are both duplicate declarations and a weak model. I treated representable invalid payloads and unchecked casts as the domain-model finding; I used independently maintained event-invalidation sets as the primary duplication finding. The size of `run-stages.tsx` is primarily simplicity pressure, not evidence that its route ownership is unclear. + +## `repository-ci` + +### Ownership boundaries — 4, High confidence + +`.github/workflows/rust.yml` and `.github/workflows/typescript.yml` have an explicit language split and named jobs for formatting, linting, generated documentation, tests, type checking, and builds. Each workflow sets narrow permissions, concurrency behavior is visible, and toolchain/action versions are pinned. The TypeScript build job's Rust build step has a clear purpose: verify the embedded production SPA through the repository's actual build command. + +Strongest counterevidence: the Rust clippy job contains a repository-specific legacy-auth `git grep` policy check, rather than delegating that policy to a named script or dedicated job. + +Why adjacent scores do not fit: 3 does not fit because the special check is still plainly owned by repository validation, while language-level checks, permissions, and production build validation have unambiguous homes and no competing workflow was observed. + +### Simplicity — 3, High confidence + +The workflows are short and linear, with direct commands corresponding to local development commands. Friction is isolated: setup steps are repeated across jobs, the clippy job embeds a multi-pattern shell assertion for legacy auth identity removal, and the ignored twin E2E selection is encoded directly in a long `nextest` expression. These cost attention but do not obscure the overall validation flow. + +A representative routine change is adding a new TypeScript validation job. It would repeat the checkout, Bun setup, and dependency-install sequence already present in `.github/workflows/typescript.yml:jobs.typecheck`, `jobs.test`, and `jobs.build`, then add the new command. + +Strongest counterevidence: each job can be understood independently, commands are explicit, and there is no multi-layer reusable-workflow indirection. + +Why adjacent scores do not fit: 4 does not fit because repeated setup and inline special policies add avoidable local friction. 2 does not fit because ordinary check changes still have a direct path through one small workflow and do not cross a complex control structure. + +### Domain model — 2, High confidence + +Some configuration identifiers no longer denote repository reality. Both push and pull-request triggers in `.github/workflows/rust.yml` refer to `openapi/**`, but that path does not exist; the actual API contract is `docs/public/api-reference/fabro-api.yaml`, which the same workflow's legacy-auth check names directly. `.github/workflows/typescript.yml` also omits that contract path even though the TypeScript API client is generated from it. A contract-only change can therefore fall outside the configured validation vocabulary. + +`.github/zizmor.yml:rules.stale-action-refs.ignore` identifies three exceptions by `rust.yml` source line. History shows those locations originally denoted Rust toolchain actions, while the current line numbers point elsewhere after workflow edits. The exception's identity is coupled to incidental layout rather than the action it is meant to describe. + +Strongest counterevidence: jobs, test modes, toolchain versions, permissions, and build profiles are otherwise named explicitly and line up with repository commands. + +Why adjacent scores do not fit: 3 does not fit because the stale/nonexistent identifiers affect whether central source-of-truth changes are validated and whether static-validation exceptions retain their intended meaning. 1 does not fit because most CI vocabulary remains stable and the affected values can be corrected from clear repository authorities. + +### Duplication of knowledge — 2, High confidence + +Trigger-path knowledge is repeated in every workflow and twice within each workflow: `.github/workflows/rust.yml:on.push.paths` duplicates `on.pull_request.paths`, and `.github/workflows/typescript.yml` does the same. Cross-language contract inputs then require synchronized edits in both files. The stale `openapi/**` entry and omission of `docs/public/api-reference/fabro-api.yaml` are direct evidence that this repeated knowledge has drifted. + +A representative routine change is moving or adding a source-of-truth file that must trigger all relevant CI. It requires updating `rust.yml:on.push.paths`, `rust.yml:on.pull_request.paths`, `typescript.yml:on.push.paths`, and `typescript.yml:on.pull_request.paths`; there is no shared authority that makes one update cover the four consumers. + +Strongest counterevidence: commands and action versions are local to their jobs, so much of the visible repetition is deliberate job isolation, and each language workflow is small. + +Why adjacent scores do not fit: 3 does not fit because trigger selection is central to CI's purpose, the synchronization crosses both event sections and language workflows, and actual drift is present. 1 does not fit because the duplicated lists are easy to locate and most entries still agree. + +Lens-boundary note: the stale OpenAPI trigger could be scored only as duplicate path knowledge. I used the repeated four-list maintenance burden for duplication, while treating the fact that `openapi/**` currently has no referent—and that line-based Zizmor identities no longer name the intended actions—as domain vocabulary drift. diff --git a/.chisel/calibration/work/reviewer-2.md b/.chisel/calibration/work/reviewer-2.md new file mode 100644 index 000000000..a9082f5c0 --- /dev/null +++ b/.chisel/calibration/work/reviewer-2.md @@ -0,0 +1,176 @@ +# Calibration Sample Review — Reviewer 2 + +Revision: `6bb6b5efcc0e36b52e3c097f532d9f2c00914c6c` + +This review uses the component boundaries in `.chisel/cartography/codebase-map.md`. In particular, `fabro-web-app` excludes `apps/fabro-web/app/components/playground/**`, and `repository-ci` contains only `.github/workflows/rust.yml`, `.github/workflows/typescript.yml`, and `.github/zizmor.yml`. + +## Score summary + +| Component | Ownership and boundaries | Simplicity | Domain model | Duplication of knowledge | +|---|---:|---:|---:|---:| +| `fabro-workflow` | 3 (Medium) | 2 (High) | 3 (Medium) | 2 (High) | +| `fabro-http` | 4 (High) | 4 (High) | 3 (High) | 4 (High) | +| `fabro-web-app` | 4 (Medium) | 2 (Medium) | 2 (Medium) | 2 (Medium) | +| `repository-ci` | 4 (High) | 3 (High) | 3 (High) | 2 (High) | + +## `fabro-workflow` + +### `ownership-boundaries` — 3, Medium confidence + +The component has a recognizable high-level owner and intended dependency direction. `lib/components/fabro-workflow/src/operations/mod.rs` owns run-level operations, while `lib/components/fabro-workflow/src/pipeline/mod.rs` owns the ordered phase API. `lib/components/fabro-workflow/src/pipeline/types.rs:Parsed`, `Transformed`, `Validated`, `Persisted`, `Initialized`, `Executed`, `Concluded`, and `Finalized` make phase ownership explicit. `lib/components/fabro-workflow/src/services.rs:RunServices` and `EngineServices` distinguish run-lifetime services from node-execution services, and `lib/components/fabro-workflow/src/node_handler.rs:WorkflowNodeHandler` is a visible adapter to `fabro-core`. + +The friction is at the public edge: `lib/components/fabro-workflow/src/lib.rs` exposes operations, pipeline phases, handlers, records, services, runtime storage, and several `#[doc(hidden)]` modules. Callers can therefore enter below the complete lifecycle as well as through `lib/components/fabro-workflow/src/operations/start.rs:start`. This weakens containment, but it does not create a competing production owner. + +**Strongest counterevidence:** The typed phase outputs and the `RunServices`/`EngineServices` split strongly reinforce one workflow lifecycle. + +**Why adjacent scores do not fit:** A 4 does not fit because the broad facade exposes enough lifecycle internals to make the boundary porous. A 2 does not fit because the normal `start` path and each phase owner remain identifiable and dependencies are delegated to dedicated crates. + +### `simplicity` — 2, High confidence + +The stable common path is traceable, but routine work crosses substantial central machinery: `lib/components/fabro-workflow/src/operations/start.rs:start` → `execute_persisted_run` → `RunSession::new` → `RunSession::run` → `pipeline::initialize` → `pipeline::execute` → `pipeline::finalize` → `pipeline::pull_request`. Along that path, `StartServices`, `RunSession`, and `lib/components/fabro-workflow/src/pipeline/types.rs:InitOptions` each carry many run concerns, while bootstrap, completion, cleanup, steering-drain, sandbox, and event-flush guards add multiple exit paths. `lib/components/fabro-workflow/src/pipeline/initialize.rs:initialize` also coordinates sandbox creation/reconnection, hooks, credentials, Git setup, handler construction, and resume state. + +**Representative routine change:** Adding one run-scoped execution service would normally thread through `operations/start.rs:StartServices`, `RunSession`, and `RunSession::new`; `pipeline/types.rs:InitOptions`; `pipeline/initialize.rs:initialize`; and `services.rs:RunServices` or `EngineServices`. + +**Strongest counterevidence:** `operations/start.rs:RunSession::run` presents the main phases in a linear order, and the phase-specific types preserve that order despite the setup machinery. + +**Why adjacent scores do not fit:** A 3 does not fit because the pressure is on the main run path rather than at an edge. A 1 does not fit because there is a stable phase sequence and named service bundles to follow. + +### `domain-model` — 3, Medium confidence + +The strongest mechanism is the phase-state model in `lib/components/fabro-workflow/src/pipeline/types.rs`; private fields on `Validated` and `Persisted` and opaque `ResumeState` prevent several invalid transitions. `lib/components/fabro-workflow/src/pipeline/finalize.rs:classify_engine_result` is also a clear authority for translating an engine result into `StageOutcome`, failure detail, and `RunStatus`. + +The main friction is the extensible, string-valued handler vocabulary on the common graph path. `lib/components/fabro-workflow/src/handler/mod.rs:HandlerRegistry::resolve` works with type strings and falls back to the default handler, while `default_registry` registers the built-in strings. Validation in `fabro-validate` protects normal runs, but execution itself does not carry a closed built-in handler type. + +**Strongest counterevidence:** `pipeline/types.rs:ResumeState::from_projection`, the phase output types, and `pipeline/finalize.rs:classify_engine_result` give important workflow concepts one enforced shape. + +**Why adjacent scores do not fit:** A 4 does not fit because handler identity remains string-valued and default-resolved through a central execution boundary. A 2 does not fit because validation and typed phase states canonicalize the normal run before execution. + +### `duplication-knowledge` — 2, High confidence + +Event knowledge is repeated across central authorities. `lib/components/fabro-workflow/src/event/events.rs:Event` defines the emitter-facing shape, `lib/components/fabro-workflow/src/event/convert.rs:event_body_from_event` translates it to the stored `fabro_types::EventBody`, `lib/components/fabro-workflow/src/event/names.rs:event_name` separately assigns wire names, and `lib/components/fabro-workflow/src/event/stored_fields.rs:stored_event_fields_for_variant` separately assigns envelope metadata. These exhaustive matches help detect omissions, but every ordinary event extension still requires synchronized semantic decisions. + +**Representative routine change:** Adding a stored workflow event can touch `event/events.rs:Event`, `event/convert.rs:event_body_from_event`, `event/names.rs:event_name`, `event/stored_fields.rs:stored_event_fields_for_variant`, and the canonical `lib/foundation/fabro-types/src/run_event/mod.rs:EventBody` authority. + +**Strongest counterevidence:** `event/convert.rs:to_run_event_at` is the single assembly point, and Rust's exhaustive matches turn many missed updates into compile failures. + +**Why adjacent scores do not fit:** A 3 does not fit because event emission and persistence are central, recurring behavior. A 1 does not fit because the authorities are explicit and compiler-checked rather than unidentifiable. + +## `fabro-http` + +### `ownership-boundaries` — 4, High confidence + +`lib/foundation/fabro-http/src/lib.rs` has one focused transport-construction boundary. `HttpClientBuilder`, `BlockingHttpClientBuilder`, `ProxyPolicy`, the client aliases, and the production/test constructors all live there; the crate depends only on `fabro-static`, `reqwest`, and `thiserror`. Repository policy reinforces the boundary through `clippy.toml:disallowed-methods`, which directs raw reqwest construction to this facade. + +**Strongest counterevidence:** The public reqwest aliases and re-exports make the abstraction intentionally permeable, so it does not own higher-level request behavior. + +**Why the adjacent score does not fit:** A 3 does not fit because exposing reqwest types is part of the mapped purpose, while construction policy and proxy resolution still have one clear owner. + +### `simplicity` — 4, High confidence + +`lib/foundation/fabro-http/src/lib.rs:define_builder` expresses shared async/blocking forwarding once. Both builders end at the same short `ProxyPolicy::resolve` and `build` path, and `http_client`, `test_http_client`, `blocking_http_client`, and `blocking_test_http_client` are thin named entry points. A shared reqwest builder option is normally added once to the macro. + +**Strongest counterevidence:** The macro hides generated methods, and async-only `HttpClientBuilder::read_timeout` must sit outside it. + +**Why the adjacent score does not fit:** A 3 does not fit because this indirection directly removes twin implementations and leaves callers with a single conventional builder path. + +### `domain-model` — 3, High confidence + +`lib/foundation/fabro-http/src/lib.rs:ProxyPolicy` gives the repository policy two named states, `ProxyPolicy::resolve_with_env_value` defines explicit-over-environment precedence, and `HttpClientBuildError::InvalidProxyPolicy` rejects unknown values. The tests cover default, environment, invalid, and explicit-override cases. + +The isolated ambiguity is that `HttpClientBuilder::no_proxy` and `HttpClientBuilder::proxy_policy(ProxyPolicy::Disabled)` both publicly express disabled proxy behavior, but `no_proxy` mutates the inner builder without updating the policy field. Their relationship is not represented or documented in the type. + +**Strongest counterevidence:** The closed enum, typed error, and resolver tests make the environment-facing policy meaning unusually explicit. + +**Why adjacent scores do not fit:** A 4 does not fit because two public controls overlap without an encoded relationship. A 2 does not fit because the overlap is local and every normal constructor still passes through one two-state resolver. + +### `duplication-knowledge` — 4, High confidence + +The builder macro is the authority for behavior shared by synchronous and asynchronous clients, and every constructor delegates to those builders. The production/test and async/blocking helper names repeat syntax, not policy: test behavior is expressed once as `ProxyPolicy::Disabled`. + +**Strongest counterevidence:** Four constructor helpers and the separate async-only impl are superficially repetitive. + +**Why the adjacent score does not fit:** A 3 does not fit because changing proxy precedence or disabled behavior has one authority; the remaining repetition does not require synchronized policy decisions. + +## `fabro-web-app` + +### `ownership-boundaries` — 4, Medium confidence + +The main browser lifecycle has clear homes. `apps/fabro-web/app/entry.tsx` selects install or normal routing and owns root providers; `app/router.tsx:routes` owns the product route graph; `app/install-router.tsx:installRoutes` owns first-run routing; `app/lib/api-client.ts` owns HTTP normalization; `app/lib/queries.ts` and `app/lib/mutations.ts` own shared server access; and `app/hooks/effects.ts` contains reusable browser-effect lifecycles. Route modules own page-specific composition. The separately mapped playground enters through `app/router.tsx` without its excluded implementation being absorbed into this assessment. + +**Strongest counterevidence:** `app/routes/run-stages.tsx` and `app/install-app.tsx` each combine page state, domain projection, and rendering in one route-owned file. + +**Why the adjacent score does not fit:** A 3 does not fit because those combinations create local complexity, but no competing owner or reversed dependency was identified; shared cross-route responsibilities still have clear modules. + +### `simplicity` — 2, Medium confidence + +Two common product paths carry central transformation machinery. `apps/fabro-web/app/routes/run-stages.tsx` turns event envelopes into `TurnType` values in `buildStageActivity`, then separately groups, filters, timelines, labels, summarizes, and renders them through `buildChatItems`, `groupConsecutiveTools`, `filterDisplayItems`, `buildThreadDnaItems`, and the route's view components. `apps/fabro-web/app/install-app.tsx` similarly contains the install reducer, session hydration, controller, step forms, review, finishing, payload construction, and supporting controls in one flow. + +**Representative routine change:** Changing how a tool event appears on the stage page requires tracing `run-stages.tsx:buildStageActivity`, `buildChatItems`/`groupConsecutiveTools`, `buildThreadDnaItems`, `turnLabel`, `turnSummary`, `EventDetails`, and `StageChatView`. + +**Strongest counterevidence:** The stage path uses discriminated unions and mostly pure exported transformations with focused tests, so each individual step can be reasoned about. + +**Why adjacent scores do not fit:** A 3 does not fit because the long transformation chains are central to major routes. A 1 does not fit because the named pure functions provide a stable trace through both flows. + +### `domain-model` — 2, Medium confidence + +Generated API types provide a useful boundary, but the central event path accepts several simultaneous shapes. `apps/fabro-web/app/lib/run-events.ts:RunEventPayload` makes event identity and metadata optional and `stageIdFromPayload` falls back from `stage_id` to `node_id` to `properties.node_id`. `app/routes/run-stages.tsx:activityEventStageId` repeats that shape tolerance for stored `EventEnvelope`s, while `buildStageActivity` reads tool, text, argument, and output values from both `properties` and legacy top-level fields via `app/lib/unknown.ts`. + +**Representative routine change:** Moving one stage-event field to its canonical envelope location can require coordinated interpretation changes in `lib/run-events.ts:RunEventPayload` and `stageIdFromPayload`, plus `routes/run-stages.tsx:activityEventStageId` and `buildStageActivity`. + +**Strongest counterevidence:** Once parsed, `run-stages.tsx:TurnType`, `StageRenderer`, and generated `StageHandler`/`StageState` types give the UI clear closed shapes. + +**Why adjacent scores do not fit:** A 3 does not fit because the multi-shape event interpretation is on live invalidation and the main stage view, not an edge. A 1 does not fit because generated types and discriminated UI projections establish a stable canonical shape after parsing. + +### `duplication-knowledge` — 2, Medium confidence + +Stage-state presentation policy is authoritative in several common views. `apps/fabro-web/app/lib/stage-sidebar.ts:ACTIVE_STAGE_STATES`, `IN_FLIGHT_STAGE_STATES`, `SUCCEEDED_STAGE_STATES`, `STAGE_STATUS_TONE`, and `STAGE_STATUS_LABEL` define classifications and visuals, while `app/components/stage-sidebar.tsx:statusConfig`, `app/components/run-waterfall.tsx:stageBarClass` and `isStageInFlight`, and `app/components/stage-popover.tsx:StatusPill` make parallel state decisions. + +**Representative routine change:** Adding a generated `StageState` requires reviewing or changing all of those authorities so the sidebar, waterfall, and popover agree on activity, success, label, and tone. + +**Strongest counterevidence:** Generated `StageState` plus exhaustive `Record` mappings catch many omissions, and `lib/stage-sidebar.ts` already centralizes several shared classifications. + +**Why adjacent scores do not fit:** A 3 does not fit because stage status is central to multiple routine run views and synchronization is recurring. A 1 does not fit because the generated enum is a clear semantic authority and TypeScript catches many missing cases. + +## `repository-ci` + +### `ownership-boundaries` — 4, High confidence + +The two workflows divide validation by ecosystem: `.github/workflows/rust.yml:jobs` owns Rust format, lint, generated-doc, workspace test, twin-mode ignored tests, and manual macOS validation; `.github/workflows/typescript.yml:jobs` owns web/client typecheck, web tests, and the embedded-SPA production build. Both use top-level empty permissions and job-local read permission. The cross-language Cargo build in the TypeScript build job validates the mapped embedded-SPA integration rather than creating a second build owner. + +**Strongest counterevidence:** The Rust clippy job contains a repository-wide legacy-auth guard that also scans TypeScript and API paths. + +**Why the adjacent score does not fit:** A 3 does not fit because that cross-language invariant remains an explicitly named CI check, while job and workflow lifecycle ownership stays clear. + +### `simplicity` — 3, High confidence + +The main flow is explicit: named jobs perform checkout, tool setup, and one or two direct repository commands. The isolated friction is `.github/workflows/rust.yml:jobs.clippy.steps.Verify legacy auth identity removal`, where a long regular expression and shell exit-status protocol are embedded in a lint job. The twin-mode test semantics also need a substantial comment and package expression in `jobs.test`. + +**Strongest counterevidence:** Separate jobs, direct commands, pinned tools, and no reusable-workflow indirection make routine CI behavior easy to locate. + +**Why adjacent scores do not fit:** A 4 does not fit because the legacy guard and twin-mode selection require non-obvious local interpretation. A 2 does not fit because that machinery is isolated and ordinary check changes still follow a direct job structure. + +### `domain-model` — 3, High confidence + +Job names, triggers, permissions, platforms, and commands have consistent meanings in the GitHub Actions structure. Exact action SHAs and named modes such as `--profile ci` reduce ambiguity. The main gap is that `.github/workflows/rust.yml:jobs.test` relies on the external default meaning of `FABRO_TEST_MODE` for its twin run rather than setting the mode in the workflow; the comment is the only local declaration of that state. + +**Strongest counterevidence:** The command, package selector, and explanation tightly describe the intended twin-only behavior, and every job has an explicit runner and permission set. + +**Why adjacent scores do not fit:** A 4 does not fit because a central test mode is implicit in an external default. A 2 does not fit because the rest of the workflow vocabulary is coherent and the implicit state is limited to one documented test step. + +### `duplication-knowledge` — 2, High confidence + +Trigger policy is repeated verbatim between `on.push.paths` and `on.pull_request.paths` in both workflow files. Action versions and bootstrap steps are also copied across every job. `.github/zizmor.yml:rules.stale-action-refs.ignore` adds line-number references to `rust.yml`, creating another manually synchronized representation; at this revision its listed lines 37, 49, and 62 are respectively a blank line, the `fmt` job key, and a Cargo command rather than action references. + +**Representative routine change:** Adding a new Rust-owned source area requires matching edits to `.github/workflows/rust.yml:on.push.paths` and `on.pull_request.paths`; upgrading checkout requires synchronized edits in `jobs.fmt`, `clippy`, `generated-docs`, `test`, and `test-macos`, followed by review of `.github/zizmor.yml:rules.stale-action-refs.ignore`. + +**Strongest counterevidence:** The duplication is explicit and small enough to inspect, and each actual validation command appears once in its intended job. + +**Why adjacent scores do not fit:** A 3 does not fit because triggers and action versions are central, recurring maintenance knowledge and the stale line selectors demonstrate drift. A 1 does not fit because the canonical workflows and intended checks remain identifiable. + +## Lens-boundary confusion + +- The `fabro-workflow` `Event`/`EventBody` split could be described as two domain shapes. I assigned its score effect to `duplication-knowledge` because the discriminating problem is the synchronized event name, conversion, and envelope-field decisions, not an inability to identify either type's meaning. +- The size and mixed contents of `fabro-web-app` route files could look like misplaced responsibility. I assigned the main effect to `simplicity` because the route remains the clear owner; the problem is tracing the amount of local machinery. +- Repeated `StageState` maps could be treated as domain drift. I assigned them to `duplication-knowledge` because the generated enum preserves meaning and the observed burden is repeating presentation/classification policy across views. +- The `.github/zizmor.yml` line selectors could be treated as invalid configuration meaning. I assigned their main effect to `duplication-knowledge` because the failure mechanism is manual synchronization with line positions; `repository-ci` domain scoring instead uses the implicit twin-mode default. +- `fabro-http`'s macro could be treated as simplicity indirection, while its two proxy-disable controls could be treated as duplicate policy. I treated the macro as a positive simplicity/duplication mechanism and the overlapping controls as `domain-model` friction because the unresolved question is what each public control means. diff --git a/.chisel/calibration/work/reviewer-3.md b/.chisel/calibration/work/reviewer-3.md new file mode 100644 index 000000000..98558bff4 --- /dev/null +++ b/.chisel/calibration/work/reviewer-3.md @@ -0,0 +1,490 @@ +# Calibration Sample Review — Reviewer 3 + +Revision: `6bb6b5efcc0e36b52e3c097f532d9f2c00914c6c` + +Scope follows `.chisel/cartography/codebase-map.md`: `fabro-workflow`, +`fabro-http`, `fabro-web-app`, and `repository-ci`. The `fabro-web-app` +reading excludes `apps/fabro-web/app/components/playground/**`; +`repository-ci` includes only `.github/workflows/rust.yml`, +`.github/workflows/typescript.yml`, and `.github/zizmor.yml`. + +## Provisional Matrix + +| Component | Ownership and boundaries | Simplicity | Domain model | Duplication of knowledge | +|---|---:|---:|---:|---:| +| `fabro-workflow` | 4 / High | 2 / High | 2 / High | 2 / High | +| `fabro-http` | 4 / High | 4 / High | 4 / High | 4 / High | +| `fabro-web-app` | 3 / High | 2 / High | 2 / High | 2 / High | +| `repository-ci` | 3 / High | 3 / High | 2 / High | 2 / High | + +## `fabro-workflow` + +### `ownership-boundaries` — 4, High confidence + +Evidence: + +- `lib/components/fabro-workflow/src/pipeline/mod.rs` exposes an ordered phase + facade, while `pipeline/types.rs:Parsed`, `Transformed`, `Validated`, + `Persisted`, `Initialized`, `Executed`, `Concluded`, and `Finalized` give each + phase an explicit handoff. +- `lib/components/fabro-workflow/src/handler/mod.rs:Handler` and + `HandlerRegistry` own workflow-specific dispatch; + `src/node_handler.rs:WorkflowNodeHandler` is the narrow adapter to + `fabro_core::handler::NodeHandler`. +- `lib/components/fabro-workflow/src/lifecycle/mod.rs:WorkflowLifecycle` states + that it owns callback ordering and delegates event, hook, fidelity, + auto-status, circuit-breaker, Git, and artifact work to focused lifecycle + objects. +- `lib/components/fabro-workflow/Cargo.toml:[dependencies]` points from the + orchestrator to parsing, validation, sandbox, persistence, model, and generic + execution crates; generic traversal remains in `fabro-core`. + +Strongest counterevidence: startup state is carried through +`operations/start.rs:StartServices`, `RunSession`, +`pipeline/types.rs:InitOptions`, and `services.rs:RunServices` / +`EngineServices`, so the lifecycle boundary has substantial wiring. + +Why adjacent scores do not fit: 3 would treat that wiring as unclear ownership, +but the common path consistently identifies phase, handler, lifecycle, and +generic-executor owners. The counterevidence is primarily machinery inside the +intended orchestration owner, not a competing dependency direction or lifecycle +home. + +### `simplicity` — 2, High confidence + +Evidence: + +- The normal start path crosses + `operations/start.rs:start` → `execute_persisted_run` → + `RunSession::new` → `RunSession::run` → + `pipeline::initialize` → `pipeline::execute` → + `pipeline::finalize` → `pipeline::pull_request`. +- The same run-scoped collaborators are reshaped across + `operations/start.rs:StartServices`, `RunSession`, + `pipeline/types.rs:InitOptions`, `services.rs:RunServices`, and + `EngineServices`. +- `lifecycle/mod.rs:WorkflowLifecycle::new` takes the full set of lifecycle + collaborators and has an explicit `too_many_arguments` exception before + constructing seven sub-lifecycles with shared coordination state. + +Strongest counterevidence: the phase-state types in +`pipeline/types.rs` and the focused handler/lifecycle modules make this +machinery traceable; the common path is not hidden. + +Why adjacent scores do not fit: 3 does not fit because every ordinary run +traverses the service reshaping and multi-stage cleanup/finalization path; this +is central rather than edge friction. 1 does not fit because the named phase +sequence and handoff types provide a stable path through the machinery. + +Representative routine change: adding a run-scoped execution-audit sink for +handlers would require threading it through +`operations/start.rs:StartServices`, `RunSession`, +`RunSession::new`, `RunSession::run`, +`pipeline/types.rs:InitOptions`, `pipeline/initialize.rs:initialize`, and +`services.rs:RunServices` or `EngineServices`. + +### `domain-model` — 2, High confidence + +Evidence: + +- Positive mechanisms are substantial: + `pipeline/types.rs:Validated` hides its graph and exposes validation + operations, `ResumeState::from_projection` creates opaque resume state, and + `run_status.rs` plus `outcome.rs` reuse canonical types from `fabro-types` and + `fabro-core`. +- A central exception remains: + `event/events.rs:Event::StageCompleted` represents `status` as `String`, while + execution uses typed `outcome.rs:StageOutcome`. + `event/convert.rs:stage_status_from_string` reparses the string and maps every + unknown value to a failed outcome. +- The common producer + `lifecycle/event.rs:EventLifecycle::after_node` converts the typed outcome to + a string before the canonical event conversion converts it back. + +Strongest counterevidence: the pipeline phase types, `RunStatus`, +`StageOutcome`, `StageId`, and the durable `fabro_types::EventBody` otherwise +give the main workflow concepts canonical typed shapes. + +Why adjacent scores do not fit: 3 does not fit because stage completion is on +the execution hot path and accepts states the canonical outcome enum rejects. +1 does not fit because the canonical types and phase states still give the +workflow a coherent vocabulary overall. + +Representative routine change: adding or changing a stage outcome would touch +the canonical `lib/foundation/fabro-core/src/outcome.rs:StageOutcome`, string +construction in `lifecycle/event.rs:EventLifecycle::after_node`, +`event/events.rs:Event::StageCompleted`, +`event/convert.rs:stage_status_from_string`, and terminal interpretation in +`pipeline/finalize.rs:classify_engine_result`. + +### `duplication-knowledge` — 2, High confidence + +Evidence: + +- `event/events.rs:Event` defines the internal event shape, + `event/names.rs:event_name` independently maps every variant to its external + name, `event/stored_fields.rs:stored_event_fields` independently selects + envelope fields, and `event/convert.rs:event_body_from_event` constructs the + canonical `fabro_types::EventBody`. +- `docs/internal/events-strategy.md:Adding A New Event` explicitly requires + synchronized edits to the internal event, tracing, external name, + `EventBody`, stored fields, conversion, and consumers. +- Exhaustive matches make omissions visible, but they do not make one of those + mappings authoritative for the others. + +Strongest counterevidence: `event/emitter.rs:Emitter` canonicalizes each emitted +event once, all listeners receive the same `RunEvent`, and exhaustive matching +plus conversion tests detect much of the synchronization drift. + +Why adjacent scores do not fit: 3 does not fit because adding an event is a +routine extension to this component and centrally requires several independent +authorities. 1 does not fit because the events strategy clearly identifies all +authorities and the compiler/test suite gives a stable update path. + +Representative routine change: adding `run.suspended` would touch +`event/events.rs:Event`, `events.rs:Event::trace`, +`event/names.rs:event_name`, +`lib/foundation/fabro-types/src/run_event/mod.rs:EventBody`, +`event/stored_fields.rs:stored_event_fields`, +`event/convert.rs:event_body_from_event`, and relevant store/UI consumers. + +## `fabro-http` + +### `ownership-boundaries` — 4, High confidence + +Evidence: + +- The component is one focused source module: + `lib/foundation/fabro-http/src/lib.rs` owns the reqwest facade, + `ProxyPolicy`, client builders, build errors, and deterministic test clients. +- `src/lib.rs:HttpClientBuilder::build` and + `BlockingHttpClientBuilder::build` are the construction boundary where the + process proxy policy is applied. +- `clippy.toml:disallowed-methods` denies direct reqwest client constructors and + points callers to this component; `fabro_static::EnvVars` supplies the one + environment-variable name without introducing higher-level configuration. + +Strongest counterevidence: the facade deliberately re-exports many reqwest +types, and exceptional consumers still carry direct reqwest dependencies for +generated clients or incompatible dependency versions. + +Why adjacent scores do not fit: 3 does not fit because the normal async, +blocking, production, and test construction paths all converge on the same +owned policy, with a repository lint reinforcing that boundary. + +### `simplicity` — 4, High confidence + +Evidence: + +- `src/lib.rs:define_builder!` expresses the common async/blocking builder once; + the four convenience constructors are thin calls to the same builders. +- The common flow is direct: + `HttpClientBuilder::new` → optional reqwest options → + `HttpClientBuilder::build` → `ProxyPolicy::resolve` → reqwest build. +- The only async-only option is visibly isolated in + `HttpClientBuilder::read_timeout`. + +Strongest counterevidence: the macro hides the two generated impls and every +new exposed reqwest option requires another forwarding method. + +Why adjacent scores do not fit: 3 does not fit because the macro removes a real +parallel API synchronization burden while leaving the common client-building +path locally readable; its indirection is not encountered beyond this file. + +### `domain-model` — 4, High confidence + +Evidence: + +- `src/lib.rs:ProxyPolicy` has exactly the two supported states, + `ProxyPolicy::resolve_with_env_value` makes explicit configuration override + environment fallback, and invalid/non-Unicode values become + `HttpClientBuildError`. +- `src/lib.rs:HttpClientBuildError` distinguishes invalid policy from underlying + reqwest construction failure. +- `test_http_client` and `blocking_test_http_client` select the typed + `ProxyPolicy::Disabled` rather than relying on ambient test environment state. + +Strongest counterevidence: the environment boundary is necessarily stringly, +and `ProxyPolicy::parse` accepts case variants before producing the enum. + +Why adjacent scores do not fit: 3 does not fit because invalid strings are +rejected at the boundary, precedence is explicit, and all downstream paths use +the closed enum. + +### `duplication-knowledge` — 4, High confidence + +Evidence: + +- `src/lib.rs:define_builder!` is the single authority for shared async and + blocking options and policy application. +- `ProxyPolicy::resolve` is the single production authority for explicit/env/ + default precedence. +- `clippy.toml:disallowed-methods` prevents ordinary callers from silently + recreating client-construction policy outside the component. + +Strongest counterevidence: async and blocking convenience constructors remain +as four syntactically similar functions, and `read_timeout` cannot live in the +shared macro surface. + +Why adjacent scores do not fit: 3 does not fit because the remaining repetition +does not duplicate a policy or require independent decisions; it exposes +parallel entry points backed by the same authority. + +## `fabro-web-app` + +### `ownership-boundaries` — 3, High confidence + +Evidence: + +- `apps/fabro-web/app/entry.tsx:AppRuntime` owns browser bootstrap and global + runtime providers; `router.tsx:routes` and + `install-router.tsx:installRoutes` own the two route graphs. +- `app/lib/queries.ts` and `app/lib/mutations.ts` own server reads and writes; + `app/lib/api-client.ts` owns transport/error normalization. +- `app/hooks/effects.ts` and purpose-named hooks such as + `useRunEvents` and `useInstallRestartHealthPolling` contain browser resource + lifecycles rather than leaving them in route rendering. +- `routes/run-detail.tsx:RunDetail` delegates its header, actions, model, + lifecycle-toast, tab-shell, and docked-control responsibilities to the + `routes/run-detail/**` modules. + +Strongest counterevidence: two mapped common paths still concentrate several +responsibilities: +`install-app.tsx:InstallApp` / `useInstallController` contains state, +hydration, submission, step routing, payload construction, and rendering, while +`routes/run-stages.tsx:RunStages` / `buildStageActivity` contains event +interpretation and a large part of stage presentation. + +Why adjacent scores do not fit: 4 does not fit because those central route +modules are not merely edge exceptions. 2 does not fit because routes, API +access, queries, mutations, browser effects, and build lifecycle still have +stable homes and dependencies generally point through those homes. + +### `simplicity` — 2, High confidence + +Evidence: + +- The first-run common path is concentrated in + `install-app.tsx:installReducer`, `useInstallController`, `InstallApp`, + `LlmStep`, `ObjectStoreStep`, `SandboxStep`, `GithubStep`, + `buildObjectStorePayload`, and `buildSandboxPayload`. +- The run-stage common path combines + `routes/run-stages.tsx:selectStageRenderer`, + `buildStageActivity`, filtering, debug views, waterfall construction, and + `RunStages`. +- Cross-tab event sharing introduces a second substantial state machine at + `app/lib/cross-tab-sse.ts:CrossTabSseCoordinator`, beneath the already + separate shared-event-source logic in `app/lib/sse.ts:subscribeToSharedEventSource`. + +Strongest counterevidence: reducers, discriminated unions, shared query hooks, +purpose-named integration hooks, and extracted run-detail modules make many +individual flows explicit and testable. + +Why adjacent scores do not fit: 3 does not fit because installation, run-stage +inspection, and live refresh are mapped common paths, not optional edge +machinery. 1 does not fit because each path still has identifiable entry +points, state machines, and tests. + +Representative routine change: adding an installation step for telemetry would +touch `install-app.tsx:INSTALL_STEPS`, `InstallState`, `InstallAction`, +`installReducer`, `useInstallController`, `InstallApp`, a new step component, +review-summary helpers, `install-api.ts`, and the generated install API +authority in `docs/public/api-reference/fabro-api.yaml`. + +### `domain-model` — 2, High confidence + +Evidence: + +- Positive mechanisms include generated API types throughout the query and + route layers, `mode.ts:FabroMode`, and exhaustive display maps such as + `lib/sandbox-state.ts:SANDBOX_STATE_DISPLAY`. +- The central SSE boundary instead uses + `lib/sse.ts:EventPayload`, where `event` is optional and all other fields are + unknown, then extends it as + `lib/run-events.ts:RunEventPayload` with optional string identifiers and + another untyped `properties` map. +- `lib/run-events.ts:stageIdFromPayload` accepts `stage_id`, `node_id`, or + `properties.node_id` as the stage identity. +- `lib/run-sandbox-lifecycle.ts:sandboxLifecycleKind` and `sandboxInstance` + cast generated values into compatibility shapes and infer lifecycle from + either `kind`, `instance`, or legacy `runtime` / `provider` fields. + +Strongest counterevidence: normal HTTP reads and writes use +`@qltysh/fabro-api-client` types, and `Record` display maps +make many API vocabulary changes compile-visible. + +Why adjacent scores do not fit: 3 does not fit because SSE drives normal run +refresh and stage views while permitting absent event and identity fields with +multiple meanings. 1 does not fit because generated HTTP types and local +discriminated unions still provide a coherent model for most operations. + +Representative routine change: making stage identity canonical across live +events would touch the wire authority +`docs/public/api-reference/fabro-api.yaml`, +`lib/sse.ts:EventPayload`, `lib/run-events.ts:RunEventPayload`, +`stageIdFromPayload`, and consumers such as +`routes/run-stages.tsx:buildStageActivity`. + +### `duplication-knowledge` — 2, High confidence + +Evidence: + +- `lib/board-events.ts:BOARD_STATUS_EVENTS` independently decides which run + events refresh lists, while `lib/run-events.ts:RUN_SUMMARY_EVENTS`, + `TERMINAL_EVENTS`, and other sets decide detail invalidations. +- `lib/run-phases.ts:deriveRunPhases` independently matches the same lifecycle + event vocabulary to build the pre-stage timeline. +- `lib/run-events.ts:STAGE_ACTIVITY_EVENT_TYPES` is a positive local authority + shared with `routes/run-stages.tsx:buildStageActivity`, but it covers only one + slice of the broader manual event policy. + +Strongest counterevidence: list and detail invalidation are genuinely different +consumer decisions, `query-keys.ts:queryKeys` centralizes cache identities, and +the stage-activity list is deliberately shared with its reducer. + +Why adjacent scores do not fit: 3 does not fit because a normal lifecycle-event +extension that affects board and run detail requires synchronized policy edits +in separate common subscriptions. 1 does not fit because each consumer's +authority is named, localized, and covered by focused tests. + +Representative routine change: adding a `run.suspended` transition that should +refresh both list and detail views would touch +`board-events.ts:BOARD_STATUS_EVENTS`, +`run-events.ts:RUN_SUMMARY_EVENTS` (and possibly `TERMINAL_EVENTS` if its +semantics require it), `board-events.test.tsx`, `run-events.test.tsx`, and the +upstream event/OpenAPI authorities. + +## `repository-ci` + +### `ownership-boundaries` — 3, High confidence + +Evidence: + +- `.github/workflows/rust.yml:jobs` owns Rust formatting, lint, generated-doc, + Linux test, twin-E2E, and manual macOS validation. +- `.github/workflows/typescript.yml:jobs` owns browser/client typecheck, web + tests, and the embedded-SPA release build. +- Both workflows set top-level empty permissions and grant only + `contents: read` per job; all third-party actions are commit-pinned. +- Generated-document and embedded-SPA behavior is delegated to + `cargo dev docs check` and `cargo dev build`, leaving those build procedures + in `fabro-build-tooling`. + +Strongest counterevidence: +`.github/workflows/rust.yml:jobs.clippy.steps[name="Verify legacy auth identity removal"]` +contains an authentication-migration vocabulary grep inside the general CI +workflow, so an auth-domain transition also has a policy home here. + +Why adjacent scores do not fit: 4 does not fit because that product-domain +policy crosses into the CI owner and the trigger boundary has drift discussed +under domain model. 2 does not fit because the normal validation jobs and their +delegated build/test authorities remain clearly owned and directional. + +Representative routine change: renaming or restoring an authentication identity +would require changing the product types and also the legacy-name authority in +`.github/workflows/rust.yml:jobs.clippy.steps[name="Verify legacy auth identity removal"]`. + +### `simplicity` — 3, High confidence + +Evidence: + +- Each job is a short checkout/setup/command sequence, and the two workflows + split by the repository's Rust and Bun validation surfaces. +- `.github/workflows/rust.yml:jobs.test` explains the non-obvious twin-mode + expression and why it must not use the strict E2E profile. +- `.github/workflows/typescript.yml:jobs.build` delegates the mixed Rust/SPA + build to one repository command rather than reproducing its internals. + +Strongest counterevidence: checkout, tool setup, install, permissions, runner, +and cache declarations are repeated across every job; the inline legacy-auth +shell condition is more elaborate than the surrounding declarative checks. + +Why adjacent scores do not fit: 4 does not fit because routine maintenance must +scan repeated job scaffolding and one bespoke shell policy. 2 does not fit +because a contributor can still trace each common validation path directly +from one named job to one repository command. + +### `domain-model` — 2, High confidence + +Evidence: + +- `.github/workflows/rust.yml:on.push.paths` and `on.pull_request.paths` contain + `openapi/**`, but that directory does not exist at the assessed revision. +- The actual contract authority is + `docs/public/api-reference/fabro-api.yaml`, as named by + `AGENTS.md:API workflow`, + `lib/foundation/fabro-api/build.rs:main`, and + `lib/packages/fabro-api-client/package.json:scripts.generate`. +- Neither `.github/workflows/rust.yml:on.*.paths` nor + `.github/workflows/typescript.yml:on.*.paths` names that actual contract + path, even though both generated clients depend on it. + +Strongest counterevidence: job names, Rust versus TypeScript scope, twin versus +live test meaning, and toolchain versions are otherwise explicit; the commands +the jobs run correspond to checked-in project commands. + +Why adjacent scores do not fit: 3 does not fit because an ordinary edit to the +HTTP source of truth falls outside both central validation trigger models. 1 +does not fit because the workflows still have a stable and mostly accurate +vocabulary for jobs, branches, tools, and commands. + +Representative routine change: editing only +`docs/public/api-reference/fabro-api.yaml` should exercise Rust generation and +TypeScript typecheck/build, but its meaning would have to be repaired in +`.github/workflows/rust.yml:on.push.paths`, +`.github/workflows/rust.yml:on.pull_request.paths`, +`.github/workflows/typescript.yml:on.push.paths`, and +`.github/workflows/typescript.yml:on.pull_request.paths`. + +### `duplication-knowledge` — 2, High confidence + +Evidence: + +- Each workflow repeats its path set under both `on.push.paths` and + `on.pull_request.paths`; a new CI-relevant repository path has two authorities + per language. +- `.github/workflows/rust.yml:jobs.fmt`, `jobs.clippy`, + `jobs.generated-docs`, `jobs.test`, and `jobs.test-macos` independently repeat + checkout pins, credential policy, runner/toolchain setup, and often cache + setup. +- `.github/workflows/typescript.yml:jobs.typecheck`, `jobs.test`, and + `jobs.build` independently repeat checkout, Bun setup, and frozen install. + +Strongest counterevidence: independent jobs preserve failure isolation and +least-privilege permissions, while the substantive docs/build procedures are +delegated to repository commands rather than copied into YAML. + +Why adjacent scores do not fit: 3 does not fit because path and tool-bootstrap +knowledge is repeated on every routine trigger or tool-version update. 1 does +not fit because all copies remain confined to two small workflow files and the +substantive check authorities are still identifiable. + +Representative routine change: adding a new Rust-relevant `tools/**` tree would +require synchronized edits to +`.github/workflows/rust.yml:on.push.paths` and +`on.pull_request.paths`; updating the Rust checkout/toolchain baseline requires +reviewing the pins in every `rust.yml:jobs.*.steps` copy. + +## Lens-Boundary Notes + +- The repeated startup carriers in `fabro-workflow` could be labeled ownership + or simplicity. I counted their unclear amount of machinery under simplicity; + ownership was judged from whether each phase, resource lifecycle, and + dependency direction has a named home. +- The workflow's internal `Event` and durable `EventBody` have documented + distinct meanings. I therefore counted the many synchronized mappings under + duplication, not domain model. The separate `StageCompleted.status: String` + finding drives the domain-model score because it admits invalid states. +- Large web route files are not ownership findings merely because they are + large. They lower simplicity where common behavior is difficult to trace; the + ownership score moves only where several responsibilities remain concentrated + despite otherwise clear route/data/effect homes. +- In the web event layer, optional/untyped payload shape is a domain-model + finding. Repeating lifecycle-event policy across list, detail, and phase + consumers is a duplication finding. +- In CI, the stale `openapi/**` referent is a domain-model finding because the + path no longer means the API authority it purports to cover. Repeating trigger + and setup lists is separately a duplication finding. +- The `fabro-http` builder macro adds local indirection, but its primary effect + is to make shared async/blocking policy authoritative. I treated it as a + positive duplication mechanism rather than simplicity friction. diff --git a/.chisel/calibration/work/validation-1.md b/.chisel/calibration/work/validation-1.md new file mode 100644 index 000000000..6954086ae --- /dev/null +++ b/.chisel/calibration/work/validation-1.md @@ -0,0 +1,424 @@ +# Chisel calibration validation 1 + +Revision: `6bb6b5efcc0e36b52e3c097f532d9f2c00914c6c` + +This is an independent reading of only the requested assignments. Scores use the +mapped purposes and the final calibration rubric. Boundary evidence is included +where it establishes whether a scoped mechanism is on a production common path. + +## Summary + +| Component | Lens | Score | Evidence confidence | +|---|---|---:|---| +| `fabro-workflow` | `ownership-boundaries` | 2 | High | +| `fabro-workflow` | `domain-model` | 2 | High | +| `fabro-http` | `domain-model` | 4 | High | +| `fabro-http` | `duplication-knowledge` | 4 | High | +| `fabro-web-app` | `ownership-boundaries` | 4 | Medium | +| `repository-ci` | `ownership-boundaries` | 4 | Medium | +| `repository-ci` | `domain-model` | 2 | High | +| `fabro-checkpoint` | `ownership-boundaries` | 2 | High | +| `fabro-checkpoint` | `simplicity` | 4 | Medium | +| `fabro-checkpoint` | `domain-model` | 2 | High | +| `fabro-checkpoint` | `duplication-knowledge` | 3 | Medium | + +## `fabro-workflow` + +### `ownership-boundaries`: 2 + +- **Evidence:** `lifecycle/mod.rs:53-80` presents `WorkflowLifecycle` as the + callback owner, and `lifecycle/git.rs:77-93, 397-401` gives `GitLifecycle` + its own `last_git_sha` state. The normal `RunSession::run` path nevertheless + creates a second `last_git_sha`, reconstructs it by listening to emitted + checkpoint, terminal, and Git events, then passes it back into finalization + (`operations/start.rs:821-856, 914-923`). Terminal responsibility is split + again: engine outcomes become terminal events in + `pipeline/finalize.rs:524-596`, while bootstrap, initialization, and + finalization errors become `run.failed` through the outer operation in + `operations/start.rs:176-285, 288-346`. These crossings occur on the normal + run and error paths, not at an optional edge. +- **Strongest counterevidence:** `operations/start.rs:796-953` is a recognizable + top-level owner for the initialize → execute → finalize → pull-request + sequence, and `WorkflowLifecycle` explicitly orders focused delegates for + each executor callback (`lifecycle/mod.rs:221-469`). +- **Why adjacent scores do not fit:** 3 does not fit because the caller always + mirrors and resupplies Git identity on the common run path, and terminal + failure handling routinely selects between two owners. 1 does not fit because + both the executor callback owner and the outer run-session owner are stable + and traceable; the problem is their competition, not the absence of owners. +- **Rule discrimination:** Decision rule 2 is decisive for the mirrored + `last_git_sha`. The phrase “complete lifecycle” is otherwise ambiguous about + whether an executor lifecycle may end before durability finalization; the + explicit state round-trip makes the result 2 without relying on that + ambiguity. + +### `domain-model`: 2 + +- **Evidence:** The internal durable event shape stores + `Event::StageCompleted.status` as `String` + (`event/events.rs:264-272`). Both synthetic terminal-stage completion and + ordinary successful stage completion stringify the canonical + `StageOutcome` (`lifecycle/event.rs:215-240, 355-366`), after which the + mandatory event conversion reparses it and converts an unknown value to + `Failed` (`event/convert.rs:14-24, 309-333`). This typed → string → typed path + is part of every successful stage-completion event. +- **Strongest counterevidence:** `fabro_types::StageOutcome` is a stable + canonical type, most event fields are typed, and the fallback prevents an + unrecognized string from escaping into the stored projection. +- **Why adjacent scores do not fit:** 3 does not fit because common production + completion events depend on the invalid intermediate rather than using it as + a compatibility edge. 1 does not fit because the canonical status meaning is + clear and the conversion point is explicit. +- **Rule discrimination:** Decision rule 4 and the rubric's repository example + make this assignment unambiguous. + +## `fabro-http` + +### `domain-model`: 4 + +- **Evidence:** `ProxyPolicy` is a closed `System | Disabled` vocabulary; + parsing rejects every other boundary value + (`src/lib.rs:23-35`). Resolution gives explicit configuration precedence over + the environment, defaults absence to `System`, and rejects non-Unicode input + (`src/lib.rs:38-60`). Every async and blocking builder reaches that resolver + before construction (`src/lib.rs:160-166, 172-193`), while the deterministic + test helpers select the typed `Disabled` value + (`src/lib.rs:195-213`). +- **Strongest counterevidence:** The builder also exposes raw `no_proxy()` and + `proxy()` operations (`src/lib.rs:96-106`), so callers can combine an + underlying reqwest choice with `ProxyPolicy`; Unix-socket production callers + do use `no_proxy()` (`lib/foundation/fabro-client/src/client.rs:2123-2134`). +- **Why adjacent scores do not fit:** 3 does not fit because the common + policy-controlled constructors never interpret an invalid policy: they + return `HttpClientBuildError`. The raw builder operations represent valid + per-client transport configuration, not a second string vocabulary. 2 and 1 + do not fit because no common-path conversion or unstable meaning is present. +- **Rule discrimination:** Decision rule 4 is potentially non-discriminating + if every forwarded low-level builder method is called an “escape hatch.” + Here `no_proxy()` carries no invalid intermediate and does not weaken + `ProxyPolicy::resolve`, so treating it as ordinary typed builder + configuration preserves the rule's distinction. + +### `duplication-knowledge`: 4 + +- **Evidence:** `define_builder!` holds the complete shared async/blocking + builder policy once, including proxy resolution and construction + (`src/lib.rs:72-170`), and is instantiated for the two reqwest client kinds + (`src/lib.rs:172-193`). The four convenience constructors delegate to those + builders rather than reproducing policy (`src/lib.rs:195-213`). +- **Strongest counterevidence:** The generated facade necessarily lists each + forwarded reqwest method, and the test and non-test convenience constructors + have similar bodies. +- **Why adjacent scores do not fit:** 3 does not fit because the similar + forwarding and wrappers are syntax over one policy authority, not separately + maintained transport knowledge. 2 does not fit because a proxy-policy change + is made once in the macro/resolver, not synchronized across async and + blocking implementations. 1 does not fit because the authority is explicit. +- **Rule discrimination:** The rubric's `define_builder!` example directly + distinguishes shared macro expansion from semantic duplication; no material + ambiguity remains. + +## `fabro-web-app` + +### `ownership-boundaries`: 4 + +- **Evidence:** `entry.tsx:17-49` owns browser startup, chooses the normal or + installation route graph once, and installs shared SWR runtime policy. + `router.tsx:97-184` owns normal route composition. Shared transport and error + handling live in `lib/api-client.ts:64-160, 213-310`; shared reads such as + `useRun` and `useRunState` live in `lib/queries.ts:182-193`; run mutations and + their cache lifecycle live in `lib/mutations.ts:65-132`; and run-scoped SSE + subscription, invalidation, resync, and cleanup live in + `lib/run-events.ts:129-309`. The representative busy route composes those + owners rather than reimplementing them + (`routes/run-detail.tsx:79-145, 313-379`). +- **Strongest counterevidence:** Some route-local CRUD actions call the shared + API facade directly, and `run-detail.tsx:193-205` coordinates delete state, + cache invalidation, toast, and navigation in the route. +- **Why adjacent scores do not fit:** 3 does not fit because the counterevidence + is local page UX ownership; it does not split a shared transport, read, + mutation, or subscription lifecycle. 2 does not fit because routine run-page + changes use the established owners rather than coordinating competing ones. + 1 does not fit because startup, routing, transport, caching, and streaming + each have readily identifiable homes. +- **Rule discrimination:** “One owner” is mildly non-discriminating for a large + browser application unless responsibility is evaluated at lifecycle + granularity. Using the rubric's `apiData`/`useRun` example, route composition + is not itself a second owner. Confidence is Medium because this is the + largest sampled scope. + +## `repository-ci` + +### `ownership-boundaries`: 4 + +- **Evidence:** `rust.yml:3-40` owns Rust branch/PR/manual triggers and + concurrency, while its jobs contain format, lint, generated-doc, Linux test, + twin E2E, and manual macOS lifecycles (`rust.yml:48-147`). + `typescript.yml:3-34` owns the corresponding TypeScript triggers and + concurrency, and its jobs contain typecheck, test, and integrated SPA/Rust + build lifecycles (`typescript.yml:36-77`). Delegation to `cargo dev` is the + mapped dependency on build tooling, not reverse ownership. +- **Strongest counterevidence:** The TypeScript build invokes a Rust build + (`typescript.yml:75-77`), and invalid path selectors mean some intended + changes do not start the declared workflows. +- **Why adjacent scores do not fit:** 3 does not fit because the cross-language + build is the intentional embedded-SPA integration boundary, not friction, and + selector validity is classified under domain model by decision rule 6. 2 + does not fit because no routine job requires coordination between competing + CI owners. 1 does not fit because the two language validation homes and their + dependency direction are explicit. +- **Rule discrimination:** The score-4 phrase “complete lifecycle” is + non-discriminating for hosted CI if it is read to require repository + ownership of GitHub's runner lifecycle. This score treats the checked-in + trigger/job lifecycle as the mapped responsibility and the platform as an + intended boundary. + +### `domain-model`: 2 + +- **Evidence:** Both Rust trigger selectors name `openapi/**` + (`rust.yml:18,34`), but that revision has no tracked target there; the actual + API contract is `docs/public/api-reference/fabro-api.yaml`, which the + TypeScript client generation command consumes + (`lib/packages/fabro-api-client/package.json:7`). The real contract path is + absent from both workflow path filters. In addition, all three zizmor + `stale-action-refs` identifiers target `rust.yml:37`, `:49`, and `:62` + (`zizmor.yml:1-6`), which are respectively the end of trigger setup, the + `fmt` job key, and a `run` command—not action references at this revision. + These invalid identifiers sit directly in trigger and static-validation + configuration. +- **Strongest counterevidence:** The workflow/job vocabulary itself is stable, + all jobs and action pins have clear meanings, and changes under the large + valid Rust and TypeScript source selectors do trigger their expected suites. +- **Why adjacent scores do not fit:** 3 does not fit because the dead OpenAPI + selector is present in both routine branch and PR paths, while every scoped + zizmor exception lacks a current target. 1 does not fit because the overall + workflow and job model remains stable; the defect is a recurring set of + invalid identifiers. +- **Rule discrimination:** Decision rule 6 is decisive that these are domain + pressure rather than ownership or duplication. It does not state when one or + more dead selectors move from 3 to 2; centrality in both trigger modes and + total staleness of the scoped zizmor selectors supply that discrimination + here. + +## Control: `fabro-checkpoint` + +### `ownership-boundaries`: 2 + +- **Evidence:** The mapped component claims metadata branches, but its + production boundary consumer owns the metadata writer's branch, parent OID, + discovery, remote, and push lifecycle + (`fabro-workflow/src/run_metadata.rs:272-282, 313-439`). On every snapshot, + that caller validates entries, individually drives `Store` through blobs, + tree, commit, and ref update, and retains the parent identity for the next + write (`run_metadata.rs:313-350`). `BranchStore` provides a contained + read-modify-write owner (`branch.rs:17-24, 42-81`) but has no production + caller at this revision. +- **Strongest counterevidence:** The dependency direction is intended + (`fabro-workflow` depends on `fabro-checkpoint`), and the low-level `Store` + consistently owns Git object/ref operations (`git.rs:101-227`). +- **Why adjacent scores do not fit:** 3 does not fit because the lifecycle + crossing occurs on every metadata snapshot, not in an isolated adapter. 1 + does not fit because low-level Git ownership and the caller's higher-level + writer ownership are both stable; the problem is the split between them. +- **Rule discrimination:** Decision rule 2 applies because the caller retains + and resupplies branch/parent identity to complete successive writes. The + rubric does not say whether a deliberately low-level `Store` narrows the + mapped ownership claim; the explicit mapped claim to metadata branches makes + this crossing discriminating. + +### `simplicity`: 4 + +- **Evidence:** The production `Store` has direct blob, tree, commit, and ref + operations (`git.rs:123-226`). Tree conversion is a single read recursion and + a single bottom-up write path (`git.rs:229-310`). At the higher level, + `BranchStore::write_with` is a linear resolve → read → mutate → write → commit + → update sequence (`branch.rs:56-81`), and entry operations are small + delegates (`branch.rs:84-117`). Necessary Git layering is visible rather than + hidden behind competing configuration machinery. +- **Strongest counterevidence:** There are two entry levels, and the production + metadata writer uses the lower-level `Store` instead of `BranchStore`. +- **Why adjacent scores do not fit:** 3 does not fit because choosing the + low-level entry is required for replace-whole-tree and remote-parent behavior, + not unnecessary indirection. 2 does not fit because the scoped common + operations do not navigate competing implementations or configuration. 1 + does not fit because both paths are directly traceable. +- **Rule discrimination:** Ownership rule 2 could otherwise cause the + out-of-scope metadata writer's machinery to be counted again as simplicity + friction. The lens exclusions make that non-discriminating evidence here; + within the scoped implementation, the production primitives are direct. + +### `domain-model`: 2 + +- **Evidence:** `TreeEntries::set` accepts any `String` path without validation + (`git.rs:46-60`), and `write_tree` later interprets it by splitting on `/` + (`git.rs:149-153, 270-293`). The common metadata caller must therefore define + and apply `validate_metadata_path` outside this component before every + `TreeEntries` construction + (`fabro-workflow/src/run_metadata.rs:313-332, 471-480`). The component also + maps every unrecognized Git file mode to `Blob` + (`git.rs:21-35, 229-250`) rather than rejecting an unsupported state. +- **Strongest counterevidence:** `FileMode` is otherwise a closed enum, Git + object IDs use `git2::Oid`, and the current production metadata caller does + reject empty, absolute, dot-segment, and empty-segment paths before writing. +- **Why adjacent scores do not fit:** 3 does not fit because external path + validation is mandatory on every common metadata snapshot and the canonical + `TreeEntries` shape can always hold an invalid path. 1 does not fit because + the intended path and mode meanings remain clear and production does have a + validation step. +- **Rule discrimination:** Decision rule 4 clearly places the caller-validated + `TreeEntries` intermediate at 2. Whether unknown Git modes are a compatibility + escape hatch is ambiguous by itself, but it is not needed to choose the + score. + +### `duplication-knowledge`: 3 + +- **Evidence:** Branch-to-full-ref formatting is repeated in `Store::update_ref`, + `resolve_ref`, and `delete_ref` (`git.rs:182-225`), and the boundary metadata + writer has another `full_ref` transformation + (`fabro-workflow/src/run_metadata.rs:364-439`). `BranchStore::read_entry`, + `read_entries`, `list_entries`, and `tip_tree` also repeat parts of branch-tip + resolution (`branch.rs:119-184`). These repetitions are local and stable, but + there is no single helper enforcing them. +- **Strongest counterevidence:** Mutation sequencing is authoritative in + `BranchStore::write_with` (`branch.rs:56-81`), metadata branch naming has one + `META_BRANCH_PREFIX` constant (`lib.rs:7`), Git-author defaults have one + `Default` implementation (`author.rs:13-20`), and the repeated ref syntax is a + fixed Git protocol form rather than frequently changing Fabro policy. +- **Why adjacent scores do not fit:** 4 does not fit because ref normalization + and branch-tip traversal are still represented in several places. 2 does not + fit because there is no direct evidence that a routine checkpoint change + must alter those stable protocol transformations in sync; the repetitions are + isolated implementation knowledge. 1 does not fit because each policy has an + identifiable local authority even where a helper is absent. +- **Rule discrimination:** Decision rule 5 leaves a real 3-versus-4 ambiguity: + repeated `refs/heads/` can be classified as harmless protocol syntax. I score + 3 because the same branch-to-ref transformation crosses the component + boundary, but do not score 2 without evidence of routine synchronization. + +## Overall rubric observations + +- Decision rule 2 successfully distinguishes focused delegates from a lifecycle + that sends identity back through an event/caller round trip. +- Decision rule 6 prevents dead CI selectors from being double-counted as + ownership defects, but needs centrality/recurrence evidence to distinguish 2 + from 3. +- “One owner” and “complete lifecycle” need responsibility-sized interpretation + for route trees and hosted CI; otherwise healthy composition cannot reach 4. +- Decision rule 5 correctly keeps stable protocol repetition from automatically + becoming score 2, but the line between harmless syntax and a repeated + transformation remains the least discriminating part of this sample. + +## Round 2 revalidation + +| Component | Lens | Score | Confidence | +|---|---|---:|---| +| `fabro-http` | `duplication-knowledge` | 3 | Medium | +| `repository-ci` | `ownership-boundaries` | 2 | High | +| `fabro-checkpoint` | `ownership-boundaries` | 2 | High | +| `fabro-checkpoint` | `simplicity` | 3 | High | +| `fabro-checkpoint` | `domain-model` | 2 | High | +| `fabro-checkpoint` | `duplication-knowledge` | 3 | Medium | + +### `fabro-http` × `duplication-knowledge`: 3 + +- **Decisive evidence:** Proxy disabling has two concrete semantic + representations in the mapped entry layer: callers may set + `ProxyPolicy::Disabled` (`src/lib.rs:23-27, 90-94`), or call the separately + exposed `no_proxy()` builder operation (`src/lib.rs:96-100`). The former is + interpreted by calling the same underlying `inner.no_proxy()` transformation + during `build` (`src/lib.rs:160-165`). Both forms are used on direct boundary + paths: test constructors select the enum (`src/lib.rs:199-213`), while the + Unix-socket transport selects `no_proxy()` + (`lib/foundation/fabro-client/src/client.rs:2123-2134`). +- **Adjacent scores:** 4 does not fit revised rule 6 because there is a concrete + second representation of the same no-proxy decision. 2 does not fit because + an ordinary proxy-policy extension does not require manually synchronizing + those call sites; async and blocking policy construction still share the one + `define_builder!` mechanism (`src/lib.rs:72-193`). 1 does not fit because the + resolver remains a stable authority. +- **Remaining ambiguity:** `no_proxy()` can reasonably be viewed as a lower-level + reqwest operation rather than a second Fabro policy. Revised rule 6 makes 3 + the conservative result because `ProxyPolicy::Disabled` is implemented by + that exact operation, but this classification keeps confidence at Medium. + +### `repository-ci` × `ownership-boundaries`: 2 + +- **Decisive evidence:** The Rust check explicitly scans + `docs/public/api-reference/fabro-api.yaml` in its legacy-identity guard + (`rust.yml:80-92`), but neither push nor pull-request triggers include that + real path (`rust.yml:3-35`); they include the nonexistent `openapi/**` + selector instead (`rust.yml:18,34`). A routine API-contract change can + therefore change a scanned target without starting its owning check. +- **Adjacent scores:** 3 does not fit because the non-triggering target is on a + routine branch/PR check path, not an isolated manual edge. 1 does not fit + because the workflow, jobs, and intended trigger owner remain identifiable. + 4 is directly excluded by revised rule 3's trigger-coverage requirement. +- **Remaining ambiguity:** `typescript.yml:76` also invokes a Rust build from a + narrower trigger set, but that broader interpretation is unnecessary; the + explicitly scanned, non-triggering API contract is sufficient for 2. + +### `fabro-checkpoint` × `ownership-boundaries`: 2 + +- **Decisive evidence:** The mapped owner exposes low-level `Store` primitives, + while the routine metadata caller reconstructs the mapped branch lifecycle: + `RunMetadataWriter` owns branch, parent, and discovery state + (`fabro-workflow/src/run_metadata.rs:272-282`), then validates entries and + sequences blob, tree, commit, ref update, and retained parent state on every + snapshot (`run_metadata.rs:313-350`). No production boundary uses the + component's higher-level `BranchStore`. +- **Adjacent scores:** 3 does not fit because every metadata snapshot traverses + the split. 1 does not fit because the low-level Git owner and caller-side + lifecycle are both stable. 4 is directly excluded by revised rule 2: the + routine caller reconstructs a lifecycle the map assigns to this component. +- **Remaining ambiguity:** A narrower map that assigned only Git object + primitives to `fabro-checkpoint` could make this healthy delegation, but the + actual map explicitly assigns metadata branches and checkpoint commits. + +### `fabro-checkpoint` × `simplicity`: 3 + +- **Decisive evidence:** `Cargo.toml:16-24` carries `fabro-store` as a production + dependency, but scoped production code does not use it. The component also + exposes `BranchStore` as a parallel entry layer (`branch.rs:17-24`) that has + no production caller at this revision; the common metadata path uses `Store` + directly. The active `Store` path itself remains linear and direct + (`git.rs:123-226`). +- **Adjacent scores:** 4 is explicitly capped at 3 by revised rule 4 for the + unused production dependency and parallel unused entry layer. 2 does not fit + because routine production work does not repeatedly navigate those unused + elements; its `Store` path is direct. 1 does not fit because a stable common + path is easy to trace. +- **Remaining ambiguity:** Either isolated fact independently supplies the + revised rule's cap, so there is no material score ambiguity. + +### `fabro-checkpoint` × `domain-model`: 2 + +- **Decisive evidence:** `TreeEntries::set` accepts arbitrary string paths + (`git.rs:46-60`) before `write_tree` interprets them structurally + (`git.rs:149-153, 270-293`). Every common metadata snapshot must validate + those paths outside the mapped entry before constructing `TreeEntries` + (`fabro-workflow/src/run_metadata.rs:313-332, 471-480`). +- **Adjacent scores:** 3 does not fit revised rule 5 because caller validation + does not isolate an invalid-capable mapped entry used on every snapshot. 1 + does not fit because path meaning is stable and the caller does enforce it. + 4 is excluded because the canonical entry type itself admits invalid states. +- **Remaining ambiguity:** Unknown Git modes also collapse to `Blob` + (`git.rs:21-35`), but that compatibility question is not needed for the + score; the routine path shape is decisive. + +### `fabro-checkpoint` × `duplication-knowledge`: 3 + +- **Decisive evidence:** The short branch name is converted to + `refs/heads/{branch}` independently in `Store::update_ref`, `resolve_ref`, and + `delete_ref` (`git.rs:182-225`), while the routine boundary writer carries a + second `full_ref` conversion + (`fabro-workflow/src/run_metadata.rs:364-439`). These are concrete repeated + representations, but of stable Git protocol knowledge. +- **Adjacent scores:** 4 does not fit revised rule 6 because the + branch-to-full-ref transformation has a concrete second representation. 2 + does not fit because no ordinary mapped change is shown to require + synchronizing the stable Git namespace transformations; repeated call sites + alone are insufficient. 1 does not fit because the transformation and its + local authorities are clear. +- **Remaining ambiguity:** The literal can also be classified as harmless Git + syntax, which the lens excludes. Its repetition across the mapped boundary + supports 3, but the harmless-syntax distinction keeps confidence at Medium. diff --git a/.chisel/calibration/work/validation-2.md b/.chisel/calibration/work/validation-2.md new file mode 100644 index 000000000..b33efa10f --- /dev/null +++ b/.chisel/calibration/work/validation-2.md @@ -0,0 +1,450 @@ +# Chisel calibration validation 2 + +Revision reviewed: `6bb6b5efcc0e36b52e3c097f532d9f2c00914c6c` + +This is an independent reading of the final rubric. I did not seek or infer +earlier scores. + +## Scores + +| Component | Lens | Score | Evidence confidence | +|---|---|---:|---| +| `fabro-workflow` | `ownership-boundaries` | 2 | High | +| `fabro-workflow` | `domain-model` | 2 | High | +| `fabro-http` | `domain-model` | 4 | High | +| `fabro-http` | `duplication-knowledge` | 4 | Medium | +| `fabro-web-app` | `ownership-boundaries` | 4 | Medium | +| `repository-ci` | `ownership-boundaries` | 2 | High | +| `repository-ci` | `domain-model` | 2 | High | +| `fabro-checkpoint` | `ownership-boundaries` | 4 | Medium | +| `fabro-checkpoint` | `simplicity` | 4 | Medium | +| `fabro-checkpoint` | `domain-model` | 3 | Medium | +| `fabro-checkpoint` | `duplication-knowledge` | 2 | Medium | + +## Disputed assignments + +### `fabro-workflow` × `ownership-boundaries` — 2 + +**Direct evidence.** `WorkflowLifecycle` is a real central owner for engine +callback ordering: it contains the event, hook, fidelity, status, circuit +breaker, git, and artifact delegates and orders them in every callback +(`src/lifecycle/mod.rs:53-80`, `223-470`). The full run lifecycle nevertheless +crosses that owner on normal paths. `WorkflowLifecycle::on_run_end` only runs +the hook (`src/lifecycle/mod.rs:467-469`); `pipeline::finalize` separately builds +and emits the terminal event and stops the sandbox +(`src/pipeline/finalize.rs:524-635`); `RunSession::run` separately owns +initialize/execute/finalize, progress flushing, steering drain, and a second +sandbox cleanup guard (`src/operations/start.rs:796-953`); detached bootstrap +and completion guards own additional terminal-failure paths +(`src/operations/start.rs:956-1139`). A routine change to terminal ordering or +cleanup must account for these owners. + +**Strongest counterevidence.** The split is deliberate. In particular, +`finalize` documents why the terminal event must follow metadata flushing, and +the scope guards cover panic/interruption paths that an async lifecycle callback +cannot reliably cover. + +**Why adjacent scores do not fit.** Score 3 does not fit because the split is on +every ordinary terminal path, not an isolated compatibility path. Score 1 does +not fit because the owners and dependency direction are identifiable: +`RunSession` is the outer orchestrator and `WorkflowLifecycle` consistently owns +engine callbacks. + +**Rule discrimination.** Decision rule 2 is useful here, but “complete routine +lifecycle operations” must include terminal emission and resource cleanup, not +only engine callbacks. Without that reading, the positive orchestrator example +could make 3 and 2 hard to distinguish. + +### `fabro-workflow` × `domain-model` — 2 + +**Direct evidence.** The canonical execution result is the typed +`StageOutcome`, re-exported in `src/outcome.rs:1-12`. The common stage-completion +event instead stores `status: String` (`src/event/events.rs:264-293`). +`EventLifecycle::after_node` converts the typed value to a string for every +successful completion (`src/lifecycle/event.rs:319-378`), and +`event_body_from_event` reparses it into `StageOutcome` +(`src/event/convert.rs:309-348`). Unknown strings are silently reinterpreted as +a non-retryable failure (`src/event/convert.rs:14-24`). The same string +intermediate is used for synthetic terminal stages +(`src/lifecycle/event.rs:183-242`). + +**Strongest counterevidence.** Durable `fabro_types::StageCompletedProps` is +typed, and ordinary producers derive the string from a typed value rather than +accepting arbitrary user text. + +**Why adjacent scores do not fit.** Score 3 does not fit because the conversion +and invalid intermediate occur on the common event path for every completed +stage. Score 1 does not fit because `StageOutcome` supplies a stable canonical +meaning and most execution code uses it directly. + +**Rule discrimination.** Decision rule 4 and the repository example are +decisive. The rule would be non-discriminating if “compatibility escape hatch” +were allowed to describe the central `Event` type merely because the durable +type is healthier. + +### `fabro-http` × `domain-model` — 4 + +**Direct evidence.** `ProxyPolicy` is a closed two-variant vocabulary +(`src/lib.rs:23-27`). The environment boundary parses case-insensitively and +rejects every other value with a typed `HttpClientBuildError` +(`src/lib.rs:29-70`). Explicit policy has a documented precedence in +`resolve_with_env_value`, and both async and blocking builders resolve the +policy immediately before applying it (`src/lib.rs:38-59`, `160-166`, +`172-193`). The common production and test constructors all pass through those +builders (`src/lib.rs:195-213`). + +**Strongest counterevidence.** The builders also expose the lower-level +`no_proxy()` and `proxy()` methods (`src/lib.rs:96-106`), so callers can express +transport configuration outside the high-level enum. + +**Why adjacent scores do not fit.** Score 3 does not fit because the lower-level +methods are intentional reqwest-facade escape hatches; the common constructors +and environment boundary do not rely on an invalid or ambiguous policy value. +There is positive production enforcement rather than a test-only contract. + +**Rule discrimination.** Decision rule 4 discriminates well if “low-level +escape hatch” is read literally. If any alternate builder method were treated +as a second domain meaning, scores 3 and 4 would become difficult to distinguish +for facades. + +### `fabro-http` × `duplication-knowledge` — 4 + +**Direct evidence.** `define_builder!` is one production mechanism for all +shared async/blocking builder methods and for applying proxy policy +(`src/lib.rs:72-170`); the two concrete builders are declarations of that +mechanism (`src/lib.rs:172-193`). `ProxyPolicy::resolve` is the single authority +for explicit-versus-environment precedence (`src/lib.rs:38-59`), and the four +convenience constructors delegate to the builders (`src/lib.rs:195-213`). +Workspace boundary evidence reinforces this authority: `clippy.toml` disallows +raw reqwest client constructors in favor of these functions/builders. + +**Strongest counterevidence.** The tokens `system` and `disabled` also appear in +the human-readable error text, and the async/blocking test constructors repeat +the choice of `ProxyPolicy::Disabled`. + +**Why adjacent scores do not fit.** Score 3 does not fit because the repeated +tokens and two one-line convenience constructors do not form independent +authorities for a recurring transformation. The macro and resolver are what +enforce behavior. + +**Rule discrimination.** Decision rule 5 is useful but leaves a small judgment +gap around repeated diagnostic vocabulary. Here that repetition is +non-discriminating: adding a variant would make the exhaustive application +match fail to compile, while one diagnostic sentence is not a second policy +engine. This is why confidence is Medium rather than High. + +### `fabro-web-app` × `ownership-boundaries` — 4 + +**Direct evidence.** Shared HTTP configuration, authentication redirect, and +error normalization live in `app/lib/api-client.ts:64-160,213-309`. Read state +and cache keys live in `app/lib/queries.ts` and +`app/lib/query-keys.ts`; for example, `useRun` owns the run-detail fetch/cache +lifecycle (`queries.ts:182-187`). Shared run mutations and their cache updates +live in `app/lib/mutations.ts:42-208`. Run SSE connection sharing, cleanup, and +cache invalidation live in `app/lib/sse.ts:42-189` and +`app/lib/run-events.ts:129-308`. Browser resources with more specialized +lifecycles are likewise contained: terminal WebSocket/xterm/listener cleanup is +in `app/hooks/use-terminal-session.ts:62-229`, and install polling owns its +timer, interval, and abort controller in +`app/hooks/use-install-effects.ts:72-127`. + +`RunDetail` composes these owners and retains view-local state and interaction +ordering (`app/routes/run-detail.tsx:79-145,148-379`). Its size does not make it +the owner of transport or resource cleanup. + +**Strongest counterevidence.** Several feature routes perform feature-local +create/edit/delete calls and SWR invalidation directly, and `RunDetail` owns the +delete dialog, pending state, toast, list invalidation, and navigation +(`run-detail.tsx:193-205`) rather than using a single mutation hook for that +entire interaction. + +**Why adjacent scores do not fit.** Score 3 does not fit without a concrete +isolated lifecycle that has competing owners. The direct route mutations keep +their feature interaction lifecycle local and still use the shared transport; +they are not evidence that ordinary reads, SSE, or browser resources leak into +route composition. + +**Rule discrimination.** The final repository example is discriminating: +“busy route” must not itself count as boundary leakage. Confidence remains +Medium because the application scope is broad, although the representative +read, mutation, live-update, terminal, install, and route boundaries converge. + +### `repository-ci` × `ownership-boundaries` — 2 + +**Direct evidence.** The Rust workflow’s Clippy job owns a repository-wide +“legacy auth identity removal” guard that scans `lib/apps`, `lib/components`, +`lib/foundation`, `apps`, `lib/packages`, and the OpenAPI document +(`.github/workflows/rust.yml:80-91`). The workflow’s path filters do not include +`apps/**`, `lib/packages/**`, or +`docs/public/api-reference/fabro-api.yaml` +(`rust.yml:3-35`). A routine change in a scanned TypeScript/package/API path can +therefore introduce a forbidden identity without starting the job that owns the +guard. The policy lifecycle is placed under a narrower Rust trigger than the +responsibility it claims. + +**Strongest counterevidence.** The primary Rust and TypeScript build/test +responsibilities otherwise have clear workflow homes, read-only permissions, +and stable concurrency ownership (`rust.yml:38-147`; +`typescript.yml:30-77`). The TypeScript production build’s Rust step is a +legitimate composition point because it builds the Rust binary with the +embedded SPA. + +**Why adjacent scores do not fit.** Score 3 does not fit because the trigger +mismatch affects ordinary changes in multiple scanned source areas, not an +isolated maintenance path. Score 1 does not fit because the two main language +workflows and their jobs still have stable owners and dependency direction. + +**Rule discrimination.** No final rule explicitly says how to classify a check +whose declared scan scope exceeds its trigger scope. The ownership lens’s +“complete lifecycle” language is sufficient, but an explicit trigger/target +coverage rule would make 2 versus 3 less ambiguous. + +### `repository-ci` × `domain-model` — 2 + +**Direct evidence.** Every value in `.github/zizmor.yml` is a line-addressed +identifier: `rust.yml:37`, `rust.yml:49`, and `rust.yml:62` +(`.github/zizmor.yml:1-6`). At this revision those lines are respectively a +blank separator, the `fmt` job key, and a `run:` step—not action references. +Thus none is a current target for the configured `stale-action-refs` ignores. +Routine edits to `rust.yml` can change the accidental referents again without +changing the selectors. + +**Strongest counterevidence.** The syntax still communicates an intended +workflow-and-line selector, and the main workflow job/status vocabulary is +otherwise stable. + +**Why adjacent scores do not fit.** Score 3 does not fit because all three +values in the entire scoped zizmor configuration lack their intended current +referent; this is not one isolated compatibility value. Score 1 does not fit +because the selector format and intended concept remain identifiable even +though the instances are stale. + +**Rule discrimination.** Decision rule 6 is decisive and correctly keeps this +under domain model rather than ownership. It would not by itself distinguish 2 +from 3; the fact that every configured identifier is stale and line edits make +the condition recur supplies that distinction. + +## Control: `fabro-checkpoint` + +### `fabro-checkpoint` × `ownership-boundaries` — 4 + +**Direct evidence.** `git::Store` owns the `git2::Repository` and the low-level +blob/tree/commit/ref operations (`src/git.rs:101-227`). +`branch::BranchStore` owns branch identity, author identity, and the complete +local read-modify-write lifecycle, including parent resolution, tree read, +commit, and ref update (`src/branch.rs:17-82`). Author and trailer concerns are +focused modules rather than state hidden in callers (`src/author.rs`; +`src/trailer.rs`). Boundary evidence points in the intended direction: +`fabro-workflow` depends on these primitives, while its +`RunMetadataWriter` owns the additional temp repository, remote discovery, +credentials, push, and degradation lifecycle. That is a higher-level owner +using a lower-level delegate, not a reverse dependency. + +**Strongest counterevidence.** The production metadata writer uses `Store` +directly and manually sequences blob, tree, commit, and ref operations +(`fabro-workflow/src/run_metadata.rs:313-361`) instead of using `BranchStore`. +The crate name/description can make that look like the mapped checkpoint +lifecycle has escaped the component. + +**Why adjacent scores do not fit.** Score 3 does not fit if responsibilities are +classified by their actual state: `Store` owns local Git mechanics, +`BranchStore` owns local branch writes, and `RunMetadataWriter` owns remote run +metadata. No concrete resource is acquired by one of those owners and released +by another. + +**Rule discrimination.** Decision rule 2 is ambiguous for intentionally +low-level facades. Passing a branch to `Store::update_ref` should not alone mean +“resupplying identity” when the caller owns the higher-level remote branch +lifecycle and `Store` never claimed it. If the mapped purpose is instead read +as all run-checkpoint lifecycle, this assignment could become 2; that purpose +boundary should be fixed before using the control for strict agreement. + +### `fabro-checkpoint` × `simplicity` — 4 + +**Direct evidence.** The local branch write path is linear in +`BranchStore::write_with`: resolve parent, read tree, apply one caller mutation, +write tree, commit, update ref (`src/branch.rs:56-81`). Single-file, +multi-file, and delete operations are thin delegates to that path +(`src/branch.rs:84-117`). The lower-level tree conversion is one direct +flat-to-nested algorithm (`src/git.rs:229-309`), and trailer formatting/parsing +uses straightforward local control flow (`src/trailer.rs:9-87`). + +**Strongest counterevidence.** `BranchStore` has no external production caller +at this revision; the actual metadata path uses the lower-level `Store` API. +There is also some unused-looking surface such as `MetadataError` and generic +branch read/list/log helpers. + +**Why adjacent scores do not fit.** Score 3 does not fit because no direct +production evidence shows routine changes navigating the unused surface or +competing implementations. The production `Store` call sequence is itself +linear. The rubric explicitly says a public method alone does not establish +frequency, so unused API breadth cannot by itself create common-path +indirection. + +**Rule discrimination.** The score-4 requirement for a “production mechanism” +is mildly ambiguous when the clearest high-level mechanism has no production +caller but its lower-level mechanism does. Treating compiled non-test code as +sufficient would make the rule non-discriminating; this score instead relies on +the directly used `Store` path also being traceable. + +### `fabro-checkpoint` × `domain-model` — 3 + +**Direct evidence.** The common metadata boundary validates every path before +putting it into `TreeEntries` +(`fabro-workflow/src/run_metadata.rs:319-336,471-481`), explicitly selects +`FileMode::Blob`, and converts author strings with the fallible +`git2::Signature::now` before committing (`run_metadata.rs:337-345`). Within the +control, `FileMode` and `TreeEntries` give Git tree entries a stable meaning +(`src/git.rs:13-99`), and Git failures stay typed (`src/error.rs:3-32`). + +There is nevertheless isolated model friction. `TreeEntries::set` accepts any +string path with no invariant-bearing path type (`src/git.rs:59-61`); +`FileMode::from_i32` maps every unrecognized Git mode to `Blob` +(`src/git.rs:30-35`); `GitAuthor` has public raw string fields +(`src/author.rs:6-11`); and `BranchStore` says trees grow monotonically while +also exposing `delete_entry` (`src/branch.rs:17-19,111-117`). + +**Strongest counterevidence.** These are not merely hypothetical invalid +shapes: low-level public callers can bypass the production metadata-path +validation, and Git supports meaningful modes omitted by `FileMode`. + +**Why adjacent scores do not fit.** Score 4 does not fit because the low-level +types themselves do not reject invalid paths/authors or preserve every Git +mode. Score 2 does not fit because the directly traced production metadata path +validates before interpretation and does not depend on the fallback +`from_i32`; the friction is in lower-level escape paths and the currently +unused `BranchStore`, not every common snapshot. + +**Rule discrimination.** Decision rule 4 is useful but ambiguous about whether +a common caller validating raw values before a low-level API counts as a +“common-path invalid intermediate.” The rule should distinguish an actually +reparsed/ambiguous value from a raw value that has already passed one boundary +check but lacks an invariant-bearing Rust type. + +### `fabro-checkpoint` × `duplication-knowledge` — 2 + +**Direct evidence.** The branch-name-to-full-ref transformation +`refs/heads/{branch}` is repeated independently in `Store::update_ref`, +`Store::resolve_ref`, and `Store::delete_ref` +(`src/git.rs:182-225`). The direct production boundary repeats it again in +`RunMetadataWriter::full_ref` +(`fabro-workflow/src/run_metadata.rs:425-439`). A routine addition or change to +branch ref handling must preserve the same transformation in each location. +The trailer grammar has a second, smaller recurrence: `": "` is independently +formatted, parsed, and detected in `append`, `parse`, `format_message`, and +`has_trailing_trailer_block` (`src/trailer.rs:11-12,28-40,45-59,68-86`). + +**Strongest counterevidence.** Both grammars are tiny and stable, tests cover +the trailer forms, and the three Store methods currently agree. A helper could +look like cosmetic deduplication rather than a material abstraction. + +**Why adjacent scores do not fit.** Score 3 does not fit because branch +resolution/update/deletion are ordinary Store operations and direct boundary +code already supplies a fourth recurrence; this is not only a hypothetical +future variant. Score 1 does not fit because the repeated transformations are +stable and readily identifiable even though they lack a single authority. + +**Rule discrimination.** Decision rule 5 is decisive only if “direct evidence +of routine recurrence” includes several current operations applying the same +transformation. If it instead requires historical change evidence, the final +rule would be non-discriminating for a revision-only review and this assignment +would move toward 3. + +## Round 2 revalidation + +These scores supersede the corresponding Round 1 scores. + +### `fabro-http` × `duplication-knowledge` — 3 (Medium) + +**Decisive evidence.** `ProxyPolicy::parse` is the behavioral authority for the +external `system`/`disabled` vocabulary, while +`HttpClientBuildError::InvalidProxyPolicy` separately enumerates those values +in its diagnostic (`src/lib.rs:29-35,63-66`). The builder macro remains one +authority for applying the policy to both client kinds (`src/lib.rs:72-193`). + +**Adjacent scores and ambiguity.** Score 4 does not fit because the diagnostic +is a concrete second representation that can drift. Score 2 does not fit +because proxy behavior is not independently reimplemented: the shared +resolver and macro enforce it, and the two no-proxy convenience constructors +are call sites rather than separate authorities (`src/lib.rs:195-213`). The +remaining ambiguity is whether changing the closed proxy vocabulary is routine +enough to make the diagnostic synchronization central; I treat it as isolated. + +### `repository-ci` × `ownership-boundaries` — 2 (High) + +**Decisive evidence.** The Rust workflow's legacy-auth check scans `apps`, +`lib/packages`, and `docs/public/api-reference/fabro-api.yaml` +(`rust.yml:80-91`), but its push and pull-request path filters omit all three +(`rust.yml:3-35`). Under decision rule 3, that check owns trigger coverage for +every path it scans, so routine changes in those targets bypass its lifecycle. + +**Adjacent scores and ambiguity.** Score 3 does not fit because the missing +triggers affect several routine source and contract paths, not an isolated +edge. Score 1 does not fit because the Rust and TypeScript workflow owners and +dependency direction remain stable. No material ambiguity remains under the +new trigger-coverage rule. + +### `fabro-checkpoint` × `ownership-boundaries` — 2 (High) + +**Decisive evidence.** The map assigns checkpoint commits, trees, metadata +branches, authorship, and trailers to this component. The routine +`RunMetadataWriter` caller reconstructs that mapped lifecycle from `Store` +primitives: it writes blobs and a tree, creates the commit and author/message, +updates the ref, and pushes +(`fabro-workflow/src/run_metadata.rs:313-361`). Decision rule 2 therefore +places ownership at 2 even though the crate dependency points toward +`fabro-checkpoint`. + +**Adjacent scores and ambiguity.** Score 3 does not fit because this is the +common metadata snapshot path, not an edge case. Score 1 does not fit because +the dependency direction and the low-level `Store` role are stable, and +`BranchStore::write_with` demonstrates a coherent lifecycle owner inside the +crate (`src/branch.rs:56-81`). The only remaining ambiguity is how specialized +the metadata commit is, but the map explicitly includes metadata branches. + +### `fabro-checkpoint` × `simplicity` — 3 (High) + +**Decisive evidence.** `fabro-store` and `serde` are production dependencies +with no source use (`Cargo.toml:16-24`), and `BranchStore` is a parallel +high-level entry layer with no production caller outside this crate. Decision +rule 4 makes those isolated simplicity frictions and caps 4 at 3. + +**Adjacent scores and ambiguity.** Score 4 does not fit because the unused +production edges and parallel layer are concrete. Score 2 does not fit because +the production `Store` path remains direct; normal callers do not navigate the +unused dependencies or `BranchStore`. Whether `BranchStore` is retained for a +future caller is ambiguous, but the unused dependencies alone sustain 3. + +### `fabro-checkpoint` × `domain-model` — 2 (Medium) + +**Decisive evidence.** The mapped Git-tree entry accepts any `String` path +through `TreeEntries::set` (`src/git.rs:44-61`), while the routine metadata +writer must validate paths before constructing those entries +(`fabro-workflow/src/run_metadata.rs:319-336,471-481`). Decision rule 5 says +caller validation does not isolate an invalid-capable mapped entry. +`FileMode::from_i32` also collapses every unrecognized mode to `Blob` +(`src/git.rs:29-35`). + +**Adjacent scores and ambiguity.** Score 3 does not fit because raw paths cross +the common write boundary. Score 1 does not fit because tree entries, modes, +and authors retain stable meanings and the caller does validate its input. +Confidence is Medium because a deliberately low-level Git store can reasonably +leave some path constraints to higher-level schemas, although the revised rule +weighs against that interpretation. + +### `fabro-checkpoint` × `duplication-knowledge` — 3 (Medium) + +**Decisive evidence.** The `refs/heads/{branch}` transformation is repeated in +three `Store` operations and once at the workflow boundary +(`src/git.rs:182-225`; `fabro-workflow/src/run_metadata.rs:425-439`). +Trailer formatting, parsing, and block detection also encode the `": "` +convention separately (`src/trailer.rs:11-12,28-40,45-59,68-86`). These are +concrete second representations, so decision rule 6 caps 4 at 3. + +**Adjacent scores and ambiguity.** Score 2 does not fit on the current evidence: +adding a Store operation or trailer key may repeat a call-site convention, but +does not require an ordinary mapped change to modify all existing locations. +Score 4 does not fit because the representations are nevertheless concrete and +can drift. The remaining ambiguity is whether broader trailer-syntax support +would be routine maintenance; if so, its formatter/parser/detector +synchronization would support 2. diff --git a/.chisel/calibration/work/validation-3.md b/.chisel/calibration/work/validation-3.md new file mode 100644 index 000000000..3969b0b13 --- /dev/null +++ b/.chisel/calibration/work/validation-3.md @@ -0,0 +1,447 @@ +# Chisel calibration validation 3 + +Revision reviewed: `6bb6b5efcc0e36b52e3c097f532d9f2c00914c6c`. + +This is an independent reading of the final rubric and the assigned component +scopes. I traced representative production entry points and direct boundary +callers. I did not inspect prior calibration scores or any other file in +`.chisel/calibration/work/`. + +## Score summary + +| Component | Lens | Score | Evidence confidence | +| --- | --- | ---: | --- | +| `fabro-workflow` | ownership-boundaries | 2 | High | +| `fabro-workflow` | domain-model | 2 | High | +| `fabro-http` | domain-model | 4 | High | +| `fabro-http` | duplication-knowledge | 3 | Medium | +| `fabro-web-app` | ownership-boundaries | 4 | Medium | +| `repository-ci` | ownership-boundaries | 4 | Medium | +| `repository-ci` | domain-model | 2 | High | +| `fabro-checkpoint` | ownership-boundaries | 2 | High | +| `fabro-checkpoint` | simplicity | 3 | Medium | +| `fabro-checkpoint` | domain-model | 2 | High | +| `fabro-checkpoint` | duplication-knowledge | 3 | Medium | + +## `fabro-workflow` + +### `ownership-boundaries`: 2 (High) + +- **Evidence:** `src/lifecycle/mod.rs:53-80,221-469` provides a real central + `WorkflowLifecycle` and explicitly orders focused event, hook, fidelity, Git, + artifact, status, and circuit-breaker delegates. Its terminal callback, + however, only forwards `on_run_end` to the hook. Normal terminal persistence, + metadata completion, terminal event emission, and sandbox stopping instead + live in `src/pipeline/finalize.rs:524-635`. Bootstrap and execution failures + take another terminal path in `src/operations/start.rs:176-345`, while + `RunSession::run` also installs cleanup and drain guards at + `src/operations/start.rs:889-947`. A routine terminal-lifecycle change must + therefore coordinate the lifecycle orchestrator, finalizer, and detached + failure/guard paths. +- **Strongest counterevidence:** The normal phase sequence is plainly owned by + `RunSession::run` (`initialize -> execute -> finalize -> pull_request`), and + callback ordering inside graph execution has one obvious owner, + `WorkflowLifecycle`. +- **Why 3 does not fit:** Terminal completion, failure, persistence, and cleanup + are common paths, not isolated edge compatibility. The split therefore + remains central even though each individual phase is understandable. +- **Why 1 does not fit:** Stable phase owners and a stable dependency direction + are readily identifiable; the problem is coordination among them, not the + absence of ownership. +- **Rule discrimination:** The repository example correctly requires terminal + inspection and rule 1 makes the common terminal split score-capping. Decision + rule 2 is less literal here because no single identity is resupplied across + every split, but the score does not depend on that rule. + +### `domain-model`: 2 (High) + +- **Evidence:** `src/lifecycle/event.rs:319-390` starts with the typed + `StageOutcome` on an `Outcome`, serializes it with + `outcome.status.to_string()`, and stores the result in the + `Event::StageCompleted.status: String` field declared at + `src/event/events.rs:264-293`. Every successful stage then passes through + `src/event/convert.rs:14-24,309-348`, which reparses the string and silently + converts an unknown value into a non-retryable failure. This is the ordinary + durable-event path, not an import-only compatibility path. +- **Strongest counterevidence:** The destination event model already has the + canonical `fabro_types::StageOutcome`, parallel-branch completion carries it + directly, and other core run concepts use typed IDs, reasons, timings, and an + opaque `ResumeState` (`src/pipeline/types.rs:252-285`). +- **Why 3 does not fit:** The invalid intermediate occurs for each ordinary + successful stage before durable interpretation, so it is central rather than + an isolated escape hatch. +- **Why 1 does not fit:** `StageOutcome` itself has a stable, typed meaning; the + defect is the recurring string round trip between two typed points. +- **Rule discrimination:** Decision rule 4 is directly discriminating here: + this is exactly a common-path invalid intermediate. + +## `fabro-http` + +### `domain-model`: 4 (High) + +- **Evidence:** `src/lib.rs:23-61` gives proxy behavior a closed + `ProxyPolicy::{System, Disabled}` vocabulary. The environment boundary + accepts case-insensitive valid names, rejects every other value with a typed + `HttpClientBuildError`, handles non-Unicode values explicitly, gives explicit + policy precedence over the environment, and resolves absence to `System`. + Both generated builders invoke this resolver before constructing a client + (`src/lib.rs:72-193`), and the test-client entry points select + `ProxyPolicy::Disabled` rather than passing an unchecked string + (`src/lib.rs:195-213`). +- **Strongest counterevidence:** The facade deliberately exposes reqwest's + lower-level `Proxy` and `.no_proxy()` operations, so callers can compose + transport details outside the two-value environment policy. +- **Why 3 does not fit:** Those operations are typed builder choices, not + unvalidated representations of the `FABRO_HTTP_PROXY_POLICY` value. Every + common construction path still validates that boundary before use; I found no + material meaning or validation friction. +- **Why 1-2 do not fit:** There is one stable meaning, one resolver, and no + recurring conversion through an invalid intermediate. +- **Rule discrimination:** Decision rule 4 could be read ambiguously if every + low-level builder method is called a policy escape hatch. The rubric's own + `ProxyPolicy` example resolves that ambiguity in favor of the closed, + validated environment-policy model. + +### `duplication-knowledge`: 3 (Medium) + +- **Evidence:** `define_builder!` at `src/lib.rs:72-193` is one authoritative + production mechanism for the shared async/blocking builder surface and for + applying the resolved proxy policy. The four convenience constructors route + through those builders. The remaining repeated knowledge is narrow: + `"system"` and `"disabled"` appear both in the parser and in the manually + maintained `InvalidProxyPolicy` expectation text + (`src/lib.rs:29-35,63-69`). +- **Strongest counterevidence:** The macro removes the materially risky + async/blocking synchronization, and the compiler forces the policy-application + match to cover every enum variant. The two test helpers' use of + `ProxyPolicy::Disabled` is ordinary reuse, not a second policy authority. +- **Why 4 does not fit:** The user-facing valid-value list is a small second + representation that can drift from the parser, so there is some isolated + repeated domain knowledge. +- **Why 2 does not fit:** There is no direct evidence that routine changes + repeatedly synchronize separate async/blocking implementations. A future + enum variant is hypothetical, and rule 5 specifically says exhaustive + compiler-checked branches and hypothetical variants do not establish + competing authorities. +- **Rule discrimination:** Rule 5 cleanly rules out 2 but is non-discriminating + between 3 and 4 for a duplicated allowed-value error message. I treat that + message as real but isolated maintenance friction, hence 3. + +## `fabro-web-app` + +### `ownership-boundaries`: 4 (Medium) + +- **Evidence:** `app/entry.tsx:17-48` owns root creation, global SWR policy, + build-version guarding, toast mounting, and the single normal/install router + choice. `app/router.tsx:97-184` owns normal route composition, while + `app/install-router.tsx:6-22` owns the install graph. Shared HTTP translation + and unauthorized handling live in `app/lib/api-client.ts:213-309`; shared + reads such as `useRun` live in `app/lib/queries.ts:182-187`; recurring run + mutations and cache follow-up live in + `app/lib/mutations.ts:65-149`. Route components compose these owners. + Separately, `scripts/build.ts:183-249,289-368` contains the complete + app-local build, atomic publication, and old-build pruning lifecycle and + publishes only `apps/fabro-web/dist`; boundary tooling mirrors that output + into the Rust SPA rather than the web build writing across the boundary. +- **Strongest counterevidence:** Some route-specific CRUD mutations import + `apiData` and generated API objects directly, and the install feature spans + `install-app.tsx`, `install-api.ts`, `install-query.ts`, and effect hooks. + `run-detail.tsx` is also a busy composition point. +- **Why 3 does not fit:** The direct calls remain at the route-specific UX + owner and still use the shared transport/error boundary; shared read and + recurring run-lifecycle responsibilities are not reimplemented there. + Install state, transport, query, and browser effects have distinct homes. + I found no isolated lifecycle that must leave its owner and resupply identity. +- **Why 1-2 do not fit:** Runtime, routing, transport, queries, route UX, and + build publication all have stable owners with dependencies pointing from + composition toward shared services. +- **Rule discrimination:** The final repository example is useful and + discriminating: a large route is not by itself boundary leakage. The score + would change if direct routes reimplemented shared transport or cache + lifecycles, but representative boundary checks did not show that. + +## `repository-ci` + +### `ownership-boundaries`: 4 (Medium) + +- **Evidence:** `.github/workflows/rust.yml:48-147` owns Rust formatting, + lint/architecture checks, generated docs, Linux tests, twin E2E selection, + and manual macOS tests. `.github/workflows/typescript.yml:36-77` owns web and + generated-client typechecks, web tests, and the production embedded-SPA + integration build. Each workflow owns its concurrency and least-privilege job + permissions. The TypeScript workflow's `cargo dev build` is the intentional + integration boundary that consumes the web bundle; it does not create a + competing implementation of the web build. +- **Strongest counterevidence:** The TypeScript build job invokes Rust build + tooling, path scopes overlap around `lib/apps/fabro-spa/**`, and + `.github/zizmor.yml` is configuration whose consumer is not shown in these + files. +- **Why 3 does not fit:** Cross-language integration is part of the mapped CI + purpose and has one concrete home. The stale configuration values discussed + below are domain-model findings, while duplicated push/pull selectors are + duplication findings; counting either again as ownership friction would + violate the rubric's primary-lens rule. +- **Why 1-2 do not fit:** The Rust and TypeScript responsibilities and their + dependency direction are stable. Routine validation changes have an obvious + workflow owner rather than requiring competing lifecycle owners. +- **Rule discrimination:** The instruction not to penalize an unevidenced + missing lifecycle matters for the unseen zizmor consumer. The rubric is + otherwise discriminating once repeated selector policy is kept out of the + ownership lens. + +### `domain-model`: 2 (High) + +- **Evidence:** Both Rust trigger selectors name `openapi/**` + (`.github/workflows/rust.yml:6-19,22-35`), but that revision has no tracked + `openapi/` target. The actual Rust generator and TypeScript generator consume + `docs/public/api-reference/fabro-api.yaml` + (`lib/foundation/fabro-api/build.rs:159` and + `lib/packages/fabro-api-client/package.json:7`), a path omitted from both + workflow trigger models. This makes a core API-spec change invisible to the + intended CI trigger. In addition, all three + `.github/zizmor.yml:4-6` line selectors target + `.github/workflows/rust.yml` lines 37, 49, and 62, which are respectively + `workflow_dispatch`, the `fmt` job key, and a shell `run`, not action + references for `stale-action-refs`. +- **Strongest counterevidence:** Most configured branches, paths, action SHAs, + runner labels, job names, and commands have clear current targets, and both + workflow documents have a stable overall schema. +- **Why 3 does not fit:** The dead OpenAPI selector sits in both central Rust + push and pull-request triggers and omits the actual source of truth. It is not + merely an isolated stale lint suppression. +- **Why 1 does not fit:** The CI configuration language and almost all values + remain interpretable; the problem is recurring invalid/no-target identifiers, + not the absence of a stable configuration model. +- **Rule discrimination:** Decision rule 6 correctly classifies the no-target + identifiers as domain pressure, but it does not itself distinguish 2 from 3. + The centrality of the API source-of-truth trigger is what selects 2. + +## Control: `fabro-checkpoint` + +### `ownership-boundaries`: 2 (High) + +- **Evidence:** Inside the component, `BranchStore` owns a branch string and + author and delegates Git objects to `Store` + (`src/branch.rs:17-82`), which is a sensible direction. At the production + boundary, however, no production caller constructs `BranchStore`. + `fabro-workflow/src/run_metadata.rs:272-451` instead keeps `Store`, branch, + author, `parent_oid`, and discovery state as separate fields, manually writes + blobs and trees, supplies parents to `Store::write_commit`, resupplies the + branch to `Store::update_ref`, and owns fetch/push discovery. Other checkpoint + commit and trailer lifecycle work also remains in `fabro-workflow`. Thus the + mapped checkpoint/metadata-branch lifecycle crosses the scoped owner on the + normal production path. +- **Strongest counterevidence:** `Store` is itself a mapped public entry point, + the dependency direction remains `fabro-workflow -> fabro-checkpoint`, and + remote authentication/push orchestration reasonably belongs near a workflow + run rather than in a low-level Git object store. +- **Why 3 does not fit:** The caller-held branch and parent identity are used on + every metadata snapshot, not only in an isolated migration or uncommon + fallback. +- **Why 1 does not fit:** Low-level Git ownership and the higher workflow + orchestration are both stable and understandable; they simply split one + routine persistence lifecycle. +- **Rule discrimination:** Decision rule 2 is directly discriminating: + `RunMetadataWriter` retains and repeatedly resupplies the identity needed to + complete operations on `Store`. The mapped breadth of “metadata branches” + makes this more than ordinary parameter passing. + +### `simplicity`: 3 (Medium) + +- **Evidence:** The production low-level path is traceable: + `Store::write_blob -> TreeEntries::set -> Store::write_tree -> + Store::write_commit -> Store::update_ref` + (`src/git.rs:123-188`). `BranchStore::write_with` also gives branch-oriented + writes one linear read/modify/write implementation + (`src/branch.rs:56-117`). The recursive flat-tree conversion is justified by + Git's nested tree representation. The friction is isolated: `BranchStore` is + a sizeable second entry layer with tests but no production caller at this + revision, and `Cargo.toml:18` declares `fabro-store` although scoped + production code does not reference it. +- **Strongest counterevidence:** The two entry points represent legitimate + abstraction levels, and the mapped cartography names both. None of the normal + `Store` operations requires navigating configuration machinery or dynamic + dispatch. +- **Why 4 does not fit:** The unused higher layer/dependency is concrete, + avoidable surface and configuration burden, even though it is off the current + production common path. +- **Why 2 does not fit:** Routine production writes do not repeatedly choose + between `Store` and `BranchStore`; the observed caller consistently uses + `Store`, and that path is direct. +- **Rule discrimination:** The “public method alone does not establish + frequency” rule prevents treating `BranchStore` as a competing common path. + It is less discriminating between 3 and 4; the concrete unused dependency and + unused entry layer are why I select 3. + +### `domain-model`: 2 (High) + +- **Evidence:** `GitAuthor::from_options` accepts arbitrary name/email strings + (`src/author.rs:22-30`), while `BranchStore::new` only interprets them by + calling `Signature::now(...).expect(...)` + (`src/branch.rs:26-39`). `TreeEntries` stores paths as unrestricted `String` + and `BranchStore::write_entry/write_entries` put caller strings into it + without validation (`src/git.rs:46-90`, + `src/branch.rs:84-109`); interpretation and possible rejection occur later + while rebuilding Git trees. `FileMode::from_i32` also maps every unknown Git + mode to `Blob` (`src/git.rs:21-36`) rather than preserving or rejecting an + unknown shape. These invalid-capable intermediates sit on the mapped storage + entry paths. +- **Strongest counterevidence:** `FileMode` is closed for values the component + writes, normal metadata callers validate paths before constructing + `TreeEntries`, Git itself rejects malformed signatures/trees, and object IDs + use git2's typed `Oid`. +- **Why 3 does not fit:** Raw author and path values are carried by the ordinary + entry-point types and interpreted later; they are not confined to a separate + compatibility importer. +- **Why 1 does not fit:** Authors, tree entries, modes, branches, and commits all + have stable intended meanings. The issue is delayed validation and lossy + fallback, not an unidentifiable core concept. +- **Rule discrimination:** Decision rule 4 is discriminating here: these are + common-path invalid-capable intermediate shapes rather than a low-level + escape hatch unused by the entry path. + +### `duplication-knowledge`: 3 (Medium) + +- **Evidence:** Important transformations are mostly authoritative: + `FileMode::{as_i32,from_i32}` contains the mode mapping, + `BranchStore::write_with` contains branch read/modify/write, and + `GitAuthor::default` contains the default identity. The narrow repeated + knowledge is the bare-branch to full-ref transformation + `format!("refs/heads/{branch}")` in each of + `Store::{update_ref,resolve_ref,delete_ref}` + (`src/git.rs:182-225`), with another full-ref rendering at the direct + workflow metadata boundary. Trailer rendering also spells + `"{}: {}"` in both `append` and `format_message` + (`src/trailer.rs:9-65`). +- **Strongest counterevidence:** The repeated ref syntax is stable low-level Git + syntax, the three ref methods implement different operations, and the + apparent duplication in single-entry/multi-entry or tip/commit reads has + intentionally different result shapes. Unifying those operations would risk + a parameterized mega-helper. +- **Why 4 does not fit:** Full-ref and trailer-line rendering have small but real + second representations rather than one helper/type enforcing each + transformation. +- **Why 2 does not fit:** There is no direct evidence of routine changes + repeatedly synchronizing those stable renderings, and hypothetical future ref + methods do not satisfy decision rule 5. The repeated knowledge is isolated + from ordinary checkpoint-format extension. +- **Rule discrimination:** Rule 5 usefully rules out 2 but is + non-discriminating between 3 and 4 for repeated, stable protocol syntax. I + score 3 because the repetitions are concrete, while keeping confidence + Medium because their maintenance materiality is limited. + +## Round 2 revalidation + +I independently reapplied the simplified decision rules to only the requested +assignments. Scores below supersede the corresponding Round 1 judgments for +this revalidation. + +| Component | Lens | Round 2 score | Confidence | +| --- | --- | ---: | --- | +| `fabro-http` | duplication-knowledge | 3 | High | +| `repository-ci` | ownership-boundaries | 2 | High | +| `fabro-checkpoint` | ownership-boundaries | 2 | High | +| `fabro-checkpoint` | simplicity | 3 | High | +| `fabro-checkpoint` | domain-model | 2 | High | +| `fabro-checkpoint` | duplication-knowledge | 3 | Medium | + +### `fabro-http` × `duplication-knowledge`: 3 (High) + +- **Decisive evidence:** `define_builder!` remains the one mechanism for the + materially recurring async/blocking builder policy + (`src/lib.rs:72-193`). The parser and `InvalidProxyPolicy` message still hold + a concrete second representation of the allowed `"system"`/`"disabled"` + vocabulary (`src/lib.rs:29-35,63-69`). +- **Adjacent scores:** 4 does not fit because revised rule 6 explicitly caps a + concrete second semantic representation at 3. Score 2 does not fit because an + ordinary mapped change does not currently synchronize separate async and + blocking implementations; adding a future policy variant is not direct + recurrence evidence. +- **Remaining ambiguity:** None material. Revised rule 6 now resolves the prior + 3-versus-4 uncertainty. + +### `repository-ci` × `ownership-boundaries`: 2 (High) + +- **Decisive evidence:** The Rust workflow's architecture check scans + `apps`, `lib/packages`, and + `docs/public/api-reference/fabro-api.yaml` + (`.github/workflows/rust.yml:80-91`), but its push and pull-request triggers + omit all three routine target paths (`rust.yml:6-19,22-35`). Its Cargo jobs + also consume the real API specification through + `lib/foundation/fabro-api/build.rs`, yet that specification does not trigger + the workflow. The TypeScript workflow likewise consumes the generated API + client and performs the embedded integration build without making the source + specification a trigger. Under revised rule 3, each check owns this coverage; + the omitted routine targets are therefore central ownership pressure. +- **Adjacent scores:** 3 does not fit because API, app, and package changes are + routine targets of checks the workflow actually runs, not isolated edge + inputs. Score 1 does not fit because Rust and TypeScript job ownership and + dependency direction otherwise remain stable. +- **Remaining ambiguity:** None material. The nonexistent `openapi/**` value is + still a separate domain-model finding; the ownership finding rests on the + real scanned/consumed paths that fail to trigger. + +### `fabro-checkpoint` × `ownership-boundaries`: 2 (High) + +- **Decisive evidence:** The mapped higher owner is `BranchStore`, but the + routine production metadata caller instead retains `Store`, branch, author, + parent, and discovery state and reconstructs blob/tree/commit/ref lifecycle + from `Store` primitives in + `fabro-workflow/src/run_metadata.rs:272-451`. Revised rule 2 names this shape + directly. +- **Adjacent scores:** 3 does not fit because reconstruction occurs on every + metadata snapshot, not at an isolated edge. Score 1 does not fit because the + low-level `Store` and workflow-level caller are stable, identifiable owners; + the concern is the lifecycle split between them. +- **Remaining ambiguity:** The workflow reasonably owns remote authentication, + but that does not remove its reconstruction of the mapped checkpoint and + metadata-branch persistence lifecycle. + +### `fabro-checkpoint` × `simplicity`: 3 (High) + +- **Decisive evidence:** The current production `Store` write sequence is + linear and direct (`src/git.rs:123-188`). `BranchStore` is a parallel mapped + entry layer with no production caller at this revision, and `Cargo.toml:18` + declares the unused production dependency `fabro-store`. Revised rule 4 + classifies exactly this as isolated simplicity friction that caps 4 at 3. +- **Adjacent scores:** 4 does not fit because the parallel unused layer and + dependency are concrete. Score 2 does not fit because routine callers do not + navigate competing paths or machinery; they consistently follow the direct + `Store` path. +- **Remaining ambiguity:** None material after rule 4. `BranchStore` being a + mapped entry does not make it frequent when the boundary search finds no + production caller. + +### `fabro-checkpoint` × `domain-model`: 2 (High) + +- **Decisive evidence:** Mapped entry shapes accept unrestricted author and path + strings: `GitAuthor::from_options` stores raw values before + `BranchStore::new` interprets them with `Signature::now(...).expect(...)` + (`src/author.rs:22-30`, `src/branch.rs:26-39`), and + `TreeEntries`/`write_entry` carry unchecked string paths until Git-tree + construction (`src/git.rs:46-90`, `src/branch.rs:84-109`). Revised rule 5 + says caller validation and a typed destination do not isolate this + invalid-capable mapped entry. +- **Adjacent scores:** 3 does not fit because the invalid-capable shapes are on + mapped entry paths, not a compatibility-only edge. Score 1 does not fit + because the intended meanings of authors, paths, modes, and commits remain + stable. +- **Remaining ambiguity:** None material. Normal callers supplying valid values + does not make the entry type canonical by construction. + +### `fabro-checkpoint` × `duplication-knowledge`: 3 (Medium) + +- **Decisive evidence:** Bare branch names are independently rendered as + `refs/heads/{branch}` in `Store::update_ref`, `resolve_ref`, and `delete_ref` + (`src/git.rs:182-225`), and trailer lines are independently rendered in + `trailer::append` and `format_message` (`src/trailer.rs:9-65`). These are + concrete second semantic representations, so revised rule 6 excludes 4. +- **Adjacent scores:** 4 does not fit because the second renderings are real. + Score 2 does not fit because no evidenced ordinary mapped change must + synchronize the stable Git ref or trailer syntax across those locations; + future ref operations are hypothetical, while the existing operations have + distinct behavior. +- **Remaining ambiguity:** Limited ambiguity remains over whether stable + protocol syntax is material enough to count as semantic repetition at all. + Rule 6 does not define that threshold, so confidence remains Medium; if it + counts, 3 is the rule-directed score.