Skip to content

skill-quality: narrow what check 21 claims to parse (follow-up to the #1096 parsing contract) #1493

Description

@kyle-sexton

Follow-up to #1096, which shipped check 21 (fresh-eyes declaration conformance) together with a stated parsing contract at plugins/skill-quality/skills/check/reference/fresh-eyes-declarations.md.

Scope conclusion

Incremental CommonMark fidelity patching in a hand-rolled awk scanner does not converge. That is the conclusion this issue records, and the evidence is the review progression on #1096, not a judgement call:

  • Six consecutive external-review rounds each found a distinct CommonMark edge case in the same structure pass.
  • Two consecutive rounds were defects in the previous round's own fix. The container-nested-fence support added in one round produced an unclosed-fence runaway in the next; the escaped-backtick fix in one round destroyed a legitimate span closer in the next (escapes are not processed inside a code span, so a backslash before the closing run is content and the run still closes — two independent passes cannot express that coupling).
  • Findings arrived within about five minutes of each push (measured from commit timestamps against review-comment timestamps), so every fix round immediately produced the next.
  • A seventh round then found a defect in the ambiguity-suppression mechanism itself, which had been added to stop the loop.

The root cause was never any individual construct. It was that check 21 had no stated parsing contract, so every construct CommonMark permits was implicitly in scope and the review surface was unbounded. #1096 addressed that half by stating the claim and making ambiguity decline rather than guess. This issue is the remaining half.

Direction

Narrow what check 21 claims to parse — do not widen what it models.

A real markdown parser was rejected deliberately: check-skill.sh must run on a contributor's machine with nothing but a POSIX shell and awk, so taking a Node or markdown-parser dependency to make an authoring gate pedantically conformant is the wrong trade. Candidate directions, not a decided design:

  • Recognise declarations only outside any code context, and stop attempting to reconstruct CommonMark inline pairing (spans, escapes, cross-line carries) in awk.
  • Consider whether the hard-FAIL verdicts (DIRECTIVE_MALFORMED, DIRECTIVE_NOREASON) need markdown structure awareness at all, or whether directive syntax can be validated on a narrower, structurally simpler basis.
  • Re-evaluate whether any construct in the contract's Not attempted list is worth modelling, versus documenting permanently.

Constraint that makes the naive fix wrong

fresh-eyes-exempt directives must stay visible to classification, because a directive IS an HTML comment. Any narrowing that strips or ignores comments wholesale destroys Form 2. The shipped single-line comment strip runs after directive classification for exactly this reason, and any replacement must preserve that ordering property.

Named gaps in the contract

Each degrades in the safe direction — an unmodelled construct causes a directive to be skipped, so the residual is a missed nudge, never a wrongly-failed author:

  • Indented code blocks — a four-space-indented line is either indented code or a list-item continuation, and separating them needs a block parser. Handled by the ambiguity rule: the hard verdicts and the stale WARN are withheld, and such a directive cannot satisfy a judgment step. Pre-existing gap, declined rather than approximated, because guessing would silently drop declarations inside nested lists.
  • Mixed container stacks — only blockquote depth and a single list-marker column are tracked, so a fence opened at > - ~~~markdown is not released when the inner list ends while the quote continues.
  • Paragraph-interrupting block constructs — a pending cross-line span carry expires at a blank line or a fence, but not at an ATX heading, thematic break, or table. Concrete shape: an unmatched backtick, then ## Heading with no blank line between, then a directive.
  • Multi-line HTML comment state — comment state is not carried across lines, so a comment body on a later line is read as prose. Tracked as a separate defect below.
  • Reference definitions, HTML blocks, setext headings, and link/image syntax are not interpreted at all.

Already fixed in #1096 — NOT residual gaps

Two items that were raised as findings landed as fixes rather than documented gaps, so they are recorded here only to prevent them being re-opened as open work:

  • Frontmatter fence tracking — fixed. YAML frontmatter is skipped rather than parsed as markdown, and every structural carry resets at its closing ---. Previously a block-scalar description carrying an example fence opened a fence the closing --- never ended, suppressing the entire body so the file passed silently.
  • Boundary after the directive name — fixed. The name must be followed by :, whitespace, or -->, so an ordinary comment about a longer identifier (<!-- fresh-eyes-exemption is explained here -->) is no longer read as a directive. Previously that was a false-positive hard FAIL on prose.

Separate defect, tracked here for the follow-up

Multi-line HTML comment state contradicts the contract's own Form 1 requirement that delegation wording be visible prose: hidden wording inside a multi-line comment still satisfies the detector, even though the single-line case is now stripped. Concrete shape — <!--, then dispatch this to a fresh-context agent, then --> across three lines, with a nearby Self-review your own work. reporting HIT_WORDING instead of HIT_NONE.

Bounded fix: carry comment state through the closing -->, keeping fresh-eyes-exempt directives visible to classification per the constraint above. It should land regardless of how the narrowing decision goes. It did not make it into #1096 before merge.

Reference

Every finding, its reproduction, and the reasoning for each accepted gap are on #1096.

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

    needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.priority: mediumReal value, no hard deadline; normal backlog flow.work-class: structuralRefactors, migrations, contract changes; cross-cutting and hard to reverse.

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions