engineering-issue-workflow

# Engineering Issue Workflow

**MANDATORY** for all engineering-critical issues.

> **Planning Steps delegated to `issue-planning-mode` skill.**
> Steps 1-5 of this workflow (Triage → Resource Intelligence → Plan → Adversarial Review → User Approval)
> are now fully defined in `.Codex/skills/coordination/issue-planning-mode/SKILL.md`.
> Load that skill for all planning work. This skill picks up at STEP 6 (Implement).

## Scope

An issue is engineering-critical if it has ANY of:
- Labels: `cat:engineering`, `cat:engineering-calculations`, `cat:engineering-methodology`, `cat:data-pipeline`
- Paths: `digitalmodel/`, `worldenergydata/`, `assetutilities/`
- Topics: offshore engineering standards (DNV, API, ABS, ISO), OrcaFlex, metocean, cathodic protection

Reference: `docs/standards/HARD-STOP-POLICY.md`

## The Workflow (7 Steps)

```
STEP 1: Triage              — classify issue, announce  \
STEP 2: Resource Intel      — search all knowledge sources, map artifact locations  | issue-planning-mode
STEP 3: Draft Plan          — pseudocode, file map, tests, acceptance criteria      |
STEP 4: Adversarial Review  — Codex + Codex + Gemini review the plan              |
STEP 5: ◆ HARD STOP ◆       — post to GitHub, label, wait for user approval        /
STEP 6: Implement           — TDD: tests first, then code
STEP 7: Cross-Review        — Codex + Gemini review implementation vs approved plan
STEP 8: Close               — commit, push, close issue with summary
```

### STEP 1: Triage

On first contact with an engineering issue:
1. Read the **full issue body** — scope, acceptance criteria, references
2. Classify complexity:
   - **T1** (trivial): single-line fix, config, typo → brief plan, still requires approval
   - **T2** (standard): new module, multiple file changes, tests needed → full workflow
   - **T3** (complex): multi-module, architecture change, standards implementation → full workflow + subagents
3. Identify what **standards, modules, test fixtures, and documents** are relevant
4. **TELL THE USER**: "This is engineering-critical. I'm running resource intelligence before writing the plan."

### STEP 2: Resource Intelligence (AUTOMATIC)

Before writing the plan, search ALL available sources. Don't skip this — past sessions that jumped straight to implementation produced wrong code because they didn't check what already existed.

**Search in this order:**

a) **Existing repo code** — is this already implemented?
   - `search_files(pattern="function_or_calc_name", path="digitalmodel/")`
   - `search_files(pattern="module_name", path="worldenergydata/")`
   - `search_files(pattern="related_topic", path="assetutilities/")`

b) **Standards coverage** — does the standard exist in the registry?
   - Read `data/document-index/standards-transfer-ledger.yaml`
   - Look for the standard number (e.g., DNV-RP-C212, API 579, ISO 19901-6)
   - Check if status is "done" or "gap"

c) **Document intelligence** — are there relevant indexed documents?
   - Read `data/document-index/online-resource-registry.yaml`
   - Search for document name, topic, or standard
   - Check /mnt/ace/ for local PDFs (if mounted and accessible)
   - If the user asks to preserve raw context before implementation, promote verified `/mnt/ace` references into the relevant `knowledge/wikis/<domain>/` LLM wiki before coding: source pages + concept anchors + comparison/extraction summary; then update the plan and GitHub issue with the wiki anchors.

d) **Engineering reference data** — what parameters/constants apply?
   - `search_files(pattern="constants", path="digitalmodel/")`
   - `search_files(pattern="parameters", path="digitalmodel/")`
   - Read any relevant reference markdown in docs/

**Index metadata usage (post #1878):** `index.jsonl` now carries `content_type` (100% populated, derived from extension) and `summary_done` (True iff a non-empty summary exists on the ace drive). Across the 649K-record corpus, `content_type` is highly discriminating but `summary_done=True` is only ~16% because ~72% of records are CAD files with no extractable text. For curated lookups still prefer `online-resource-registry.yaml` (247 entries) and `standards-transfer-ledger.yaml` (425 standards). See #1878 for enrichment provenance and #2309 for the planned `summary_file_exists` split.

### STEP 3: Write the Plan

Present the plan to the user. Format:

```markdown
## Plan for #ISSUE: Title

### Resource Intelligence Summary
- **Existing code found**: {file paths and what exists}
- **Standards applicable**: {list with status: done/gap}
- **Documents consulted**: {list from registry, /mnt/ace/, etc.}
- **Gaps identified**: {what is missing from the repo}

### Deliverable
One sentence: what will be built or changed.

### Files to Change
| Action | File | Reason |
|--------|------|--------|
| Create | path/to/new_file.py | main implementation |
| Create | path/to/test_new_file.py | TDD test suite |
| Modify | path/to/existing.py | extend functionality |

### Tests (TDD)
- [ ] Test: {name} → {what it verifies}
- [ ] Test: {name} → {what it verifies}
- [ ] Test: {name} → {what it verifies}

### Acceptance Criteria
- [ ] All new tests pass via `uv run pytest ...`
- [ ] No regression in existing tests
- [ ] {specific numerical/engineering check}
- [ ] Documentation updated

### Risk
- {what could go wrong}
- {user attention needed}

### Complexity: T1 | T2 | T3
```

### STEP 4: ◆ HARD STOP — USER APPROVAL REQUIRED ◆

**STOP. Do NOT write any code. Do NOT create any files. Do NOT run any tests.**

Wait for the user to respond with one of:
- **APPROVE** / **GO** / **YES** → continue to Step 5
- **REVISE** / **CHANGE** → user provides feedback, re-do Step 3
- **REJECT** → ask what approach the user prefers

**If the user says "just do it" or "go ahead" WITHOUT seeing the plan:**
Present the plan first. Then wait. The approval must come AFTER seeing the plan.

For **overnight/batch sessions** (user not present):
- Write the plan as a **GitHub issue comment** before implementing
- Implementation starts only after the plan comment is posted
- User reviews results the next morning

### STEP 5: Implement (TDD)

After user approval:

1. **Tests FIRST** — write the test file, run it, confirm it FAILS
2. **Implement** — minimum code to make tests pass
3. **Run tests** — confirm they PASS
4. **Full test suite** — `uv run pytest` on the affected repo, confirm no regressions
5. **Self-review** — check the code against the approved plan

**Digitalmodel is a SEPARATE git repo** — commit from within `digitalmodel/` dir, NOT workspace-hub root.

**Use `uv run`** — never bare `python3` or `pip`.

### STEP 6: Adversarial Cross-Review

After implementation passes all tests:

1. Route to **Codex AND Gemini**
2. Each reviewer receives: the approved plan, the diff, test results, acceptance criteria
3. Collect verdicts: APPROVE | MINOR | MAJOR
4. If any MAJOR: present to user, fix, re-test
5. If all APPROVE or MINOR (resolved): proceed to Step 7

### STEP 7: Close

- Conventional commit message referencing the issue
- Push
- Close GitHub issue with summary: implementation done, test results, cross-review verdicts, follow-ups

---

## Non-Critical Issues

Issues WITHOUT engineering-critical labels:
- **Do NOT skip planning.** `issue-planning-mode` applies to ALL issues going forward.
- Run the same planning sequence: Issue Intake → Resource Intelligence → Draft Plan → Adversarial Review → User Approval
- After approval, implementation may use a lighter execution workflow if the issue is not engineering-critical
- **TDD is still mandatory** — tests before implementation, always
- Implement → review as appropriate → commit → close

---

## Pitfalls & Gotchas (from historical session data)

### Engineering calculation plan hardening

When planning or executing engineering-calculation issues, especially in `digitalmodel`, apply the checklist in `references/engineering-calculation-plan-hardening.md` before moving to `status:plan-review` and again before implementation closeout. Key lessons: split equal/opposite physical quantities into explicit fields, define force-line lever arms geometrically, name exact output artifacts/charts, add non-tautological sign and identity tests, verify public imports outside pytest path injection, include upstream-helper regression slices, record engineering-registry retrieval evidence, explicitly decide whether `/mnt/ace`/wiki promotion is required before coding, and during implementation honor YAML-configured artifact names/chart subsets, reject empty sweeps early, preserve subprocess environments, and pin physical sign conventions in code/YAML/docs/tests.

### Agents Skipping the Workflow

**What happened:** 120+ engineering commits in 14 days, 542 commits since Mar 24, only 1 review artifact. The existing enforcement scripts (cross-review gate, review router, pre-push hook) all default to WARNING mode. Nobody blocked anything.

**How to avoid:** This skill is the new baseline. The plan must be presented BEFORE implementation. The user must approve. If you're unsure whether an issue is engineering-critical, ASK — don't assume.

### User Says "Just Implement It"

**What happened:** User tells the agent to skip planning and go straight to code. Agent complies, producing code without context.

**How to handle:** Show a brief plan first. The user can still approve quickly, but they must SEE the plan. "Here's what I'll do: [3 lines]. OK?" is sufficient.

### Thinking Work Is "Too Trivial" for a Plan

**What happened:** Agent decides a change is simple and skips the plan. Often the "simple" change was actually part of a larger system and broke something.

**How to handle:** Even T1 trivial changes need at least a one-line plan statement and user acknowledgment. No exceptions.

### Digitalmodel Is a Separate Git Repo

**What happened:** Agent commits from workspace-hub root, but `digitalmodel/` is a gitignored separate repo. Commits are lost.

**How to handle:** When touching `digitalmodel/` files, `cd digitalmodel/` before running `git add/commit/push`.

### Index metadata reference (post #1878)

**Current state:** `index.jsonl` carries `content_type` (100% populated) and `summary_done` (True for ~16% of records; the 84% False is dominated by CAD files without extractable text).

**How to query:** Read records directly from `data/document-index/index.jsonl` — fields are present on every record. Validator at `scripts/data/document-index/validate-index-metadata.py` enforces coverage thresholds. For curated engineering lookups (small, domain-specific), `online-resource-registry.yaml` and `standards-transfer-ledger.yaml` remain the reliable complementary sources.

### Bypass Environment Variables

**What happened:** The enforcement scripts support `SKIP_REVIEW_GATE=1` and `GIT_PRE_PUSH_SKIP=1`. Agents discover and use them to skip checks.

**How to handle:** These are for emergencies only. If you're considering using them, present the reason to the user first and get explicit approval.

### Cross-Review Artifacts Location

**Where review results live:** `scripts/review/results/`
**Pattern:** `{date}-{description}-{provider}.md`
**Last review:** Apr 2 (one Codex retroactive review). Everything since is unreviewed.

### Hermes Does Not Have SessionStart Hooks

**What happened:** Codex has `.Codex/settings.json` hooks that can enforce behavior at session start. Hermes has no equivalent — it relies on AGENTS.md (always loaded) and skills (loaded on demand).

**How to handle:** This skill MUST be referenced when working on engineering issues. For Hermes sessions, the agent may need to load it manually: "I should use the engineering-issue-workflow skill for this issue."

### Past Session: 6.1M Wasted Tool Calls

**What happened:** Three WRK items (WRK-1022, WRK-1012, WRK-1005) consumed 6.1M tool calls across runaway sessions with no exit conditions, no completion gates.

**How to handle:** If a task is taking more than 200 tool calls, STOP. Present progress summary to the user. Ask if they want you to continue or change approach.

### Smoke Test Failures Unnoticed for 12 Days

**What happened:** worldenergydata test runner crashed (passed=0, failed=0) for 12 consecutive days with no fix.

**How to handle:** After making changes to test files, run `uv run pytest` and confirm the output is sensible. If tests disappear or the runner fails, fix immediately.

---

## Enforcement Escalation Strategy (from #1839)

This skill is Option 1 of a 3-phase enforcement approach:

**Option 1: Skill-based (CURRENT — implementing now)**
- Works across all agents (Codex, Codex, Gemini, Hermes)
- Instructions, not enforcement — agent can ignore it
- Deployed as `engineering-issue-workflow` skill

**Option 2: Hermes prefill (LAYER IF AGENCY IS 2 WEEKS)**
- Set `~/.hermes/config.yaml`: `prefill_messages_file: 'docs/standards/engineering-workflow-prefill.md'`
- Auto-injects workflow at every Hermes session start
- Hermes only, requires `hermes chat` invocation

**Option 3: Codex hooks (LAYER IF OPTION 1 FAIL AFTER 2 weeks)**
- SessionStart hook in `.Codex/settings.json` detects engineering issues and injects workflow
- PreToolUse hook blocks Write/Edit/Agent if no `.planning/plan-approved/${ISSUE_NUM}.md` exists
- Cannot be bypassed without hooks disabled

**Escalation triggers (check after 2 weeks):**
- Engineering commits still ship without plan review
- Cross-review rate stays below 30%
- User reports agents skipping the workflow

Reference: Issue #1876 tracks Option 2+3 implementation.