Skip to content

phase-review-gate reports PASSED on a partial §Approach parse (silently absorbs unmatched items) #1443

Description

@AlexOcheretianyi

Affects nx 4.32.14 (marketplace HEAD b991377).

Summary

/nx:phase-review-gate can enumerate a subset of an RDR's §Approach items and
then report PASSED, because a line that fails its item regex is silently absorbed
as continuation text of the previous item rather than being flagged.

Observed on RDR-034 in a downstream project: the gate cross-walked 8 of 10
§Approach items and would have passed on the subset. The two missing items were
noticed only because they were counted by hand.

This defeats the gate's stated purpose. From its own SKILL.md:

Root cause it prevents: silent scope reduction discovered mid-implementation.
RDR-112 Phase 1 shipped T2-only work; §Approach item 2 (T3 daemon) was silently
dropped. Discovered three closed phases later … Cost: 2-3 days of replanning.

A gate that mis-enumerates reproduces exactly that failure while reporting green,
which is worse than having no gate — it converts an unknown into a false assurance.

Location

nx/commands/phase-review-gate.md, parse_approach_items() (~line 100).

m = re.match(r'^(\d+)\.\s+\*\*([^*]+)\*\*[:\s]*(.*)', line)
if m:
    ...                      # starts a new item
elif current_num is not None:
    stripped = line.strip()
    if stripped and not stripped.startswith('-'):
        current_lines.append(stripped)      # <-- silently absorbs a missed item

Three inputs that go invisible

  1. Non-integer item numbers. ^(\d+)\. cannot match 5a. / 5b. — a common way
    to record a sub-item added after an RDR is drafted.
  2. A bold label that wraps. \*\*([^*]+)\*\* requires the label to open and
    close on the same line, so a long label spanning two lines fails.
  3. A label starting on the following line. The number and the bold label must share
    a line.

In all three cases the line falls through to the elif and is appended to the previous
item's summary. No warning, no count mismatch, no error.

Why the existing guard does not catch it

There is a zero-items error path:

> **ERROR**: §Approach section found but no numbered items parsed.

That covers a total parse failure. Nothing covers a partial one — which is the
case that occurs in practice, since most items parse fine and only the irregular ones
vanish.

Reproduction

§Approach containing:

1. **Normal item**: parses.
2. **Another normal item**: parses.
5a. **Sub-item added later**: silently absorbed into item 2's summary.
6. **An item whose bold label happens to run past the
   end of the line**: also absorbed.

Pass 1 enumerates 2 items. Supplying evidence for both yields PASSED.

Suggested fixes, cheapest first

  1. Detect the discrepancy. Independently count candidate item starters with a loose
    pattern (e.g. ^\s*\d+[a-z]?\.\s) and compare to the number parsed. On mismatch,
    fail with the unparsed lines quoted. This preserves the strict parse while making a
    partial one impossible to pass — and is the change most in keeping with the gate's
    purpose.
  2. Widen the number pattern to ^(\d+[a-z]?)\. and key items by the string rather
    than int(). Note current_num = int(m.group(1)) would need to change with it.
  3. Decouple the label from the number line — match the numbered starter first, then
    look for the bold label across the item's following lines.

(1) alone removes the false-assurance property even if (2) and (3) are declined.

Downstream mitigations, for anyone hitting this before it is fixed

  • Count §Approach items by hand and compare against Pass 1's table before accepting
    PASSED.
  • Author §Approach with plain integer numbering and a bold label that opens and
    closes on the number's own line.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions