Skip to content

fix(api): GET /specs/:id returns paragraph tree — wire getSpecTree into the read path#158

Merged
thewrz merged 1 commit into
mainfrom
feat/issue-152
Jun 11, 2026
Merged

fix(api): GET /specs/:id returns paragraph tree — wire getSpecTree into the read path#158
thewrz merged 1 commit into
mainfrom
feat/issue-152

Conversation

@thewrz

@thewrz thewrz commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Closes #152

Summary

  • GET /specs/:id was documented (README + OpenAPI) as returning the spec with its paragraph tree, but getSpecHandler called findSpecById, which hardcodes parts: []. The handler now calls getSpecTree (the same reconstruction already used by POST /specs/:id/generate, MCP get_spec, and the mockup tree endpoint) and returns result.tree.
  • 400/404/500 semantics unchanged: null result → 404 spec not found, thrown error → 500 with logger.error.
  • Regression coverage: unit test asserts the handler returns the reconstructed tree (non-empty parts), and a new integration test inserts real paragraph rows (part + child article) and asserts the reconstructed shape — regression #152: parsed spec returns reconstructed paragraph tree, not parts: [].

Design decisions

  • Wire the tree in, not docs-downgrade. README and openapi.yaml already promise the tree, and three other read paths (generate, MCP, mockup) already reconstruct it. Matching the code to the documented contract is the minimal-surprise fix; no doc changes needed.
  • Response data = SpecTree only — references intentionally excluded. getSpecTree returns { tree, references }, but the OpenAPI contract for this endpoint is data: SpecTree. Including references would change the documented schema (and require openapi.yaml edits, which a parallel PR for feat(parser): surface DOCX inference conflicts via get_paragraph MCP tool #56 owns). References remain available via the generate/MCP paths; exposing them on GET can be a follow-up if wanted.
  • findSpecById left in place. It now has no non-test caller, but removing it would touch src/db/queries/specs.ts + src/db/index.ts — files the parallel feat(parser): surface DOCX inference conflicts via get_paragraph MCP tool #56 work is editing. This PR makes zero changes to those files (and to openapi.yaml/README) to avoid merge conflicts. Dead-code removal is a follow-up candidate.

Test Plan

docker run -d --name specr-pg-152 -p 5442:5432 -e POSTGRES_USER=specr -e POSTGRES_PASSWORD=specr -e POSTGRES_DB=specr postgres:16
DATABASE_URL=postgres://specr:specr@localhost:5442/specr pnpm migrate
DATABASE_URL=postgres://specr:specr@localhost:5442/specr NODE_ENV=test pnpm seed
pnpm lint && pnpm test
DATABASE_URL=postgres://specr:specr@localhost:5442/specr NODE_ENV=test pnpm test:integration
docker rm -f specr-pg-152
  • pnpm lint clean (eslint + tsc --noEmit + prettier)
  • Unit: 655/655 pass (TDD: regression test failed RED before the handler change — 500 from missing mock, then GREEN)
  • Integration: 192 pass / 110 skipped (regression test failed RED with parts.length 0 ≠ 1 before the fix)
  • Diff scope: exactly 3 files (src/api/specs.ts, src/api/specs.test.ts, src/api/specs.integration.test.ts) — no edits to src/db/queries/specs.ts, src/db/index.ts, openapi.yaml, or README (coordination with feat(parser): surface DOCX inference conflicts via get_paragraph MCP tool #56)

Out of scope

  • references in the GET response, findSpecById removal, openapi.yaml/README changes, pagination/perf work.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed spec API to properly reconstruct and return paragraph hierarchies in spec details, resolving an issue where part relationships were not displayed.
  • Tests

    • Enhanced test coverage for spec retrieval to validate correct paragraph tree structure.

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f4ee5cd7-116b-4285-9724-c200c045c063

📥 Commits

Reviewing files that changed from the base of the PR and between 449fb48 and be57ebb.

📒 Files selected for processing (3)
  • src/api/specs.integration.test.ts
  • src/api/specs.test.ts
  • src/api/specs.ts

📝 Walkthrough

Walkthrough

The PR fixes issue #152 by switching GET /specs/:id from findSpecById (which returns empty parts) to getSpecTree (which reconstructs the paragraph tree). The handler, unit tests, and integration tests are all updated to use the new database function and verify tree reconstruction.

Changes

Tree Reconstruction Fix

Layer / File(s) Summary
Handler tree reconstruction via getSpecTree
src/api/specs.ts
getSpecHandler now calls getSpecTree(id) instead of findSpecById(id) and returns result.tree in the response, ensuring the reconstructed paragraph tree (with children) is returned instead of an empty parts array.
Unit test updates for getSpecTree
src/api/specs.test.ts
getSpecHandler unit tests are rewritten to mock getSpecTree with reconstructed tree payloads; success, not-found, and error cases now use the new DB function. The success case asserts data.parts has length 1.
Integration test data and regression test for #152
src/api/specs.integration.test.ts
Test setup inserts a part paragraph and a child paragraph; a new regression test validates GET /specs/:id returns reconstructed tree with children rather than returning an empty parts array.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • wrzonance/SpecR#78: Internal DB migration that introduced getSpecTree and exposed reconstructed SpecTree objects, which this PR now leverages in the GET handler.

Poem

🐰 A tree once lost in empty arrays,
Now blooms with children, no delays!
getSpecTree brings parts alive,
From root to branch, all parts thrive. 🌳

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and specifically describes the main change: wiring getSpecTree into the GET /specs/:id handler to return a paragraph tree, which directly addresses issue #152.
Linked Issues check ✅ Passed The PR fully addresses #152 by wiring getSpecTree into the GET read path, ensuring /specs/:id returns a reconstructed paragraph tree instead of empty parts, and adds regression tests validating non-empty parts.
Out of Scope Changes check ✅ Passed All changes are directly in scope: handler implementation (getSpecTree wiring), unit tests (handler assertions), and integration tests (regression test for #152). Explicitly excluded are OpenAPI/README docs, findSpecById removal, and database file edits.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/issue-152

Comment @coderabbitai help to get the list of available commands and usage tips.

@thewrz thewrz merged commit 44e8abc into main Jun 11, 2026
5 checks passed
@thewrz thewrz deleted the feat/issue-152 branch June 11, 2026 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(api): GET /specs/:id returns parts: [] — tree reconstruction never wired into findSpecById

1 participant