TimZander / TimZander/claude

deep-review: emit structured findings as the canonical intermediate form (with stable fingerprints)

Open
#218 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
Dominant language
Python
Stars
0
Forks
1
Avg merge
1d 3h
Merged PRs (30d)
7

Description

Part of the deep-review: delivery rework milestone. Design context: #217.

Problem

/deep-review produces exactly one thing: prose formatted for a terminal reader (commands/deep-review.md:231-280). Every other delivery we want — a short PR comment, inline anchored comments, a multi-PR triage table, a delta against the last round — has to re-derive structure by re-parsing that prose.

That's backwards, and it's fragile: the same skill already forces a rigid line format (emoji + backtick file:line + em dash + description) precisely because something downstream needs to parse it. We're one regex away from structured data and pretending we aren't.

There is also no identity for a finding. Run the review twice on the same PR and there is no way to say "this is the same finding as last time" — which is why delta re-review (#222) can't exist yet.

Proposal

Make the analysis produce a structured finding set once, and treat every human-readable output as a rendering of it.

Sketch of a finding record:

severity      CRITICAL | WARNING | SUGGESTION | PRAISE
file          path/to/file.ts
line          42
description   one-line human text
rationale     why it matters (optional, longer)
step          which review step produced it (feature-fitness, security, tests, …)
blocking      true | false          # can this merge without it?
fingerprint   stable hash

Fingerprint is the interesting field. It must survive the author editing surrounding code — so hashing file:line is wrong (line numbers shift). Candidates to evaluate: hash of (normalized file path, symbol/function name, severity, normalized description), or a content hash of the offending line with whitespace and identifiers normalized. Getting this ~80% right is enough; a missed match degrades to "reported as new," which is survivable.

Open questions

  • Where does it live? A JSON block the skill emits alongside the prose? A file written to the scratchpad? Embedded in the posted PR comment as an HTML comment so the state travels with the PR (#217 §5C)? The last option is attractive — no external store, and any agent that fetches the comment gets the full set.
  • Who assembles it? The four Step 2 subagents already emit SEVERITY|file:line|description lines. Tightening that to the full record and having the orchestrator merge/dedupe is a small change to an existing contract, not a new mechanism.
  • Does blocking come from severity, or is it independent? A 🟡 that blocks and a 🔴 that doesn't are both real. #217 §4 argues the PR channel should gate on blocking, not on severity — if so, this field carries the gating decision and needs to be assigned deliberately, not derived.
  • Interaction with #211: if a refute-pass reassigns severity, it must reassign it on the record, not on the prose.

Depends on

Blocked on the delivery-model decision in #217 §6.1/§6.2 — specifically whether we render from a shared intermediate form at all, or keep separate skills that each do their own analysis.

Acceptance criteria (provisional — pending #217)

  1. A single review run produces a structured finding set covering every finding in the prose output — no finding appears in one and not the other.
  2. Each finding carries a fingerprint that is stable across an unrelated edit elsewhere in the same file.
  3. The existing terminal output is generated from the structured set and is unchanged in appearance.
  4. The format is documented well enough that a second skill (#163, #164) can consume it without reading deep-review's internals.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with #217 §6.1/§6.2, since this work is blocked on the delivery-model decision. Then inspect commands/deep-review.md:231-280 and the existing Step 2 subagent output contract. Done means one structured finding set covers the prose, fingerprints survive unrelated edits, terminal output remains unchanged, and the format is documented for other skills.

Written by the indexing model from the issue text.

Assessment

Domain
tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.