reviewer judges open-closed ownership and lists undeclared new classes as review items #1316

Closed
opened 2026-10-25 00:41:08 +00:00 by jared · 3 comments
Owner

Problem

No review role asks whether a new object should have existed. reviewer.md and contract-auditor.md have no line about new objects in either direction. A class the programmer creates without a CREATES declaration is neither rejected nor flagged: CREATES only relabels evidence lines (plugins/os-sdlc/lib/os_sdlc/evidence/phantom_entry.rb:13). So a diff that pushes a new decision into an existing object passes review, and a diff that creates an object nobody asked for also passes without comment. Design record: docs/system-design-format/README.md section 4.

Reproduce

n/a: charter change.

Approach

Add one judgment question to the reviewer's design-judgment section: for each decision the diff adds to an existing object, does that object own the data the decision reads; if not, name the object that should, per lens A12. Add one visibility item: list every class or module the diff creates that no CREATES item named, as a review item, not a failure. For each CREATES item the diff realizes, check the same three gate answers seam-designer recorded (#1313): the secret, the existing object checked, one caller that would change. A class whose secret changes with no caller changing is listed as a review item naming the existing owner it should fold into, not a failure. Both produce findings in the existing transform-brief shape.

Required work:

  1. The open-closed judgment question in the reviewer charter — check: present own the data in plugins/os-sdlc/agents/reviewer.md
  2. Undeclared new classes listed as review items — check: present CREATES in plugins/os-sdlc/agents/reviewer.md
  3. The reviewer checks the three gate answers of each CREATES item — check: present one caller that would change in plugins/os-sdlc/agents/reviewer.md

Orchestrator work: run bin/refresh-plugins after the charter edit.

Rejected: failing the review on an undeclared class — that would punish object creation, which is the bias this batch removes.

Acceptance criteria

  1. Given a diff that adds a decision to an existing object that does not own the data it reads, when the reviewer runs, then a finding names the object that should own it.
  2. Given a diff that creates a class no CREATES item named, when the reviewer runs, then a review item lists the class and the review verdict is not failed for that reason alone.
  3. Given a diff with neither condition, when the reviewer runs, then no such finding appears.
  4. Given a CREATES item whose class has no caller that changes when its secret changes, when the reviewer runs, then a review item names the existing owner to fold into and the verdict is not failed for that reason alone.

Owner sketch

# prose artifact: plugins/os-sdlc/agents/reviewer.md

Origin

  • Trigger: session review of os-sdlc agent charters on 2026-10-24
  • Improvised this session: none
  • Chain: open-closed violations pass review ← no reviewer question about owners or new objects ← DESIGN (plugins/os-sdlc/agents/reviewer.md)
  • Root candidate: none found; searched reviewer.md, CREATES, open-closed, A12, undeclared, owns the data
  • Where: plugins/os-sdlc/agents/reviewer.md
  • Session: 5da956dc-a39d-4933-8b16-0fa90b4bb8ca
  • Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/5da956dc-a39d-4933-8b16-0fa90b4bb8ca.jsonl

VERDICT: CREATE

## Problem No review role asks whether a new object should have existed. reviewer.md and contract-auditor.md have no line about new objects in either direction. A class the programmer creates without a CREATES declaration is neither rejected nor flagged: CREATES only relabels evidence lines (plugins/os-sdlc/lib/os_sdlc/evidence/phantom_entry.rb:13). So a diff that pushes a new decision into an existing object passes review, and a diff that creates an object nobody asked for also passes without comment. Design record: docs/system-design-format/README.md section 4. ## Reproduce n/a: charter change. ## Approach Add one judgment question to the reviewer's design-judgment section: for each decision the diff adds to an existing object, does that object own the data the decision reads; if not, name the object that should, per lens A12. Add one visibility item: list every class or module the diff creates that no CREATES item named, as a review item, not a failure. For each CREATES item the diff realizes, check the same three gate answers seam-designer recorded (#1313): the secret, the existing object checked, one caller that would change. A class whose secret changes with no caller changing is listed as a review item naming the existing owner it should fold into, not a failure. Both produce findings in the existing transform-brief shape. Required work: 1. The open-closed judgment question in the reviewer charter — check: present `own the data` in `plugins/os-sdlc/agents/reviewer.md` 2. Undeclared new classes listed as review items — check: present `CREATES` in `plugins/os-sdlc/agents/reviewer.md` 3. The reviewer checks the three gate answers of each CREATES item — check: present `one caller that would change` in `plugins/os-sdlc/agents/reviewer.md` Orchestrator work: run bin/refresh-plugins after the charter edit. Rejected: failing the review on an undeclared class — that would punish object creation, which is the bias this batch removes. ## Acceptance criteria 1. Given a diff that adds a decision to an existing object that does not own the data it reads, when the reviewer runs, then a finding names the object that should own it. 2. Given a diff that creates a class no CREATES item named, when the reviewer runs, then a review item lists the class and the review verdict is not failed for that reason alone. 3. Given a diff with neither condition, when the reviewer runs, then no such finding appears. 4. Given a CREATES item whose class has no caller that changes when its secret changes, when the reviewer runs, then a review item names the existing owner to fold into and the verdict is not failed for that reason alone. ## Owner sketch ```ruby # prose artifact: plugins/os-sdlc/agents/reviewer.md ``` ## Origin - Trigger: session review of os-sdlc agent charters on 2026-10-24 - Improvised this session: none - Chain: open-closed violations pass review ← no reviewer question about owners or new objects ← DESIGN (plugins/os-sdlc/agents/reviewer.md) - Root candidate: none found; searched reviewer.md, CREATES, open-closed, A12, undeclared, owns the data - Where: plugins/os-sdlc/agents/reviewer.md - Session: 5da956dc-a39d-4933-8b16-0fa90b4bb8ca - Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/5da956dc-a39d-4933-8b16-0fa90b4bb8ca.jsonl VERDICT: CREATE
Author
Owner

Amended 2026-10-25 per accepted prediction: the reviewer checks the three gate answers recorded on each CREATES item (#1313) and lists a class with no changing caller as a review item naming the owner to fold into, not a failure.

Amended 2026-10-25 per accepted prediction: the reviewer checks the three gate answers recorded on each CREATES item (#1313) and lists a class with no changing caller as a review item naming the owner to fold into, not a failure.
Author
Owner

Work started on branch ticket-1315 (shared worktree for #1313-#1316). Deviation: no implementation map accepts category/prose-artifact, so these charter edits are hand-finished per the implement skill's recovery path: one agent per file, gated by the ticket's Required-work checks, aidd-lint, the suites, and one design audit.

Work started on branch ticket-1315 (shared worktree for #1313-#1316). Deviation: no implementation map accepts category/prose-artifact, so these charter edits are hand-finished per the implement skill's recovery path: one agent per file, gated by the ticket's Required-work checks, aidd-lint, the suites, and one design audit.
Author
Owner

Resolution

Done: reviewer charter gains judgment items 9 to 11: open-closed ownership of each decision added to an existing object (lens A12, finding names the owner that should hold it), undeclared new classes listed as review items, and the three gate answers checked on each CREATES item (secret, existing object checked, one caller that would change). Neither condition alone fails the review. Findings keep the transform-brief shape; CREATES card lines are read from the seam-designer handoff when the brief carries one.

Evidence: main 59f6cdb0 (commits da64a53c, faa10b62). Required-work checks: present 'own the data', 'CREATES', 'one caller that would change' in plugins/os-sdlc/agents/reviewer.md. aidd-lint: one pre-existing finding before and after, none new. ruby plugins/os-sdlc/tests/all.rb: 2514 runs, 0 failures. Design audit finding 8: the reviewer is in no approved map and no skill dispatches it, so these items do not yet run in a pipeline. Deviation: hand-finished, no map accepts category/prose-artifact.

Follow-ups: #1318 wires the reviewer into poodr-ticket-implementation with enrich seam-designer, or retires the charter (skeptic CREATE, P2).

## Resolution **Done:** reviewer charter gains judgment items 9 to 11: open-closed ownership of each decision added to an existing object (lens A12, finding names the owner that should hold it), undeclared new classes listed as review items, and the three gate answers checked on each CREATES item (secret, existing object checked, one caller that would change). Neither condition alone fails the review. Findings keep the transform-brief shape; CREATES card lines are read from the seam-designer handoff when the brief carries one. **Evidence:** main 59f6cdb0 (commits da64a53c, faa10b62). Required-work checks: present 'own the data', 'CREATES', 'one caller that would change' in plugins/os-sdlc/agents/reviewer.md. aidd-lint: one pre-existing finding before and after, none new. ruby plugins/os-sdlc/tests/all.rb: 2514 runs, 0 failures. Design audit finding 8: the reviewer is in no approved map and no skill dispatches it, so these items do not yet run in a pipeline. Deviation: hand-finished, no map accepts category/prose-artifact. **Follow-ups:** #1318 wires the reviewer into poodr-ticket-implementation with enrich seam-designer, or retires the charter (skeptic CREATE, P2).
jared closed this issue 2026-10-25 14:34:15 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
jared/cc-os#1316
No description provided.