Docs/api reference and contributing#44
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces comprehensive API reference and architecture documentation for the claude-code-chat-browser system, adds a contributing guide, updates the README to cross-link new docs, makes a small session exclusion timing adjustment, and tightens frontend and search validation tests. ChangesDocumentation, tests, and supporting code for API contract and architecture
🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
101-125:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSpecify a language for the project-tree code fence.
Line 101 starts a fenced block without a language, which trips markdownlint MD040. Mark it as plain text.
Suggested fix
-``` +```text claude-code-chat-browser/ ├── app.py # Flask entry point (default port 5000) ... └── tests/ -``` +```🤖 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 `@README.md` around lines 101 - 125, The README.md project's tree fenced code block starting at the block beginning (the triple backticks before "claude-code-chat-browser/") needs an explicit language to satisfy markdownlint MD040; change the opening fence from ``` to ```text so the block is marked as plain text (keep the closing ``` as-is). Locate the block that lists the repository tree (the one showing app.py, api/, docs/, etc.) and update only the opening fence to include "text".
🧹 Nitpick comments (1)
package.json (1)
9-13: ⚡ Quick winUpdate devDependency versions (currency); npm audit reports no vulnerabilities.
npm auditreturned zero vulnerabilities for the current dependency set.- Latest npm versions are:
vitest@4.1.7,@vitest/coverage-v8@4.1.7,jsdom@29.1.1(current:3.2.4,3.2.4,26.1.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 `@package.json` around lines 9 - 13, Update the devDependency versions in package.json by bumping the three keys "`@vitest/coverage-v8`", "vitest", and "jsdom" to their latest compatible versions (e.g., "`@vitest/coverage-v8`": "4.1.7", "vitest": "4.1.7", "jsdom": "29.1.1"); modify the values under the "devDependencies" object to those newer semver strings and then run npm install (or yarn) and npm audit/lockfile update to ensure the lockfile and dependency tree are consistent.
🤖 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 `@api/sessions.py`:
- Around line 65-77: The handler parses the session with parse_session and then
calls compute_stats without enforcing exclusion rules, allowing excluded
sessions to be queried; fix by checking exclusion immediately after
parse_session (before compute_stats) using the project’s session exclusion
predicate (e.g., is_session_excluded(session) or checking session.excluded /
session.metadata flags) and if excluded return an appropriate error_response
(e.g., ErrorCode.NOT_FOUND or a 403/404 with a clear message) instead of calling
compute_stats or jsonify.
In `@docs/api-reference.md`:
- Around line 19-20: The docs claim that every `/api/*` 4xx/5xx uses the
`{error, code}` envelope, but that contradicts the documented sessions-listing
400 response which returns `[]`; update the API reference to either (A) scope
the envelope guarantee to only the endpoints that actually return the structured
envelope (e.g., change "Every `/api/*`" to "Structured-error endpoints" or "Most
`/api/*` endpoints" and list which routes adhere), or (B) explicitly list
endpoint exceptions up front (call out `GET /api/sessions` or the "sessions
listing" endpoint and note its `[]` 400 body). Ensure you reference the envelope
shape `{error, code}` and the sessions-listing endpoint name when editing so the
guarantee and exceptions are unambiguous.
In `@docs/architecture.md`:
- Around line 7-44: The fenced ASCII diagram block uses plain triple backticks
without a language tag, triggering markdownlint MD040; update the opening fence
from ``` to ```text so the diagram is treated as plain-text (i.e., change the
diagram's starting triple-backtick fence to include the text language tag and
keep the closing fence unchanged) to silence the lint warning.
In `@tests/test_api_routes.py`:
- Around line 64-67: The test
test_project_sessions_invalid_path_returns_400_empty_list is asserting an empty
list for a 400 error; change it to validate the standardized error envelope
instead (use assert_error_response(resp, 400, code="INVALID_PATH") or
equivalent) so the response body matches other error tests, or alternatively fix
the route handler that serves /api/projects/.../sessions to return the error
object with code "INVALID_PATH" rather than an empty list; locate the test
function name and the client.get("/api/projects/../../outside/sessions") call to
update the assertion to assert_error_response.
In `@tests/test_cli_e2e.py`:
- Around line 23-36: The subprocess call in _run_cli can hang tests; add a
timeout to the subprocess.run invocation to bound execution time. Modify the
_run_cli function so the subprocess.run call (the call that uses cmd,
cwd=str(REPO_ROOT), capture_output=True, text=True, env=merged,
encoding="utf-8", errors="replace") includes a sensible timeout parameter (e.g.,
timeout=30 or configurable via an env var or test constant) and handle the
potential subprocess.TimeoutExpired in the test harness if needed; ensure the
change references the _run_cli function and the subprocess.run call so the CLI
invocation is always timeboxed.
In `@tests/test_search.py`:
- Around line 52-55: Update test_limit_whitespace_defaults to not only assert
200 but also verify that a whitespace-only limit falls back to the service
default: call the same endpoint with whitespace limit via
client_single.get("/api/search?q=Hello&limit=%20%20%20") and assert the response
JSON uses the default paging (for example compare len(resp.json()["items"]) ==
DEFAULT_LIMIT or compare this response to a call without any limit parameter),
referencing the test name test_limit_whitespace_defaults, the client_single
fixture and the "/api/search" endpoint to locate the test and DEFAULT_LIMIT (or
equivalent server default behavior) to validate the fallback.
- Line 25: The test currently uses _SEARCH_HIT_KEYS.issubset(item.keys()), which
permits extra unexpected fields; change the assertion to require exact key
equality by comparing the sets (e.g., set(item.keys()) == _SEARCH_HIT_KEYS or
item.keys() == _SEARCH_HIT_KEYS) so the test fails if any additional keys are
present and thereby prevents schema leakage regressions in the item produced by
the search.
---
Outside diff comments:
In `@README.md`:
- Around line 101-125: The README.md project's tree fenced code block starting
at the block beginning (the triple backticks before "claude-code-chat-browser/")
needs an explicit language to satisfy markdownlint MD040; change the opening
fence from ``` to ```text so the block is marked as plain text (keep the closing
``` as-is). Locate the block that lists the repository tree (the one showing
app.py, api/, docs/, etc.) and update only the opening fence to include "text".
---
Nitpick comments:
In `@package.json`:
- Around line 9-13: Update the devDependency versions in package.json by bumping
the three keys "`@vitest/coverage-v8`", "vitest", and "jsdom" to their latest
compatible versions (e.g., "`@vitest/coverage-v8`": "4.1.7", "vitest": "4.1.7",
"jsdom": "29.1.1"); modify the values under the "devDependencies" object to
those newer semver strings and then run npm install (or yarn) and npm
audit/lockfile update to ensure the lockfile and dependency tree are consistent.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e8d237d6-09f3-4ba4-bb8b-0299d52c78a3
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (28)
.github/workflows/ci.yml.gitignoreCONTRIBUTING.mdREADME.mdapi/error_codes.pyapi/export_api.pyapi/search.pyapi/sessions.pydocs/api-reference.mddocs/architecture.mdpackage.jsonpyproject.tomlrequirements-dev.txtstatic/js/shared/markdown.test.jsstatic/js/shared/state.test.jsstatic/js/shared/utils.test.jstests/conftest.pytests/fixtures/session_minimal.jsonltests/fixtures/session_with_thinking.jsonltests/fixtures/session_with_tools.jsonltests/test_api_integration.pytests/test_api_routes.pytests/test_cli_e2e.pytests/test_error_codes.pytests/test_error_propagation.pytests/test_export_api_bulk.pytests/test_search.pyvitest.config.js
timon0305
requested changes
May 22, 2026
clean6378-max-it
force-pushed
the
docs/api-reference-and-contributing
branch
from
c7cbc2c to
0d5a572
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
static/js/shared/markdown.test.js (1)
31-40:⚠️ Potential issue | 🟠 Major | ⚡ Quick winVerify DOMPurify.sanitize is actually invoked.
These tests only assert the output doesn't contain dangerous content, but they don't verify that
DOMPurify.sanitizeis actually called. If the implementation changes to bypass DOMPurify, these tests would still pass, giving false confidence about the security mechanism.Consider adding spies to verify the integration:
it('sanitizes script tags from parsed output', () => { const spy = vi.spyOn(DOMPurify, 'sanitize'); const html = renderMarkdown('# Hello\n\n<script>alert(1)</script>'); expect(spy).toHaveBeenCalled(); expect(html).not.toContain('<script'); });🤖 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 `@static/js/shared/markdown.test.js` around lines 31 - 40, Update the two tests that assert unsafe output removal to also assert DOMPurify.sanitize was invoked: wrap a spy around DOMPurify.sanitize (e.g., vi.spyOn(DOMPurify, 'sanitize')) before calling renderMarkdown, call renderMarkdown in the test, expect the spy toHaveBeenCalled() (optionally with the rendered HTML string), and then restore the spy; reference the existing test cases that call renderMarkdown('# Hello\n\n<script>alert(1)</script>') and renderMarkdown('<img src=x onerror=alert(1)>') so you verify both script-stripping and event-handler stripping paths actually use DOMPurify.sanitize.
🤖 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.
Duplicate comments:
In `@static/js/shared/markdown.test.js`:
- Around line 31-40: Update the two tests that assert unsafe output removal to
also assert DOMPurify.sanitize was invoked: wrap a spy around DOMPurify.sanitize
(e.g., vi.spyOn(DOMPurify, 'sanitize')) before calling renderMarkdown, call
renderMarkdown in the test, expect the spy toHaveBeenCalled() (optionally with
the rendered HTML string), and then restore the spy; reference the existing test
cases that call renderMarkdown('# Hello\n\n<script>alert(1)</script>') and
renderMarkdown('<img src=x onerror=alert(1)>') so you verify both
script-stripping and event-handler stripping paths actually use
DOMPurify.sanitize.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7de14baa-030b-42ab-9b13-3cbd63114d2a
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (20)
CONTRIBUTING.mdREADME.mdapi/error_codes.pyapi/export_api.pyapi/search.pyapi/sessions.pydocs/api-reference.mddocs/architecture.mdpackage.jsonpyproject.tomlstatic/js/shared/markdown.test.jsstatic/js/shared/state.test.jstests/conftest.pytests/test_api_integration.pytests/test_api_routes.pytests/test_cli_e2e.pytests/test_error_codes.pytests/test_error_propagation.pytests/test_export_api_bulk.pytests/test_search.py
💤 Files with no reviewable changes (1)
- pyproject.toml
✅ Files skipped from review due to trivial changes (2)
- docs/api-reference.md
- CONTRIBUTING.md
timon0305
approved these changes
May 22, 2026
Document all nine HTTP routes with error codes, curl examples, and cross-cutting policies. Add contributor onboarding and architecture diagram; link from README for Week 3 Thursday deliverable. Co-authored-by: Cursor <cursoragent@cursor.com>
- Enforce exclusion rules on GET .../stats (parity with session detail) - Clarify API error envelope exception for project sessions 400 - MD040 fences, CLI subprocess timeout, stricter search hit keys Co-authored-by: Cursor <cursoragent@cursor.com>
clean6378-max-it
force-pushed
the
docs/api-reference-and-contributing
branch
from
ed291df to
1e53d72
Compare
wpak-ai
approved these changes
May 22, 2026
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
3 participants
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.
Closes #43
Problem
ErrorCodeenum exists in code; consumers still lacked a single documented contract for"code"values and export semantics.Eval clusters: invisible-contract (test 16) and onboarding-cliff (test 17).
Changes
docs/api-reference.md(5 pt)GET /GET /api/projectsGET /api/projects/<name>/sessions[]quirkGET /api/sessions/<project>/<id>SessionDictshapeGET /api/sessions/.../statscompute_stats()fieldsGET /api/searchq,limit(cap 500),SEARCH_INVALID_LIMITGET /api/export/statePOST /api/exportsincemodes, zip response,422 EXPORT_NOTHING_TO_EXPORTGET /api/export/session/...format=md|jsonEvery documented
codematchesapi/error_codes.py— no invented codes.CONTRIBUTING.md(3 pt)pytest/npm testcommandserror_response(),safe_join(), issue Security: stop leaking exception details in HTTP error responses #25 leakage ruledocs/architecture.md(3 pt)all/last/incremental)README.mddocs/,CONTRIBUTING.md,api/error_codes.pyOut of scope
mkdocssite (optional follow-up)Summary by CodeRabbit
Release Notes
Documentation
Tests