docs(#28): define Drafting Table MVP user experience - #81
docs(#28): define Drafting Table MVP user experience#81fullsend-ai-coder[bot] wants to merge 6 commits into
Conversation
|
@coderabbitai review |
lukaskellerstein
left a comment
There was a problem hiding this comment.
Request changes. The document is well built and internally organised, and its 28 relative links all resolve. Three findings block: the approval flow never reaches build-work-item materialization, the draft-state rule contradicts the evaluability constraint, and the security posture omits credential isolation. Eight further findings are non-blocking.
| ID | Label | Title |
|---|---|---|
| A1 | issue (blocking) | Approval never materializes a work item |
| A2 | issue (blocking) | Draft state rule blocks component eval |
| A3 | issue (blocking) | Security posture omits credential isolation |
| A4 | issue (non-blocking) | Direct push contradicts branch protection |
| A5 | issue (non-blocking) | Entry-point commands are unregistered |
| A6 | issue (non-blocking) | Blocked reasons miss reconciliation failure |
| A7 | issue (non-blocking) | Subcommand count matches no source |
| A8 | issue (non-blocking) | File range disagrees with the count |
| A9 | issue (non-blocking) | True-bug intake is neither covered nor deferred |
| A10 | issue (non-blocking) | No sibling document points back here |
| A11 | todo | Q1 still listed as unresolved |
Every comment is anchored in the diff, so nothing is carried in this body.
Checked: all eight specification documents and ADR-0001 read end to end; every relative link and anchor resolved by script; markdownlint-cli2 v0.23.2 with .github/markdownlint-cli2.yaml reports no issues; the /spec-doc coverage check applied per AGENTS.md.
Not checked: pre-commit run --all-files (the tool is not installed in this environment, so only the markdownlint hook was reproduced by hand), and the content of issues #28, #30, #33 and #34.
Reviewed at head eafb9f786eb373bebcf83e0a9a80f63494a498c6.
| This state lives in the harness process (OpenCode's conversation | ||
| context). It is not persisted to Git or the WMS. If the session | ||
| ends without the user committing, draft conversation state is | ||
| lost. The committed specification state and the change-set | ||
| branch are the resumption points, not the conversation. |
There was a problem hiding this comment.
A2 · Draft state rule blocks component eval
issue (blocking): The rule that draft conversation state is never persisted removes the data source that the evaluability constraint requires.
Evaluability is a guiding principle at docs/architecture/overview.md:94 and a desired outcome at docs/vision.md:80, both stating it is an architectural constraint and not a nice-to-have. The eval-surface table names the exact data source: recorded Sketching sessions at docs/architecture/components.md:1523 and recorded Dimensioning sessions at docs/architecture/components.md:1524. The Specification Toolkit's external contract at docs/architecture.md:196 requires structured trace data for every agentic operation. The out-of-scope row at line 892 defers conversation persistence, but it defers session continuity, which is a different question from capturing traces for eval.
suggestion: Separate the two ideas. Keep the rule that conversation state is not a resumption point, and add that the session emits structured trace data for component-level eval, naming the trace format as the open question it already is in docs/architecture.md:196-200. Then say in the UX contract whether the user is told their session is recorded.
| [*] --> NewProject: protobot new | ||
| [*] --> Resume: protobot resume |
There was a problem hiding this comment.
A5 · Entry-point commands are unregistered
issue (non-blocking): The state diagram introduces protobot new and protobot resume, and no sibling document defines a protobot command.
The external interface inventory at docs/architecture.md:56 lists the TUI Drafting Table as a REPL and lists no protobot CLI anywhere in rows 1 to 12. Line 165 has the same problem for a different tool: it says the session initializes the .protobot/ control namespace through ears-manager, but neither subcommand table has an initialization subcommand (docs/architecture.md:227-247, nineteen rows, and docs/architecture/components.md:375-386, twelve rows). The document's own scope line at 8 says it does not prescribe TUI details, which makes a concrete launch command look unintended.
suggestion: Either make the diagram generic ("start a new project", "resume a project") or register the entry point and the initialization subcommand in docs/architecture.md in this same PR. A component or interface that lives in only one document is a gap in the hierarchy.
| This addresses [Q1](open-questions.md#q1-spec-gap-surfacing-ux) | ||
| for the TUI implementation. The interaction pattern is inline | ||
| suggestions within the conversation flow, not a separate gap | ||
| report. | ||
|
|
There was a problem hiding this comment.
A11 · Q1 still listed as unresolved
todo: The document states that it addresses Q1 for the TUI, and open-questions.md still lists Q1 as open with no pointer here.
Q1 at docs/architecture/open-questions.md:22-27 asks three things: how gaps are presented, whether there is a separate gap report, and the interaction pattern for out-of-scope against add-a-requirement. Lines 373 to 407 answer all three. The repository already has a pattern for this: Q7 at docs/architecture/open-questions.md:203-205 sits under Resolved questions and points at the document that resolved it.
suggestion: Move Q1 to the resolved section with a pointer to this document, or leave it open and soften the claim at line 403 to say the document proposes an answer for the TUI. Either is fine; the two documents should agree.
| 6. **Commit and PR.** On approval, the agent commits all | ||
| artifacts to the branch via `ears-manager` and Git operations |
There was a problem hiding this comment.
A1 · Approval never materializes a work item
issue (blocking): The approval flow stops at the commit and the PR, so the handoff that turns an approved change set into autonomous work is missing from the contract.
Three sibling documents put a required step after the merge. docs/architecture/user-interaction-flow.md:149 states that an approved change set materializes one build work item by default. docs/architecture/components.md:1604 names the registration hook that calls the Job Site materializer with the change-set ID, merge commit and materialization key. docs/architecture.md:652 shows the same step in the Drafting Table's own control-flow diagram, and docs/architecture.md:665 states the idempotency rule. The mutation-ownership table at lines 618 to 630 has no row for it either, so the document never names who owns the step.
suggestion: Extend step 6 with what happens after the merge, and add a row to the mutation-ownership table for change-set registration and build-work-item materialization, owned by the Job Site Materializer through the WMS Adapter (docs/architecture/components.md:1196-1200). Say what the user sees when the work item is created, since that is the moment their approval becomes autonomous work.
| (governed by #30 and #34). In single-player mode, the user | ||
| may push directly to `main` or open a self-reviewed PR. In | ||
| multi-player mode, the agent prepares a PR for reviewer merge. |
There was a problem hiding this comment.
A4 · Direct push contradicts branch protection
issue (non-blocking): The document says the single-player user may push directly to main, then says at line 643 that the constraints are enforced by branch protection.
docs/architecture.md:594 lists branch protection on main as a mandatory enforcement layer that prevents direct pushes and requires a reviewed PR. docs/architecture/overview.md:149-150 says the opposite for single-player mode. The sibling documents already carry this tension, and this document repeats both halves without saying which mode each applies to. docs/architecture/components.md:1685-1688 adds that direct push is not sufficient by itself, because a local register-approved-change-set command or hook must still run.
suggestion: Scope each statement to its topology. Say branch protection applies in multi-player and web modes, and that a single-player direct push must still be followed by the local registration step. That also removes the gap A1 describes for single-player.
| These constraints are enforced structurally: branch protection, | ||
| `ears-manager check` as a CI gate, path ownership in CI, and | ||
| optionally harness-level tool permissions (#33). |
There was a problem hiding this comment.
A3 · Security posture omits credential isolation
issue (blocking): The list of structural enforcement mechanisms names branch protection, the CI gate, path ownership and harness permissions, but never credential isolation.
Credential isolation through the Bridge/Gate pattern is an environmental constraint at docs/architecture.md:796, stating that agent processes never see real credentials. The full Gate contract is at docs/architecture/components.md:1778-1810. It matters here because this document gives the agent two mutation boundaries that Gates cover: the WMS Adapter and the Git host API, both in the ownership table at lines 618 to 630. The token source also differs per topology: docs/architecture.md:326 gives single-player the user's own Git host token with no OAuth 2.1 infrastructure, while the hosted modes use Bridge/Gate.
suggestion: Add credential isolation to the enforcement list and state the single-player token model the document is written against. One sentence saying the agent never holds a real credential, with a pointer to docs/architecture/components.md#authentication-and-credential-isolation, closes the gap.
| - Affected interface: ears-manager CLI | ||
| - Impact analysis not yet run | ||
|
|
||
| The Architecture lists 11 ears-manager subcommands. |
There was a problem hiding this comment.
A7 · Subcommand count matches no source
issue (non-blocking): The transcript says the Architecture lists 11 ears-manager subcommands, and neither sibling table has 11 rows.
docs/architecture.md:229-247 lists 19 subcommands. docs/architecture/components.md:375-386 lists 12. Line 759 repeats the number when the agent claims it covered all 11 subcommands, so the figure drives the rest of the transcript.
suggestion: Use the count from docs/architecture.md, or drop the number and write "each ears-manager subcommand". A fixture that cites a real document should match it, because a reader will check.
| - specs/requirements/req-cli-004.yaml through | ||
| req-cli-019.yaml (13 files) |
There was a problem hiding this comment.
A8 · File range disagrees with the count
issue (non-blocking): The commit summary names a contiguous range of 16 files and then calls it 13 files.
req-cli-004.yaml through req-cli-019.yaml is 16 identifiers. The count of 13 is right for the transcript: 11 subcommand requirements plus the two gap-closing ones at lines 770 and 794. That leaves req-cli-015 to req-cli-017 unexplained, and the same 13 appears at line 819 and line 849.
suggestion: Write the two runs explicitly, such as req-cli-004.yaml through req-cli-014.yaml plus req-cli-018.yaml and req-cli-019.yaml, or renumber the gap requirements to 015 and 016 so one range holds.
| | Decision | Rationale | | ||
| | --- | --- | | ||
| | Web Drafting Table UX | The web implementation shares the Specification Toolkit but has its own session management, authentication, and push notification model. Its UX contract is a separate deliverable. | | ||
| | Push notifications in the TUI | The TUI uses a pull model on session start. External notification channels (email, Slack) are a deployment concern, not a TUI UX concern ([Q2](open-questions.md#q2-async-requirement-suggestion-delivery)). | | ||
| | `ears-manager` command/result shapes | Defined by #30, not this document. This contract references `ears-manager` operations by name; #30 defines their request and result structures. | | ||
| | Specification Toolkit adapter internals | Defined by #33. This contract describes what the user sees; #33 defines how the Toolkit integrates with the OpenCode harness. | | ||
| | Git commit and branch behavior | Defined by #34. This contract describes when the user decides to commit; #34 defines the branch naming, commit content, and PR mechanics. | | ||
| | Request backlog refinement UX | The request-to-change-set refinement flow is described in [user-interaction-flow.md](user-interaction-flow.md#request-backlog-and-refinement). A detailed UX contract for the refinement interactions may be a follow-up deliverable. | | ||
| | Multi-player review ceremony | Multi-player mode adds reviewer merge as the approval gate. The UX difference is ceremony (PR review instead of direct push), not the interaction contract defined here. | | ||
| | Kit import UX | Kit discovery, import, and policy review are future capabilities. | | ||
| | Conversation persistence across sessions | Session continuity relies on committed specification state, not conversation history transfer. Whether conversation context should persist is an open question for the Specification Toolkit adapter (#33). | |
There was a problem hiding this comment.
A9 · True-bug intake is neither covered nor deferred
issue (non-blocking): A user reporting a true bug never appears in the document, and the out-of-scope table does not defer it either.
True-bug intake is an external entry point in the interface inventory at docs/architecture.md:63 and a user-facing path at docs/architecture/user-interaction-flow.md:998-1013, where the request skips Sketching and Dimensioning and enters Building directly. The classification that routes it is backlog refinement, which docs/architecture.md:505-519 calls one of the Drafting Table's three interaction phases. The table's own preamble at line 878 says the decisions are listed so they do not silently resurface, so an omission that is neither covered nor listed defeats that purpose.
suggestion: Add a row for true-bug intake and change-type classification with a rationale, or add a short section saying how the user reports a bug that needs no specification change.
| ## Related Documents | ||
|
|
||
| - [Vision](../vision.md) — Purpose, intended users, desired | ||
| outcomes, prototype scope, and non-goals. | ||
| - [Architecture](../architecture.md) — External interface | ||
| inventory, pluggable boundaries, persistent state, | ||
| environmental constraints, and Drafting Table boundary. | ||
| - [Overview](overview.md) — Guiding principles, EARS format, | ||
| workflow, and platform. | ||
| - [System Components](components.md) — Component architecture, | ||
| interfaces, and cross-cutting concerns. | ||
| - [User Interaction Flow](user-interaction-flow.md) — Phase | ||
| details, sequence diagrams, and testing strategy. | ||
| - [Open Design Questions](open-questions.md) — Unresolved | ||
| design questions across all areas. | ||
| - [Related Work](related-work.md) — Internal and external | ||
| projects informing the design. | ||
|
|
||
| [arch-guidance]: user-interaction-flow.md#what-belongs-in-the-architecture | ||
| [phase2]: user-interaction-flow.md#phase-2-dimensioning |
There was a problem hiding this comment.
A10 · No sibling document points back here
issue (non-blocking): This document links to all seven siblings, and not one of them links back, so the new file is unreachable from the hierarchy.
Every sibling's Related Documents section lists all the others: docs/architecture/components.md:1828, docs/architecture/user-interaction-flow.md:1120, docs/architecture/overview.md:302, docs/architecture/open-questions.md:210, docs/architecture.md:832 and docs/vision.md:273. The hierarchy list in AGENTS.md:16-29 also enumerates every file under docs/architecture/ and does not include this one.
suggestion: Add the entry to the sibling Related Documents sections and to the hierarchy list in AGENTS.md. Note that AGENTS.md is a Fullsend protected path, so a PR that edits it cannot be approved by the review bot and needs a maintainer.
|
/fs-fix Address 11 review comments on this PR, round 1. They are inline comments, not in your Handle exactly these, matched by the ID in the heading of each comment:
Skip everything else. Check the cited
Three notes on scope:
|
|
The fix agent completed, but the post-fix script failed before finishing. Workflow run: https://github.com/redhat-et/ProtoBot/actions/runs/34463797587 Details: |
|
🤖 Finished Fix · ❌ Failure (post-script /home/runner/work/ProtoBot/ProtoBot/.fullsend/.fullsend-cache/resources/sha256/21644e0238d0b83db763fa138c412322223b0ba4a919a542b839e11b6a3d62e7/scripts/post-fix.sh failed: exit status 1) · Started 10:02 AM UTC · Completed 10:17 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.19 |
|
The fix agent completed, but the post-fix script failed before finishing. Workflow run: https://github.com/redhat-et/ProtoBot/actions/runs/34463797587 Details: Please check the workflow logs for full details and retry with |
|
🤖 Finished Fix · ❌ Failure (post-script /home/runner/work/ProtoBot/ProtoBot/.fullsend/.fullsend-cache/resources/sha256/21644e0238d0b83db763fa138c412322223b0ba4a919a542b839e11b6a3d62e7/scripts/post-fix.sh failed: exit status 1) · Started 11:44 AM UTC · Completed 11:55 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.49 |
|
/fs-fix Address the 11 review comments on this PR. Retry after two rejected attempts. Two earlier attempts finished the work and were then rejected by the post-fix
Before you commit, lint the files you changed: The review comments are inline comments, not in your Handle exactly these, matched by the ID in the heading of each comment:
Skip everything else. A5 and A10 need edits outside Check the cited |
🔧 Fix agent — iteration 1 (human-triggered)Addressed all 11 review findings (A1-A11). Three blocking issues fixed: A1 adds post-merge materialization step and mutation-ownership row; A2 separates conversation non-persistence from trace data for eval; A3 adds credential isolation to enforcement. Eight non-blocking/todo items fixed: A4 scopes topology, A5 genericizes entry points, A6 adds blocked reasons, A7 fixes count, A8 fixes file range, A9 adds true-bug row to out-of-scope, A10 adds sibling back-links (AGENTS.md excluded as protected path), A11 resolves Q1 in open-questions.md. Fixed (11):
Tests: passed Decision points
Next steps:
|
A1: Extend step 6 with post-merge materialization step and add
change-set registration row to mutation-ownership table.
A2: Separate conversation non-persistence from structured trace
data emission for component-level evaluability.
A3: Add credential isolation (Bridge/Gate) and single-player
token model to the enforcement list.
A4: Scope branch protection to multi-player/web modes; note
single-player direct push requires local registration.
A5: Replace concrete `protobot new`/`protobot resume` commands
with generic labels; clarify initialization is a harness
detail.
A6: Add reconciliation-failure and policy-question reasons to
blocked-work presentation and resolution options.
A7: Replace incorrect "11 subcommands" count with "each
ears-manager subcommand".
A8: Split contiguous file range into two explicit runs matching
the 13-file count.
A9: Add true-bug intake and change-type classification row to
out-of-scope table.
A10: Add back-links in Related Documents of all sibling docs.
AGENTS.md is a protected path and was not modified.
A11: Move Q1 to the resolved section of open-questions.md with
a pointer to drafting-table-ux.md.
Note: pre-commit hooks were not run. `pre-commit` could not
complete (infrastructure failure); markdownlint-cli2 was run
directly and passed with zero issues.
Addresses #81
|
🤖 Finished Fix · ✅ Success · Started 11:58 AM UTC · Completed 12:09 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.06 |
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
@coderabbitai review |
|
Risk Assessment: moderate (2/5) DetailsScore held at 2 (moderate), anchored to the prior assessment: Tier 1 signals are unchanged - same file-count range (10 files), same PROTECTED_PATH_COUNT of 1 (AGENTS.md, a minor +2 line config touch), no security-sensitive, CI, or dependency changes, docs-only test ratio treated as neutral, and bot authorship. No Tier 2 or Tier 3 signal provides a specific reason to move off the prior score. Blast radius remains large (new 902-line spec plus 8 sibling doc edits) but is purely additive documentation, keeping the composite from being inflated by file/line-count size alone. Previous runRisk Assessment: moderate (2/5) DetailsScore held at 2 (moderate) — the prior rationale remains largely valid; PROTECTED_PATH_COUNT now reads 1 (AGENTS.md +2 lines) but this minor config touch does not shift the composite given bot authorship, docs-only scope, clear issue alignment, and a weighted composite of 2.03. Previous run (2)Risk Assessment: moderate (2/5) DetailsDocumentation-only PR by a bot with no protected paths, security-sensitive files, CI, or dependency changes. Blast radius is large (963-line new spec + 7 sibling updates) but change is additive and docs-only. Sibling files show high recent churn consistent with active spec development. Linked issue #28 has clear acceptance criteria with good scope match. Score unchanged from prior assessment. Previous run (3)Risk Assessment: moderate (2/5) DetailsDocumentation-only PR by a bot with no protected paths, security-sensitive files, CI, or dependency changes. Blast radius is large (963-line new spec + 7 sibling updates) but change is additive and docs-only. Sibling files show high recent churn consistent with active spec development. Linked issue has clear acceptance criteria with good scope match. Score unchanged from prior assessment. Previous run (4)Risk Assessment: moderate (2/5) DetailsDocumentation-only PR by a bot with no protected paths, security-sensitive files, CI, or dependency changes. Blast radius is large (980-line new spec + 7 sibling updates) but change is additive. Sibling files show high recent churn. Linked issue has clear acceptance criteria. |
ReviewFindingsHigh
Medium
Low
Next steps:
Previous runReviewFindingsMedium
Low
Next steps:
Previous run (2)ReviewFindingsMedium
Low
Next steps:
Previous run (3)ReviewFindingsMedium
Low
Next steps:
Previous run (4)ReviewFindingsMedium
Low
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 12:10 PM UTC · Completed 12:29 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $9.72 |
Medium: - Fix broken anchor components.md#work-item-lifecycle-states → #build-work-item-lifecycle - Fix broken anchor architecture.md#pluggable-boundaries → #environmental-constraints - Fix broken anchors architecture.md#external-interfaces → #external-interface-inventory and #drafting-table → #drafting-table-boundary - Update stale Q1 reference in components.md to reflect resolution Low: - Condense post-merge materialization to user-visible outcome, cross-reference components.md for internal details - Align section headings to title case (majority sibling pattern) - Remove parenthetical qualifier from provenance value 'agent-suggested (interactive)' → 'agent-suggested' - Condense multi-line subtitle blockquote to single-line (sibling doc pattern) - Reformat Q1 resolved entry in open-questions.md to match Q7/Q16/Q19 pattern Note: pre-commit hooks were not fully run. `pre-commit` could not complete (infrastructure failure — HTTP 403 on git fetch). Hooks were run directly: trailing-whitespace, end-of-file-fixer, and markdownlint-cli2 all passed. Addresses #81
🔧 Fix agent — iteration 2 (bot-triggered)Addressed all 9 review findings: 4 medium (3 broken cross-references, 1 stale Q1 reference) and 5 low (scope-creep in post-merge section, heading case convention, provenance naming, blockquote pattern, Q1 format). All fixes verified against actual file content. Markdownlint passes. Fixed (9):
Tests: passed Decision points
Next steps:
|
|
🤖 Finished Fix · ✅ Success · Started 12:30 PM UTC · Completed 12:40 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.43 |
|
@coderabbitai review |
|
🤖 Finished Review · ✅ Success · Started 12:41 PM UTC · Completed 1:03 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $9.66 |
- Capitalize 'Requirement Proposals and Gap Surfacing' in cross-reference link text to match the section heading (line 307) - Remove trailing periods from Related Documents entries to match the convention used by sibling specification documents Note: pre-commit hooks were not fully run. `pre-commit` could not complete (infrastructure failure — 403 fetching hook repos); markdownlint was run directly as a fallback and passed with 0 issues. Addresses #81
🔧 Fix agent — iteration 3 (bot-triggered)Fixed 2 of 3 findings: corrected cross-reference link casing and removed trailing periods from Related Documents. The CLAUDE.md hierarchy-registration finding is valid but deferred to a human because CLAUDE.md is a protected path. Fixed (2):
Disagreed (1):
Tests: passed Decision points
Next steps:
|
|
🤖 Finished Fix · ✅ Success · Started 1:04 PM UTC · Completed 1:11 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.59 |
|
@coderabbitai review |
|
🤖 Finished Review · ✅ Success · Started 1:13 PM UTC · Completed 1:35 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $7.95 |
|
The fix agent completed, but the post-fix script failed before finishing. Workflow run: https://github.com/redhat-et/ProtoBot/actions/runs/34483561392 Details: |
|
🤖 Finished Fix · ❌ Failure (post-script /home/runner/work/ProtoBot/ProtoBot/.fullsend/.fullsend-cache/resources/sha256/21644e0238d0b83db763fa138c412322223b0ba4a919a542b839e11b6a3d62e7/scripts/post-fix.sh failed: exit status 1) · Started 1:36 PM UTC · Completed 1:50 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.77 |
|
/fs-fix-stop |
|
Fix agent disabled for this PR. Remove the |
…test suite Synthesize the Drafting Table MVP UX specification with protocol invariants, hybrid gap surfacing, failure handling, and an acceptance test suite: - Add protocol state diagram and 6 invariants governing human review - Add 8-store state ownership matrix and anti-leakage approval gating - Add failure behavior section for tool, validation, and concurrency errors - Add 16-scenario acceptance evidence matrix and UX-001..015 decisions table - Resolve Q1, Q3, and Q6, and update Q2 in open-questions.md - Register drafting-table-ux.md in AGENTS.md specification hierarchy Addresses #28, #81
|
@coderabbitai review |
|
🤖 Finished Review · ✅ Success · Started 3:40 PM UTC · Completed 4:02 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.94 |
Add docs/architecture/drafting-table-ux.md defining the stable interaction contract for the first local Drafting Table during Sketching and Dimensioning. The document covers: - Three state categories (draft conversation, authoritative Git specification, WMS lifecycle) and why they must not be conflated. - Session lifecycle: new project, resume, exit, phase selection. - Sketching interactions: intent capture, Vision/Architecture proposal, review, and commit. - Dimensioning interactions: change-set workflow, requirement proposal and review, multi-interface navigation. - Gap surfacing UX: inline suggestions with accept, modify, reject, and out-of-scope options (addresses Q1). - Impact review and approval: deterministic and semantic candidates, user disposition, and change-set approval. - Blocked-work resolution: pull-model notification on session start, four resolution options (add requirement, approve out-of-scope, amend impact, defer). - Job Site status display (read-only visibility). - Authoritative mutation ownership table identifying the owning system and approval boundary for every mutation. - Representative Dimensioning transcript demonstrating session resume, requirement proposal/acceptance/modification, gap surfacing, impact analysis, and approval. - Explicit out-of-scope decisions for web UX, push notifications, and sibling contract internals. The contract is TUI-first and references sibling contracts #30 (ears-manager CLI), #33 (OpenCode adapter), and #34 (Git integration) without prescribing their internals. Note: pre-commit hooks were not run. pre-commit could not complete (infrastructure failure: network access blocked), and hooks were run directly via markdownlint-cli2 instead. All markdownlint checks passed. Closes #28
A1: Extend step 6 with post-merge materialization step and add
change-set registration row to mutation-ownership table.
A2: Separate conversation non-persistence from structured trace
data emission for component-level evaluability.
A3: Add credential isolation (Bridge/Gate) and single-player
token model to the enforcement list.
A4: Scope branch protection to multi-player/web modes; note
single-player direct push requires local registration.
A5: Replace concrete `protobot new`/`protobot resume` commands
with generic labels; clarify initialization is a harness
detail.
A6: Add reconciliation-failure and policy-question reasons to
blocked-work presentation and resolution options.
A7: Replace incorrect "11 subcommands" count with "each
ears-manager subcommand".
A8: Split contiguous file range into two explicit runs matching
the 13-file count.
A9: Add true-bug intake and change-type classification row to
out-of-scope table.
A10: Add back-links in Related Documents of all sibling docs.
AGENTS.md is a protected path and was not modified.
A11: Move Q1 to the resolved section of open-questions.md with
a pointer to drafting-table-ux.md.
Note: pre-commit hooks were not run. `pre-commit` could not
complete (infrastructure failure); markdownlint-cli2 was run
directly and passed with zero issues.
Addresses #81
Medium: - Fix broken anchor components.md#work-item-lifecycle-states → #build-work-item-lifecycle - Fix broken anchor architecture.md#pluggable-boundaries → #environmental-constraints - Fix broken anchors architecture.md#external-interfaces → #external-interface-inventory and #drafting-table → #drafting-table-boundary - Update stale Q1 reference in components.md to reflect resolution Low: - Condense post-merge materialization to user-visible outcome, cross-reference components.md for internal details - Align section headings to title case (majority sibling pattern) - Remove parenthetical qualifier from provenance value 'agent-suggested (interactive)' → 'agent-suggested' - Condense multi-line subtitle blockquote to single-line (sibling doc pattern) - Reformat Q1 resolved entry in open-questions.md to match Q7/Q16/Q19 pattern Note: pre-commit hooks were not fully run. `pre-commit` could not complete (infrastructure failure — HTTP 403 on git fetch). Hooks were run directly: trailing-whitespace, end-of-file-fixer, and markdownlint-cli2 all passed. Addresses #81
- Capitalize 'Requirement Proposals and Gap Surfacing' in cross-reference link text to match the section heading (line 307) - Remove trailing periods from Related Documents entries to match the convention used by sibling specification documents Note: pre-commit hooks were not fully run. `pre-commit` could not complete (infrastructure failure — 403 fetching hook repos); markdownlint was run directly as a fallback and passed with 0 issues. Addresses #81
…test suite Synthesize the Drafting Table MVP UX specification with protocol invariants, hybrid gap surfacing, failure handling, and an acceptance test suite: - Add protocol state diagram and 6 invariants governing human review - Add 8-store state ownership matrix and anti-leakage approval gating - Add failure behavior section for tool, validation, and concurrency errors - Add 16-scenario acceptance evidence matrix and UX-001..015 decisions table - Resolve Q1, Q3, and Q6, and update Q2 in open-questions.md - Register drafting-table-ux.md in AGENTS.md specification hierarchy Addresses #28, #81
…aseline - Clarify single change set per session lifecycle in drafting-table-ux.md - Align transcript preamble with actions demonstrated (no out-of-scope claimed) - Align Related Documents descriptions with majority sibling convention - Use sentence-case for Drafting table UX link in vision.md - Use 3-space continuation indent for Q2 in open-questions.md - Refresh skillsaw baseline following rebase on origin/main Addresses #81
926e59e to
605db80
Compare
|
@coderabbitai review |
| Agent: Approval recorded for revision e8f29ab. | ||
| Staging artifacts and committing to branch cs/add-cli-help-requirements. | ||
| Committed revision e8f29ab. | ||
| Running single-player registration hook... |
There was a problem hiding this comment.
[high] logic-error
The representative transcript's approval handoff contradicts this document's own authoritative-mutation protocol. After the user approves change set CS-005 at revision e8f29ab (line 786), the agent commits that same revision to the contributor branch cs/add-cli-help-requirements (line 792) and immediately runs the single-player registration hook (line 793), materializing work item WI-078 as 'ready-for-building' (line 794) — with no push or merge to main shown anywhere in between. This conflicts with the document's own 'Approval and post-merge materialization' section (commit/PR, then registration triggers the Materializer only once the change set lands on main), the mutation ownership table (registration hook is 'post-merge'), and docs/architecture/components.md (single-player mode pushes/merges to main, then calls register-approved-change-set; direct push alone does not materialize work). Since this transcript is the PR's representative acceptance fixture for issue #28 ('every authoritative mutation identifies its owning system and approval boundary'), it currently encodes the wrong approval boundary. Additionally, revision e8f29ab is presented both as the pre-approval draft revision and, unchanged, as the post-commit Git object — inconsistent with UX-009's claim that this identifier is a Git commit SHA, since staging and committing artifacts after approval should not reproduce an identical SHA unless the working tree was already clean.
Suggested fix: Rewrite the transcript's approval handoff to match the stated protocol: keep e8f29ab as the exact proposed revision shown at final review; after explicit approval, commit only if the working tree is still dirty; push or merge that revision onto main; then run register-approved-change-set against the resulting main commit and show WI-078 only after that registration succeeds.
|
|
||
| ### Git approval failure | ||
|
|
||
| If commit, push, PR creation, merge, or registration fails, the specification |
There was a problem hiding this comment.
[medium] logic-error
The 'Git approval failure' section states that if commit, push, PR creation, merge, or registration fails, 'the specification remains unapproved.' This conflates two distinct boundaries: Git approval (landing on main) and WMS materialization (registration). docs/architecture/components.md defines a reconciler for exactly the case where the Git merge succeeds but the WMS write fails, retrying materialization idempotently without re-approving anything. Under the current wording, a successful single-player push/merge to main followed by a failed registration would be incorrectly described as leaving the specification unapproved, even though main already carries the approved change set.
Suggested fix: Split the failure cases: Git commit/push/PR/merge failure leaves the change set unapproved (report last confirmed Git state, no WMS transition). Registration/WMS failure after a successful main update leaves the specification approved on Git and reports an incomplete materialization with a safe, idempotent retry — matching the Architecture reconciler in components.md.
| revise, or reject. | ||
| 3. **Architecture proposal:** The agent proposes an Architecture enumerating | ||
| external interfaces, persistent state, and environmental constraints. | ||
| 4. **Sketch commit:** When the user approves both Vision and Architecture, the |
There was a problem hiding this comment.
[medium] logic-error
The 'Starting a new project' flow (lines 251-260) only initializes the .protobot/ control namespace and creates a contributor branch before entering Sketching — it never opens a change set via ears-manager change-set create. The 'Sketch commit' step (line 319) then writes Vision/Architecture through ears-manager artifact put and commits them. This is inconsistent with the mutation ownership table, which records 'Update Vision/Arch' as updating a draft change set, and with docs/architecture/components.md, which defines artifact put as operating within an active change set. Dimensioning explicitly opens a change set (lines 350-352) before any artifact writes; Sketching does not, leaving an undefined precondition for artifact put during the new-project flow.
Suggested fix: In 'Starting a new project' (or at the start of Sketching), open a change set (or reuse the session's change set) before any artifact put call, and describe Sketch checkpoint acceptance as updating that draft change set — consistent with how Dimensioning is specified.
|
🤖 Finished Review · ✅ Success · Started 4:34 PM UTC · Completed 4:52 PM UTC Commit: Runtime: pi · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $4.77 |
|
please-review. I don't want to get into an infinite loop with fullsend reviews |
JohnStrunk requested changes with three findings. All three are fixed, plus two bot findings that describe undefined behavior rather than style. From the human review: - T1 The header no longer says draft. Merging the PR is what makes the document accepted. The five sibling documents still say it; changing those is a separate call. - T3 The document named no command that exists. There is no protobot executable, so the sentence now says the Drafting Table proposes the default layout at initialization and the user confirms it. The name came from the pending redhat-et#28 document, which uses protobot new and protobot resume in its session diagram, so PR redhat-et#81 needs the same correction; that is already open there as finding A5. - T4 The requirement store defaults to .protobot/requirements/, which keeps the files ears-manager manages together. components.md records the default next to the control namespace table, since the path sits inside .protobot/ and the table does not list it. The store is still a registry entry, so a project may point it elsewhere. From the bot review, fixed rather than deferred under the round 3 policy, because both leave behavior undefined and the push was happening anyway: - T5 The default-branch head is read from the local ref, after a fetch from the canonical remote. The two readings gave different hashes, and base_commit anchors the rest of the contract. - T6 The slug is cut at the last hyphen before position 40, or at exactly 40 characters when no hyphen precedes it. Every intent now yields one branch name. Not changed: - T2 The header-length finding names architecture.md as precedent itself and asks for nothing. - T7 The components.md row is a one-line summary that links to the full field table. Round 3: style only. - T8 AGENTS.md, and its CLAUDE.md symlink, is a protected path. The brief for redhat-et#34 says not to edit it, and the PR body asks the maintainers. Rebased onto upstream/main, 23 commits, no conflicts.
JohnStrunk's T3 said that protobot is not a command. The round 3 fix answered it in the text and named the pending redhat-et#28 document as the source of the name. Both parts were wrong. - git-integration.md no longer claims that no protobot executable exists. "Selecting the paths" now says the write goes through the ears-manager initialization operation that "Project initialization" records as a dependency on redhat-et#30. - user-interaction-flow.md gave `protobot generate` as the example for a CLI interface. That was the only place a protobot executable appeared in the docs, and it is where the name came from. The cell now reads "Build tool, code generator". Every other example in that table names a kind of interface in plain text rather than a command, so the row now matches its siblings. The redhat-et#28 draft on PR redhat-et#81 no longer uses protobot new or protobot resume, so the round 3 claim about it does not hold. Finding A5 there is marked outdated.
Summary
Adds
docs/architecture/drafting-table-ux.md, the stable interaction contract for the first local Drafting Table during Sketching and Dimensioning.The document defines:
ears-manager impact, semantic supplements from the agent, per-candidate user disposition, and change-set approval.References sibling contracts #30 (ears-manager CLI integration), #33 (OpenCode Specification Toolkit adapter), and #34 (single-player Git integration) without prescribing their internal details.
Completion criteria coverage
Test plan
components.mdand constraints fromoverview.mdCloses #28
Post-script verification
agent/28-drafting-table-ux)607fdfecc105c98f2fe3a3ffa84071ff1b0f7650..HEAD)