fix(api): restore liveness routing and enforce public docstrings - #498
Conversation
A stray decorator had stacked GET /healthz onto read_tenant_settings,
so the liveness probe silently required auth and hit Postgres instead
of returning {"status": "ok"}; the real healthz() handler had no route
at all. Restored the decorator to the correct handler and added a
regression test.
Also closed the repository-wide docstring-coverage gap: an AST audit
of lineageweave/ and backend/app/ found 35 public functions/classes
missing docstrings (excluding private/dunder names, __init__.py, and
tests). Added them all, plus two leftover "buyer" wording references
from before the terminology rename.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011EP69xAyLaJxa6oaF6D9eq
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
35 missing production docstrings found by an independent AST re-audit were written in PR #498, with the full Python suite still green. Coverage-enforcement (pytest coverage gate, Vitest coverage provider) remains open and is called out separately now. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011EP69xAyLaJxa6oaF6D9eq
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Note: #429 (opened 2026-08-22) already fixes this exact /healthz decorator bug, plus a related tenant_settings migration-fixture gap this PR doesn't touch. Found independently while cross-checking the open-PR list this checkpoint. Both PRs move the same decorator to the same place, so whichever merges first should make the other a clean no-op on that hunk; this PR's unique contribution is the 35 missing docstrings + the new tests/test_public_docstrings.py enforcement gate (added by a concurrent push on this branch). |
|
@opencode-agent @cwl-noema-review exact-head independent review for
|
…k' into agent-pr498-compose-20260824 # Conflicts: # CHANGELOG.md # frontend/src/App.test.tsx
seonghobae
changed the title
* fix(api): route /healthz to the actual liveness probe, not settings
Two @app.get decorators were stacked before read_tenant_settings:
@app.get("/healthz")
@app.get("/api/settings", response_model=dict)
async def read_tenant_settings(...):
Both bound to the same handler -- "/healthz" required authentication
(read_tenant_settings depends on get_current_account) and the real
healthz() function below had no route decorator at all, so it was
dead code never reachable by any request. docker-compose.yml's own
backend healthcheck hits "/healthz" with a plain unauthenticated
urllib.request.urlopen call; against this bug it would receive
401/403, fail the healthcheck, and mark the container unhealthy on
every fresh deployment.
Move the decorator onto healthz() where it belongs.
Also add migration 0103_tenant_settings.sql to backend/tests/test_api.py's
seeded_db fixture -- it was never added when the migration shipped, so
the tenant_settings table (and therefore the /api/settings GET/PATCH
endpoints, both previously untested) didn't exist in the test schema
at all.
Tests: test_healthz_is_reachable_without_a_token (the regression this
bug needed) plus three new /api/settings tests (GET returns the seeded
brand name, PATCH requires post_admin, PATCH as admin actually changes
it). uv run --frozen python -m pytest -q: 757 passed, 17 skipped.
* fix(frontend): use OIDC return-url helpers and guard AdminPanel render
Same shared-ancestor bug as #418/#415/#426/#427: the login button built
an unsanitized returnUrl inline instead of returnUrlFromLocation()/
rememberOidcReturnUrl(), and AdminPanel's accessToken (string, required)
was rendered from a string | undefined at both call sites.
* fix(frontend): drop redundant accessToken guard on AdminPanel render
An earlier `if (!accessToken) return` a few hundred lines up already
narrows accessToken to string for the rest of the authenticated render
tree -- confirmed with a clean tsc build without the guard.
| for node in ast.walk(tree): | ||
| if ( | ||
| isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)) | ||
| and not node.name.startswith("_") | ||
| and ast.get_docstring(node) is None | ||
| ): | ||
| missing.append(f"{path.relative_to(root)}:{node.lineno}:{node.name}") | ||
| return missing |
There was a problem hiding this comment.
📝 Info: Docstring gate is strict and nesting-blind
The gate in test_public_docstrings.py walks all nodes with ast.walk, so it requires docstrings on public-named definitions even when nested inside private classes or functions. It passes now (0 missing), but future contributors face this broad, non-obvious requirement.
Was this helpful? React with 👍 or 👎 to provide feedback.
… merge The base branch (worktree-fix-frontend-build-break) had already merged this PR's own docstring addition into fuse_rankings(), but the earlier conflict resolution on this branch's side had dropped the 'active_weights = weights or DEFAULT_CHANNEL_WEIGHTS' line entirely while keeping only the docstring — a NameError waiting to happen at the next line, which reads active_weights. Kept both. Verified: 846 Python tests pass (15/15 in rankweave-specific tests).
2ef7afa
into
worktree-fix-frontend-build-break
1 participant
Buyer problem
The Compose liveness probe called
/healthz, but a stray decorator routed that path to authenticated tenant settings while the real liveness handler had no route. A fresh deployment could therefore report unhealthy even when the process was running. Public production definitions also lacked a repository-wide docstring contract, leaving beginners without stable API intent.Unique scope
/healthzto the public process-only handler and keep/api/settingsauthenticated.lineageweave/andbackend/app/.The inherited OIDC/Admin source and frontend-test changes are owned by parent PR #426 and are absent from this effective diff.
Exact composition
2081f7deba6c66edd73713e1566166fb798e469a.fa57c15842d4ef1f50a9e168ec79f519eb8a5e28(worktree-fix-frontend-build-break).After #426 lands on protected
main, retarget this PR tomain, refetch exact head/base, and require fresh terminal checks plus an independent exact-head approval. Then revalidate child #429 against this PR's protected-main form. Do not transfer stacked-base evidence.Exact validation
uv run pytest -q: 839 passed, 17 skipped, 4 dependency warnings.git diff --check: passed.All scanner fixtures are synthetic and remain outside production artifacts.