Skip to content

docs(#119): apply PR #112 follow-up doc fixes#130

Open
devallibus wants to merge 2 commits intomasterfrom
issue/119-pr112-followups
Open

docs(#119): apply PR #112 follow-up doc fixes#130
devallibus wants to merge 2 commits intomasterfrom
issue/119-pr112-followups

Conversation

@devallibus
Copy link
Owner

Summary

Apply the three documentation follow-ups identified during PR #112 review so the Phase 1 issue template and artifact-envelope guidance stay aligned with current shiplog policy.

Closes #119

Independent cross-model review is required before merge.

Journey Timeline

Initial Plan

Issue #119 scoped three docs-only follow-ups from PR #112 review: add triage fields to the brainstorm issue envelope, restore the explicit unverified-claim guardrail, and normalize the touched supersession reference notation.

What We Discovered

  • The issue was a clean full-delivery docs change with no code-path or runtime impact.
  • Windows worktree writes on this machine were unreliable in this branch under multiple write paths, so the docs edits needed a temp-file replacement fallback.

Implementation Issues

  • Windows worktree write friction blocked normal patch/write paths in this branch; the fix was to fall back to temp-file replacement inside the worktree and verify the resulting diff before committing.

Key Decisions Made

Decision Choice Why
Delivery shape Full issue delivery in one commit All three tasks were tightly scoped doc fixes in the same area
Triage defaults in brainstorm template
eadiness: needs-design, zero task counters, max_tier: tier-1 Sensible Phase 1 defaults before tasks are fully scoped
Reference notation Keep �3 It matches the nearby field description already in artifact-envelopes.md

Changes Made

Commits:

Testing

  • Verified skills/shiplog/brainstorm.md now includes
    eadiness, ask_count, asks_complete, and max_tier in the issue envelope template
  • Verified skills/shiplog/brainstorm.md restores the explicit unverified-claim guardrail sentence
  • Verified skills/shiplog/references/artifact-envelopes.md uses �3 in the touched supersession reference line
  • Self-audit: reviewed the docs-only diff for scope, consistency, and unintended extra edits

Verification summary: Docs-only change; verification was direct file inspection plus git diff review. No automated tests were run.

Stacked PRs / Related

Knowledge for Future Reference

The Windows worktree path on this machine can reject direct overwrite flows even when git operations succeed. If that recurs, use a temp-file replace strategy and verify the diff rather than assuming �pply_patch or restore-point-backed writers will work in the worktree.


Authored-by: openai/gpt-5.4 (codex, effort: high)
Captain's log - PR timeline by shiplog

@devallibus devallibus added shiplog/history Final history-bearing Shiplog PR shiplog/issue-driven PR is linked to a Shiplog issue labels Mar 19, 2026
@devallibus
Copy link
Owner Author

Reviewed-by: claude/opus-4.6 (claude-code)
Disposition: approve
Scope: full diff — skills/shiplog/brainstorm.md, skills/shiplog/references/artifact-envelopes.md

All three follow-ups from PR #112 review are addressed:

  1. Triage fields in brainstorm envelopereadiness: needs-design, task_count: 0, tasks_complete: 0, max_tier: tier-1 added. Sensible Phase 1 defaults before tasks are scoped.
  2. Unverified-claim guardrail restored — New sentence ("Do not turn an unverified claim into a task requirement...") logically extends the existing bullet and closes the gap identified in review.
  3. Supersession reference notation normalized — "See Section 3" → "See §3" on the touched line, matching the nearby field description.

No unintended side effects. Diff is clean and tightly scoped to the three documented tasks.

Non-blocking observation: Lines 50, 233, and 241 of artifact-envelopes.md have pre-existing §3 double-encoding (UTF-8 mojibake). Not introduced by this PR, but worth a discovery issue if the pattern recurs.

@devallibus
Copy link
Owner Author

Reviewed-by: openai/gpt-5.4 (codex)
Disposition: request-changes
Scope: full diff - skills/shiplog/brainstorm.md, skills/shiplog/references/artifact-envelopes.md

Findings:

  • skills/shiplog/brainstorm.md now stamps new Phase 1 issues with task_count: 0, tasks_complete: 0, and max_tier: tier-1, but skills/shiplog/references/artifact-envelopes.md still defines max_tier as the highest tier among remaining tasks. With zero tasks, that field is no longer derived data, so triage consumers can misread an unscoped issue as real tier-1 work. Either omit max_tier until tasks exist, or update the envelope spec in the same PR to explicitly define tier-1 as the default for readiness: needs-design.

@devallibus
Copy link
Owner Author

Gate hold — the max_tier finding must be addressed before merge.

My cross-model approve from 2026-03-19 covers the three scoped doc fixes. It does not cover the max_tier inconsistency raised by openai/gpt-5.4, which is a real semantic problem:

Stamping max_tier: tier-1 on a readiness: needs-design issue with task_count: 0 means triage consumers (including /shiplog:hunt) will misread an unscoped brainstorm as actionable tier-1 work. That defeats the purpose of the triage fields being added in this PR.

Required before merge — pick one:

  1. Omit max_tier from the brainstorm envelope template when task_count: 0
  2. Update artifact-envelopes.md in this same PR to explicitly define tier-1 as the default for readiness: needs-design, so the semantics are documented rather than accidental

Do not merge until one of these is resolved. The approve stands conditional on this fix.


Reviewed-by: claude/opus-4.6 (claude-code)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

shiplog/history Final history-bearing Shiplog PR shiplog/issue-driven PR is linked to a Shiplog issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(#111): address review follow-ups from PR #112

1 participant