feat: generated OpenAPI, /docs page, FAQ, and Swagger UI#968
feat: generated OpenAPI, /docs page, FAQ, and Swagger UI#968northdpole wants to merge 4 commits into
Conversation
|
Warning Review limit reached
Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (25)
WalkthroughThis PR adds a public OpenAPI registry/schema module, decorates existing REST routes, regenerates and commits docs/api/openapi.yaml, and adds a guardrail script/test enforcing spec freshness and route coverage, wired into CI and Makefile lint. It also adds a frontend Docs page with FAQ rendering and navigation links, plus a docs/faq.md. ChangesOpenAPI Documentation and Docs Page
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
application/frontend/src/components/MarkdownFromRepo/MarkdownFromRepo.tsx (2)
61-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winError fallback link is hardcoded to
docs/faq.md.The component accepts a generic
srcprop, but the error-state GitHub link is hardcoded todocs/faq.md. If this component is reused for other markdown sources, the error link will be incorrect. Consider accepting an optionalerrorHrefprop or deriving the link fromsrc.♻️ Proposed refactor to make error link configurable
interface MarkdownFromRepoProps { src: string; className?: string; + errorHref?: string; } -export const MarkdownFromRepo = ({ src, className = '' }: MarkdownFromRepoProps) => { +export const MarkdownFromRepo = ({ src, className = '', errorHref }: MarkdownFromRepoProps) => {And in the error JSX:
- <a href="https://github.com/OWASP/OpenCRE/blob/main/docs/faq.md" target="_blank" rel="noreferrer"> + <a href={errorHref ?? `https://github.com/OWASP/OpenCRE/blob/main/${src.replace(/^\//, '')}`} target="_blank" rel="noreferrer">🤖 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 `@application/frontend/src/components/MarkdownFromRepo/MarkdownFromRepo.tsx` around lines 61 - 65, The error-state GitHub link in MarkdownFromRepo is hardcoded to docs/faq.md instead of using the component’s src input. Update MarkdownFromRepo.tsx so the fallback link is configurable, either by adding an optional errorHref prop or deriving the GitHub URL from src, and use that value in the error JSX where the source link is rendered.
22-50: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse
AbortControllerto cancel in-flight fetches on unmount and add a timeout.The
cancelledflag prevents state updates after unmount, but the network request continues consuming resources. AnAbortControllerwould cancel the fetch itself and allow adding a timeout so the component doesn't hang indefinitely on a slow server.♻️ Proposed refactor with AbortController and timeout
useEffect(() => { - let cancelled = false; + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 10_000); + setLoading(true); setError(null); - fetch(src) + fetch(src, { signal: controller.signal }) .then((res) => { if (!res.ok) { throw new Error(`Failed to load markdown (${res.status})`); } return res.text(); }) .then((markdown) => { - if (cancelled) return; const rendered = DOMPurify.sanitize(marked.parse(markdown), { USE_PROFILES: { html: true }, }); setHtml(rendered); }) .catch((err: Error) => { - if (!cancelled) { + if (err.name !== 'AbortError') { setError(err.message); } }) .finally(() => { - if (!cancelled) { + clearTimeout(timeout); + // Only update if not aborted + if (!controller.signal.aborted) { setLoading(false); } }); return () => { - cancelled = true; + controller.abort(); + clearTimeout(timeout); }; }, [src]);🤖 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 `@application/frontend/src/components/MarkdownFromRepo/MarkdownFromRepo.tsx` around lines 22 - 50, The MarkdownFromRepo effect currently uses a cancelled flag in the fetch/src workflow, but the request itself is never aborted. Update the fetch logic in MarkdownFromRepo.tsx to use an AbortController so cleanup in the effect aborts the in-flight request, and add a timeout-based abort path to prevent hanging on slow servers. Make sure the existing error/loading handling around setError, setHtml, and setLoading continues to ignore abort-related cancellations while preserving normal failure behavior.application/web/web_main.py (1)
662-672: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFAQ endpoint path resolution is fragile but functional.
The
../../docsrelative path fromapplication/web/works for the current layout. Ifweb_main.pyis ever moved, this will silently break. Consider using a project-root constant orapp.root_pathfor robustness. Not blocking.🤖 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 `@application/web/web_main.py` around lines 662 - 672, The faq_markdown route currently resolves the docs folder with a fragile relative path, so update the path construction in faq_markdown to use a stable base such as app.root_path or an existing project-root constant instead of os.path.dirname(os.path.realpath(__file__)). Keep the route behavior the same, but make the send_from_directory directory argument resilient to future file moves by locating the docs directory from a project-level anchor.application/tests/openapi_guardrail_test.py (1)
24-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove
generate_openapi.main()call to avoid masking stale specs.The test regenerates the committed spec before running the guardrail, which means a stale spec will always pass. The test should verify the committed spec is already fresh. Additionally,
generate_openapi.main()writes todocs/api/openapi.yamlon disk, leaving the repo dirty if output differs.♻️ Proposed refactor
def test_guardrail_script_passes(self) -> None: import scripts.check_openapi_guardrail as guardrail - # Ensure committed spec matches generator output before guardrail run. - import scripts.generate_openapi as generate_openapi - - generate_openapi.main() + # The committed spec must already be fresh. + # Run `make openapi-generate` if this test fails. self.assertEqual(guardrail.main(), 0)🤖 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 `@application/tests/openapi_guardrail_test.py` around lines 24 - 31, The guardrail test currently calls generate_openapi.main() inside test_guardrail_script_passes, which masks stale committed specs and can dirty docs/api/openapi.yaml during test execution. Remove that regeneration step from the test and have test_guardrail_script_passes only invoke scripts.check_openapi_guardrail.main() after importing scripts.check_openapi_guardrail, so the test verifies the committed OpenAPI spec is already up to date.
🤖 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 `@application/frontend/src/components/MarkdownFromRepo/MarkdownFromRepo.tsx`:
- Around line 31-33: The MarkdownFromRepo render path is passing the potentially
async return type from marked.parse() directly into DOMPurify.sanitize(), which
can break typing. Update the MarkdownFromRepo component’s render logic so
marked.parse(markdown) is explicitly synchronous (for example by using the
async=false option or by narrowing the result to string before sanitize), then
keep the sanitized output flowing through DOMPurify.sanitize() as before.
In `@application/web/openapi_registry.py`:
- Around line 373-412: The _operation_from_path helper is adding query-schema
parameters on top of the path parameters without de-duplicating names, which
causes the same field to appear as both in: path and in: query. Update the
parameter assembly in _operation_from_path to track the names from
_path_parameters(path_spec.path) and filter
_marshmallow_query_parameters(path_spec.query_schema) so any query field already
used by the path is skipped before assigning operation["parameters"].
In `@docs/api/openapi.yaml`:
- Around line 201-230: The `standard` query parameter in `map_analysis` is
documented as exactly two names, but its array schema in `openapi.yaml` does not
enforce that contract. Update the `standard` parameter schema to add both
`minItems: 2` and `maxItems: 2` so the OpenAPI definition matches the intended
behavior and client validation can catch invalid request sizes.
- Around line 935-947: The NodeListResponseSchema definition leaves page untyped
with page: {}, which allows any value instead of a page number. Update the
NodeListResponseSchema properties so page is explicitly declared as type:
integer, matching total_pages and the intended numeric pagination shape.
In `@scripts/check_openapi_guardrail.py`:
- Around line 84-91: The guardrail flow in check_openapi_guardrail.py still
opens COMMITTED_SPEC after _check_freshness can report a missing file, causing
an unhandled FileNotFoundError. Update the main checks sequence so the committed
spec is only opened after confirming it exists, or gate the yaml.safe_load/open
call behind a file-existence check and turn the missing-spec case into a normal
failed check. Keep the behavior in _check_freshness, _check_validity, and
_check_route_coverage consistent so all failures are collected and reported
without crashing.
---
Nitpick comments:
In `@application/frontend/src/components/MarkdownFromRepo/MarkdownFromRepo.tsx`:
- Around line 61-65: The error-state GitHub link in MarkdownFromRepo is
hardcoded to docs/faq.md instead of using the component’s src input. Update
MarkdownFromRepo.tsx so the fallback link is configurable, either by adding an
optional errorHref prop or deriving the GitHub URL from src, and use that value
in the error JSX where the source link is rendered.
- Around line 22-50: The MarkdownFromRepo effect currently uses a cancelled flag
in the fetch/src workflow, but the request itself is never aborted. Update the
fetch logic in MarkdownFromRepo.tsx to use an AbortController so cleanup in the
effect aborts the in-flight request, and add a timeout-based abort path to
prevent hanging on slow servers. Make sure the existing error/loading handling
around setError, setHtml, and setLoading continues to ignore abort-related
cancellations while preserving normal failure behavior.
In `@application/tests/openapi_guardrail_test.py`:
- Around line 24-31: The guardrail test currently calls generate_openapi.main()
inside test_guardrail_script_passes, which masks stale committed specs and can
dirty docs/api/openapi.yaml during test execution. Remove that regeneration step
from the test and have test_guardrail_script_passes only invoke
scripts.check_openapi_guardrail.main() after importing
scripts.check_openapi_guardrail, so the test verifies the committed OpenAPI spec
is already up to date.
In `@application/web/web_main.py`:
- Around line 662-672: The faq_markdown route currently resolves the docs folder
with a fragile relative path, so update the path construction in faq_markdown to
use a stable base such as app.root_path or an existing project-root constant
instead of os.path.dirname(os.path.realpath(__file__)). Keep the route behavior
the same, but make the send_from_directory directory argument resilient to
future file moves by locating the docs directory from a project-level anchor.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 9377036f-a66a-4acb-955a-70f4cf68bc23
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (25)
.github/workflows/linter.yml.gitignoreMakefileREADME.mdapplication/__init__.pyapplication/frontend/src/components/MarkdownFromRepo/MarkdownFromRepo.tsxapplication/frontend/src/components/MarkdownFromRepo/markdownFromRepo.scssapplication/frontend/src/const.tsapplication/frontend/src/pages/Docs/Docs.tsxapplication/frontend/src/pages/Docs/docs.scssapplication/frontend/src/pages/Search/Search.tsxapplication/frontend/src/routes.tsxapplication/frontend/src/scaffolding/Header/Header.tsxapplication/frontend/www/bundle.jsapplication/frontend/www/bundle.js.LICENSE.txtapplication/tests/openapi_guardrail_test.pyapplication/web/openapi_registry.pyapplication/web/openapi_schemas.pyapplication/web/web_main.pydocs/api/openapi.yamldocs/faq.mdpackage.jsonrequirements.txtscripts/check_openapi_guardrail.pyscripts/generate_openapi.py
Add Marshmallow schemas and a registry-driven OpenAPI generator for all public GET /rest/v1 routes, with make openapi-generate, make openapi-guardrail, CI enforcement, and @openapi_documented markers on handlers. Co-authored-by: Cursor <cursoragent@cursor.com>
Add docs/faq.md as the single FAQ source of truth, expose it via Flask, track it in git, and link it from the README. Co-authored-by: Cursor <cursoragent@cursor.com>
8bda6aa to
77b1e3a
Compare
Introduce the in-app documentation route with interactive API reference, FAQ rendering from /docs/faq.md, header navigation, and homepage footer links. Co-authored-by: Cursor <cursoragent@cursor.com>
77b1e3a to
c9ee70d
Compare
Apply existing swagger-client resolution (3.37.3) so frozen-lockfile install passes on CI Node 20 without requiring swagger-client >=22. Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
make openapi-generate,make openapi-guardrail, and CI enforcement (Docs: OpenAPI spec, dedicated docs page, API links, and homepage FAQ #961 OpenAPI slice).docs/faq.mdas the FAQ source of truth, served at/docs/faq.mdand linked from the README./docspage with Swagger UI, FAQ rendering, header nav, and homepage footer links (Docs: OpenAPI spec, dedicated docs page, API links, and homepage FAQ #961 docs slice).Commits
feat(openapi): generate public REST spec with deterministic guardraildocs: add FAQ markdown and serve from /docs/faq.mdfeat(frontend): add /docs page with Swagger UI and nav linksTest plan
make openapi-guardrailpython -m unittest application.tests.openapi_guardrail_testpython -m unittest application.tests.web_main_testmake frontend/docs,/docs#api-reference,/docs#faq/docsand/docs#api-referenceCloses #961
Made with Cursor