feat(reporting): per-paragraph hierarchy-scoring report — endpoint + MCP tool + demo Scoring tab (WS2) - #425
Merged
Merged
Conversation
Brainstormed design for #424 — the "report before the gate": a stored spec's per-paragraph hierarchy-inference confidence becomes retrievable (REST + MCP) and browsable (demo two-pane "Scoring" tab), reading what getSpec already derives (meta.inference, ADR-055). Decisions: server-side shared/tested capability + demo view; all scored paragraphs worst-first; rich human-readable entries; two-pane triage + jump. DRY via one shared walk (summarizeHierarchy refactored to consume it, output shape unchanged). Sole risk pinned: label non-drift vs renderMarkdown. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
4-task TDD plan for #424: scoring module (buildHierarchyReport) → REST endpoint + openapi → MCP tool → demo Scoring tab. Single PR. Refines the spec's DRY mechanism: summarizeHierarchy stays untouched (its walk is a flat tally-recurse; the report's must be ordinal-aware for labels — merging them is the parameterized-knot the DRY rule warns against, and it touches a shipped contract). Consistency is pinned by a counts-equivalence invariant test instead. External contract unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rst-first) Adds the pure scoring module for WS2 (#424): buildHierarchyReport() turns a SpecTree into a per-paragraph hierarchy-inference report, worst-confidence first, with the same CSI labels renderMarkdown emits. The label walk mirrors markdown.ts's renderRoot/renderPart/renderArticle/renderPrNode chain (position-based labeling, ordinal advanced only past consumesNumber siblings); the counting mirrors hierarchy-summary.ts's vanish-prune + skip discipline so the two independent implementations cannot silently drift (see the counts-equivalence test) without touching that module's exports. Also completes the generator barrel: renderMarkdown was only ever exported from generator/markdown.ts directly, not re-exported through generator/index.ts, forcing existing callers (fixture-snapshot.ts, mcp/resources.ts) to reach past the module boundary. Re-exporting it from the barrel lets this module's test (and future callers) respect the sibling-barrel-only import rule. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wraps buildHierarchyReport (WS2, #424) in a REST endpoint so callers can fetch the full per-paragraph hierarchy-inference scoring report, worst-confidence-first, for a spec. Mirrors getSpecLineageHandler's validate → fetch → 404/200 shape; documents the response as new HierarchyReport/ScoredParagraph component schemas in openapi.yaml, matching the existing HierarchySummary/SpecNodeInference conventions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the MCP counterpart to GET /specs/:id/hierarchy-report (WS2, issue #424), mirroring the coordination_report template: a dedicated handler+registration file, a read-tier capability, and a contract-map entry so INV-1/2/3 route<->tool parity holds. The handler validates its own specId shape (z.uuid()) before touching the DB, so a syntactically invalid id short-circuits distinctly from a not-found spec — matching the REST handler's z.uuid().safeParse gate. Never throws; failures surface as { isError: true }. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the demo's Scoring tab, the final piece of WS2 (#424): a two-pane view (worst-first paragraph list left, spec render right) over GET /specs/:id/hierarchy-report, mirroring js/audit.js's expandAncestors / createHoverWalker / click-to-jump machinery so paragraph confidence review gets the same UX the coordination Report tab already has. The filter/sort logic (all / <50% / <60% / document-order) lives in a new pure js/scoring-filter.mjs, unit-tested in scoring.test.mjs (mirrors compare-filter.test.mjs's model-only style) so the row-selection behavior is verified without a DOM. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two non-blocking findings from the whole-branch review: - Drop the vestigial `depth` field from the 'pr' LabelCtx variant — computeLabel never reads it (pr labels use index + node type only); the article/pr childContext branches collapse to one factory. - Document the invariant behind the NON_STRUCTURAL recurse: passing ctx unchanged is safe only because parsers emit note/continuation with children:[] and 'spec' never appears in tree.parts, so no structural child inherits the constant ctx (which would collide labels). No behavior change; 7/7 hierarchy-report tests green, tsc + eslint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a shared hierarchy-scoring report module, exposes it through a new REST endpoint and MCP tool, and wires a new Scoring view into the web demo. Supporting docs, OpenAPI contracts, and tests are added across the feature path. ChangesHierarchy scoring report feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ScoringJS
participant ApiJS
participant RestEndpoint
participant HierarchyReportModule
User->>ScoringJS: select spec / filter
ScoringJS->>ApiJS: getHierarchyReport(specId)
ApiJS->>RestEndpoint: GET /specs/:id/hierarchy-report
RestEndpoint->>HierarchyReportModule: buildHierarchyReport(tree, source)
HierarchyReportModule-->>RestEndpoint: HierarchyReport
RestEndpoint-->>ApiJS: HierarchyReport JSON
ApiJS-->>ScoringJS: report data
ScoringJS->>ScoringJS: selectScoringRows(paragraphs, filter)
ScoringJS-->>User: render rows + spec pane
User->>ScoringJS: click row
ScoringJS->>ScoringJS: locateNode / pulseNode
sequenceDiagram
participant MCPClient
participant ToolRegistry
participant handleGetHierarchyReport
participant DB
participant HierarchyReportModule
MCPClient->>ToolRegistry: call get_hierarchy_report(specId)
ToolRegistry->>handleGetHierarchyReport: dispatch
handleGetHierarchyReport->>handleGetHierarchyReport: validate specId (Zod)
handleGetHierarchyReport->>DB: getSpecTree(specId)
handleGetHierarchyReport->>DB: getSpecSource(specId)
handleGetHierarchyReport->>HierarchyReportModule: buildHierarchyReport(tree, source)
HierarchyReportModule-->>handleGetHierarchyReport: HierarchyReport
handleGetHierarchyReport-->>MCPClient: ToolResult (ok or isError)
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
thewrz
marked this pull request as ready for review
July 8, 2026 22:16
…verage
CI's contract gate (contract.integration.test.ts) requires every documented
success-JSON op to have a live response assertion OR a RESPONSE_ALLOWLIST
entry. The new hierarchy-report route had neither. Every sibling /specs/{}/*
GET (incl. get /specs/{} and get /specs/{}/lineage — the route this handler
mirrors) lives in RESPONSE_ALLOWLIST, not RESPONSE_COVERED, so this follows
that convention. Only surfaced in CI — this invariant is DB-gated and could
not run in the local (no-DATABASE_URL) sandbox.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…or-state consistency Fixes three bugs a code review found in the web-UI demo's Scoring tab: (1) clicking a scored part/article row silently failed to locate because read-only sheets never stamp [data-node-id] on heading bars — now falls back to the sheet head (mirroring audit.js's openSheet 3-tier degrade); (2) the clear-selection branch of loadSelected didn't bump the stale-fetch guard, letting an old in-flight request repopulate a pane that now has nothing selected; (3) a load failure only replaced the right pane, leaving the left rows/summary showing the previous spec while the picker and error message referred to the new (failed) one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Closes the same-class hazard as the cleared-selection fix: the "spec no longer loaded in the project" early-return in loadSelected returned without invalidating the request token, so an in-flight hierarchy-report response from a prior selection could still pass the stale guard and repopulate the pane. Now calls requestGuard.bump() before returning, matching the other early exits. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/lib/hierarchy-report.ts (1)
46-46: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueMinor asymmetry between
NON_STRUCTURALandconsumesNumberfor the'spec'type.
NON_STRUCTURALtreats'spec'as non-structural (skipped invisitNode), butconsumesNumber(imported from../ast/index.js) only excludes'note'/'continuation'/vanish — it does not exclude'spec'. If a'spec'node were ever encountered as a sibling inwalkSiblings, it would be skipped for labeling/tallying purposes but would still incorrectly advanceordinalon line 143. This is currently inert only because of the parser invariant already called out in the comment abovevisitNode("'spec' never appears in tree.parts"), but the two sets diverging is a latent inconsistency if that invariant is ever relaxed.♻️ Optional defensive tightening
function walkSiblings( nodes: readonly SpecNode[], ctxAt: (index: number) => LabelCtx, acc: Acc, threshold: number ): void { let ordinal = 0; for (const node of nodes) { visitNode(node, ctxAt(ordinal), acc, threshold); - if (consumesNumber(node)) ordinal += 1; + // consumesNumber() doesn't exclude 'spec'; guard explicitly so a future + // parser relaxation can't silently shift labels via a non-structural sibling. + if (consumesNumber(node) && !NON_STRUCTURAL.has(node.type)) ordinal += 1; } }Also applies to: 131-145
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/hierarchy-report.ts` at line 46, The handling of `'spec'` is inconsistent between `NON_STRUCTURAL` and `consumesNumber`, which can let `walkSiblings` advance `ordinal` for nodes that `visitNode` skips. Update the sibling-numbering logic in `hierarchy-report` so the same node types are treated as non-structural everywhere, using the existing `NON_STRUCTURAL`/`consumesNumber` path around `visitNode`, `walkSiblings`, and the ordinal increment. Keep the parser invariant comment intact, but make the numbering behavior defensive against future `'spec'` siblings.openapi.yaml (1)
5197-5242: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider composing
ScoredParagraphfromSpecNodeInferenceviaallOf.
confidence,signalUsed,agreed, andevidencehere are copy-pasted verbatim (including descriptions/enums) fromSpecNodeInference(Lines 5167-5196). AnallOf: [{ $ref: '#/components/schemas/SpecNodeInference' }, { type: object, properties: { nodeId, nodeType, ilvl, label, preview, conflicts } } ]would remove the duplication and prevent future drift between the two schemas.Given the PR's stated intent to keep this report loosely coupled from existing summary shapes, this is optional — flagging only as a maintainability improvement.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openapi.yaml` around lines 5197 - 5242, ScoredParagraph duplicates several inference fields from SpecNodeInference, so update the schema to compose from SpecNodeInference using allOf and keep only the paragraph-specific properties in ScoredParagraph. Locate the ScoredParagraph schema and move confidence, signalUsed, agreed, evidence, and any shared inference metadata into the shared SpecNodeInference reference, leaving nodeId, nodeType, ilvl, label, preview, and conflicts as the report-specific additions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/specs/2026-07-08-hierarchy-scoring-report-design.md`:
- Around line 44-61: Update the design doc to match the locked implementation
plan by removing references to refactoring summarizeHierarchy or having it
consume walkScored, and adjust the architecture diagram, DRY section, and file
map so they only describe the new hierarchy-report path. Use the existing
symbols buildHierarchyReport, walkScored, summarizeHierarchy, and
src/lib/hierarchy-report.ts to locate the stale references, and ensure the text
reflects that summarizeHierarchy remains unchanged while the cohort only adds
the new report module and generator wiring.
In `@examples/web_ui_demo/js/scoring.js`:
- Around line 192-210: The .scoring-row items in makeRow are focusable but still
lack keyboard activation for Enter/Space, since createHoverWalker only covers
navigation keys and Escape. Update the scoring row behavior so a focused row
triggers locateRow on Enter and Space, either by adding a key handler in makeRow
or by using a button-like control with the same row styling and existing
dataset.nodeId hookup.
---
Nitpick comments:
In `@openapi.yaml`:
- Around line 5197-5242: ScoredParagraph duplicates several inference fields
from SpecNodeInference, so update the schema to compose from SpecNodeInference
using allOf and keep only the paragraph-specific properties in ScoredParagraph.
Locate the ScoredParagraph schema and move confidence, signalUsed, agreed,
evidence, and any shared inference metadata into the shared SpecNodeInference
reference, leaving nodeId, nodeType, ilvl, label, preview, and conflicts as the
report-specific additions.
In `@src/lib/hierarchy-report.ts`:
- Line 46: The handling of `'spec'` is inconsistent between `NON_STRUCTURAL` and
`consumesNumber`, which can let `walkSiblings` advance `ordinal` for nodes that
`visitNode` skips. Update the sibling-numbering logic in `hierarchy-report` so
the same node types are treated as non-structural everywhere, using the existing
`NON_STRUCTURAL`/`consumesNumber` path around `visitNode`, `walkSiblings`, and
the ordinal increment. Keep the parser invariant comment intact, but make the
numbering behavior defensive against future `'spec'` siblings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8bb9eb72-12f7-42dc-8b9f-9aefee51d8f6
📒 Files selected for processing (22)
docs/superpowers/plans/2026-07-08-hierarchy-scoring-report.mddocs/superpowers/specs/2026-07-08-hierarchy-scoring-report-design.mdexamples/web_ui_demo/css/app.cssexamples/web_ui_demo/index.htmlexamples/web_ui_demo/js/api.jsexamples/web_ui_demo/js/app.jsexamples/web_ui_demo/js/scoring-filter.mjsexamples/web_ui_demo/js/scoring.jsexamples/web_ui_demo/scoring.test.mjsopenapi.yamlsrc/api/contract.integration.test.tssrc/api/router.tssrc/api/specs.test.tssrc/api/specs.tssrc/generator/index.tssrc/lib/hierarchy-report.test.tssrc/lib/hierarchy-report.tssrc/mcp/capabilities.tssrc/mcp/contract-map.tssrc/mcp/hierarchy-report-tools.test.tssrc/mcp/hierarchy-report-tools.tssrc/mcp/tools.ts
…ard, doc sync Two actionable + one nitpick from CodeRabbit on PR #425 (the openapi allOf nitpick is intentionally skipped — the report shape is deliberately decoupled and openapi is already accurate): - scoring.js: rows are tabIndex=0 but createHoverWalker only binds Arrow/Escape, so a keyboard user who Tabs onto a row couldn't activate it. Add an Enter/Space keydown delegation mirroring the click one (preventDefault stops Space-scroll). - hierarchy-report.ts: consumesNumber does not exclude 'spec', so a 'spec' sibling would advance the CSI ordinal while visitNode skips it. Guard the ordinal advance with !NON_STRUCTURAL — inert today ('spec' never in tree.parts) but keeps the advance-set identical to the skip-set. No real-tree label change (7/7 label/counts tests still green). - design doc: sync the architecture diagram, DRY section, and file map to the locked decision (summarizeHierarchy left untouched; counts-equivalence test), which the plan already reversed — a future reader was being misled. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
CodeRabbit triage (2 actionable + 2 nitpicks) — resolved in
|
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Pressure-phase prep (WS2 of the robustness program; WS1 = #423, merged). Before WS3 automates a gold-corpus pass/fail gate, a human needs to eyeball inference quality: a worst-first, filterable view of every scored paragraph's hierarchy-inference confidence for a stored spec — the "report before the gate." Closes #424.
What
A stored spec's per-paragraph confidence becomes retrievable (REST + MCP) and browsable (demo), reading what
getSpecalready derives (meta.inference, ADR-055) — no change to the inference engine, parser, worker, or DB.src/lib/hierarchy-report.ts—buildHierarchyReport(tree, source, threshold?): all scored structural paragraphs, worst-first, rich (labelvia canonicalgetLabel,preview,confidence,signalUsed/agreed,evidence,conflicts?) + counts +unscoredReason.GET /specs/:id/hierarchy-report—ApiResponse<HierarchyReport>(400/404/500), documented inopenapi.yaml.get_hierarchy_report— read tier, contract-bound (ADR-044), never throws.all · <50% · <60% · document-order) beside the spec, click-to-jump (reusesaudit.js'sexpandAncestors/hover-walker).Design decisions
summarizeHierarchyinto one shared walk; that module is instead left untouched (its flat tally-recurse differs from the report's ordinal-aware label walk — merging = a parameterized knot + touches a shipped contract). Consistency is pinned by a counts-equivalence invariant test. Same external behavior.ScoredParagraph.labelequalsrenderMarkdown's label for that node.Testing
unscoredReasonheader) — the one unexercised gate (needs the backend + Postgres running)Deferred (non-blocking, reviewer-triaged)
Demo polish for a follow-up: a
locateRowfallback when a nodeId isn't in the sheet (mirroraudit.js's 3-tier), and a filter-switch left/right selection desync.🤖 Co-authored by Claude Opus 4.8 (1M context). Closes #424.
Summary by CodeRabbit