diff --git a/.codex/hooks.json b/.codex/hooks.json new file mode 100644 index 0000000..c29c05a --- /dev/null +++ b/.codex/hooks.json @@ -0,0 +1,24 @@ +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash|Grep", + "hooks": [ + { + "type": "command", + "command": "/Users/adam/.local/bin/graphify hook-guard search" + } + ] + }, + { + "matcher": "Read|Glob", + "hooks": [ + { + "type": "command", + "command": "/Users/adam/.local/bin/graphify hook-guard read" + } + ] + } + ] + } +} diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..6466ef5 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,109 @@ +## Development workflow + +New functionality gets demoed locally and approved by Adam **before** anything +is committed. Applies to all the druthers repos (`druthers-api`, +`druthers-web`, `druthers-mcp`, `druthers-infra`). + +1. **Build it against the local dev stack.** Use the `druthers-up` skill to + bring up Postgres + the API + `next dev`. Verify against real upstream data + (TMDB, TVMaze, Open Library, IGDB), not just mocks and unit tests. +2. **Test in browser & demo with local URL + visuals.** + Use the browser subagent to interactively test new web functionality on the + local dev stack. Present a local URL (e.g. `http://localhost:3000/u/dadam` or + `http://localhost:3000/movies/`), a list of what to look for, embedded + screenshots for visual review, and session recording videos (`.webp`) for + complex/important UX flows. Stop and wait for Adam's approval. +3. **Only after Adam approves:** spin the local environment down + (`task dd -- dev` in `druthers-api`), then commit, push, and open the PR. +4. **Hand back the PR link.** Merging, releasing, and deploying stay separate + asks — never chain them off the same approval. +5. **Once the PR is merged, return the local repo to `main` and pull** (and + delete the now-merged local branch). A repo left checked out on a stale + branch is silently inherited by the next session, which either builds new + work on top of dead history or has to spend a turn untangling it first. + +Do not commit or open a PR ahead of the demo, even when tests and CI would +pass. The approval gate is the demo, not the green build. + +Before starting new work in any of these repos, check `git branch --show-current` +and `git status` first — don't assume the checkout is `main` or clean. + +### Issue vs. PR numbers + +GitHub issue numbers and PR numbers share one repo-wide counter, so a bare +number is ambiguous — `289` could be either, and groomed backlog stories from +`story-intake` are always issues, never PRs. When told to "pull in" / "start" / +"work on" a bare number (or a repo-prefixed one like "web 134"), don't assume +which it is from context or phrasing — confirm with `gh issue view ` and/or +`gh pr view ` before branching off it, reporting its status, or otherwise +acting on it. + +Don't trust issue/PR *state* at face value either — it can drift from what's +actually in the code: + +- A PR body listing `Closes #a, #b, #c, ...` as one comma-separated list after + a single keyword reliably auto-closes only the **first** issue on merge — + the rest silently stay open even though the code shipped (#283 merged and + claimed six closes; only one fired). When writing a PR body that closes + several issues, repeat the keyword per issue (`Closes #a. Closes #b.`) — + don't rely on the comma form. When *reading* a merged PR that lists several + issues via the comma form, verify each one's state with `gh issue view` + rather than assuming the merge closed all of them. +- A PR can also be closed **without merging** and silently orphan a whole + downstream stack of branches (#287 closed, blocking #279's work and + everything branched on top of it from ever reaching `main`). If a + dependency issue/PR looks unexpectedly open or blocked, check whether the + PR that was supposed to deliver it actually merged — don't assume "closed" + means "done," and don't assume a dependency is real work remaining without + checking whether it already shipped under a different PR. + +## Testing + +A new module needs a test file in the same PR that introduces it — not as +follow-up work. This project's test debt (audited 2026-08-03, tracked in +issues #290–293) came almost entirely from modules that shipped without one +and were never revisited. + +- **New router/service/job** (`app/router/`, `app/services/`, `app/jobs/`, + `app/migration/`): add a matching `tests/integration/_test.py` or + `tests/unit/_test.py`. Every existing router already has one — + match that, don't be the exception. +- **Per-domain work** (movies/TV/books/games, or the same pattern in + druthers-mcp's tool families): if you're touching one domain, check + whether the other three need the same change *and* the same test. Silent + gaps like this are exactly what #291 and #39/#40 went back to fix — + cheaper to keep the four in lockstep than to backfill later. +- **New interactive web component** (`src/components/`): add a + `.test.tsx` alongside it once the React Testing Library setup from + #136/#137 is in place. Pure logic still belongs in `src/lib/*.ts` with a + `.test.ts` sibling, not inside the component. +- **New MCP tool** (`aleonard_mcp/server.py`): add a test in + `tests/server_test.py` following the pattern of the nearest existing + sibling tool (e.g. a new `set_*_note` tool mirrors `set_note`'s test). +- Coverage is a floor, not a target: CI fails if total coverage drops below + its current baseline (the ratchet from #292/#138), but a passing ratchet + only proves nothing else regressed — it's not evidence the new code itself + is tested. Don't point to a green build in place of a test for the thing + you just wrote. +- `test`/`lint` are becoming required status checks on `main` alongside the + security scan (#24) — once that lands, a PR with failing tests won't merge, + not just won't get reviewed. Until then, treat a red `test`/`lint` run as + a hard blocker anyway; the check not being enforced yet isn't permission + to ignore it. + +## Python formatting + +The pre-commit `black` hook runs with `--skip-string-normalization` (see +`.pre-commit-config.yaml`) — it never rewrites quote style. A separate +`double-quote-string-fixer` hook converts double quotes to single after. +If you run `black` by hand instead of relying on the hook, pass +`--skip-string-normalization` too, or it'll flip the file to double quotes +and force an extra fix-and-recommit round trip. + +## graphify + +This project has a knowledge graph at graphify-out/ with god nodes, community structure, and cross-file relationships. + +Rules: +- Read graphify-out/GRAPH_REPORT.md only for broad architecture review or when query/path/explain do not surface enough context. +- After modifying code, run `graphify update .` to keep the graph current (AST-only, no API cost). diff --git a/alembic/versions/dbe0e1d4fd86_add_onboarding_completed_to_users.py b/alembic/versions/dbe0e1d4fd86_add_onboarding_completed_to_users.py new file mode 100644 index 0000000..d9321db --- /dev/null +++ b/alembic/versions/dbe0e1d4fd86_add_onboarding_completed_to_users.py @@ -0,0 +1,36 @@ +"""Add onboarding_completed to users + +Revision ID: dbe0e1d4fd86 +Revises: f6a31c8d9e42 +Create Date: 2026-08-06 12:05:56.141707 + +""" + +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision: str = 'dbe0e1d4fd86' +down_revision: Union[str, Sequence[str], None] = 'f6a31c8d9e42' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + """Upgrade schema.""" + op.add_column( + 'users', + sa.Column( + 'onboarding_completed', + sa.Boolean(), + nullable=False, + server_default=sa.text('false'), + ), + ) + + +def downgrade() -> None: + """Downgrade schema.""" + op.drop_column('users', 'onboarding_completed') diff --git a/app/db/models.py b/app/db/models.py index 71c564b..179d2f8 100644 --- a/app/db/models.py +++ b/app/db/models.py @@ -6,6 +6,7 @@ from datetime import datetime, timezone from sqlalchemy import ( + Boolean, CheckConstraint, Column, DateTime, @@ -14,6 +15,7 @@ Integer, String, UniqueConstraint, + text, ) from sqlalchemy.orm import backref, relationship @@ -118,6 +120,14 @@ class DbUser(DBBaseModel): nullable=True, ) + # First-time onboarding state (#135). + onboarding_completed = Column( + Boolean, + nullable=False, + default=False, + server_default=text('false'), + ) + class DbApiKey(DBBaseModel): """ diff --git a/app/router/v1/router_preferences.py b/app/router/v1/router_preferences.py index 3be6c25..2a71961 100644 --- a/app/router/v1/router_preferences.py +++ b/app/router/v1/router_preferences.py @@ -20,7 +20,8 @@ def get_preferences(current_user: list = Depends(get_current_user)): user = current_user[0] return OutPreferences( - ranked_list_length=preferences.coerce(user.ranked_list_length) + ranked_list_length=preferences.coerce(user.ranked_list_length), + onboarding_completed=user.onboarding_completed, ) @@ -34,8 +35,14 @@ def update_preferences( data = request.model_dump(exclude_unset=True) if 'ranked_list_length' in data and data['ranked_list_length'] is not None: user.ranked_list_length = data['ranked_list_length'] + if 'onboarding_completed' in data and data['onboarding_completed'] is not None: + user.onboarding_completed = data['onboarding_completed'] + + if data: db.commit() db.refresh(user) + return OutPreferences( - ranked_list_length=preferences.coerce(user.ranked_list_length) + ranked_list_length=preferences.coerce(user.ranked_list_length), + onboarding_completed=user.onboarding_completed, ) diff --git a/app/schemas/model_schemas.py b/app/schemas/model_schemas.py index 6b07d90..ef5f5a6 100644 --- a/app/schemas/model_schemas.py +++ b/app/schemas/model_schemas.py @@ -274,12 +274,14 @@ class InPreferencesUpdate(BaseModel): """Request body for display preferences (#122). Only sent fields change.""" ranked_list_length: Optional[RankedListLength] = None + onboarding_completed: Optional[bool] = None class OutPreferences(BaseModel): """The caller's display preferences, defaulted where unset.""" ranked_list_length: RankedListLength = RankedListLength.TWENTY_FIVE + onboarding_completed: bool = False model_config = ConfigDict(from_attributes=True) @@ -316,3 +318,4 @@ class OutSummary(BaseModel): profile_public: bool = False shelves: list[OutSummaryShelf] total_ranked: int + onboarding_completed: bool diff --git a/app/services/summary.py b/app/services/summary.py index 13e7ce2..8ef9e49 100644 --- a/app/services/summary.py +++ b/app/services/summary.py @@ -108,6 +108,7 @@ def build_summary(db: Session, user: DbUser, top_n: int = TOP_N) -> dict: and any(s['public'] for s in shelves), 'shelves': shelves, 'total_ranked': sum(s['ranked_count'] for s in shelves), + 'onboarding_completed': user.onboarding_completed, } diff --git a/tests/conftest.py b/tests/conftest.py index 7c8ddd7..0bd0115 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -245,12 +245,17 @@ def fixture_test_user_data_generator(): ''' def _generate_user_data(num_users=1): + import uuid + user_data = [] for _ in range(num_users): user_data.append( InUserBase( display_name=fake.name(), - email=f'{fake.first_name()}.{fake.last_name_nonbinary()}@zoho.com', + email=( + f'{fake.first_name()}.{fake.last_name_nonbinary()}' + f'.{uuid.uuid4().hex[:8]}@zoho.com' + ), password=fake.password(length=20), ) ) diff --git a/tests/integration/router_preferences_test.py b/tests/integration/router_preferences_test.py index 4e7f1cc..c1b8c1b 100644 --- a/tests/integration/router_preferences_test.py +++ b/tests/integration/router_preferences_test.py @@ -12,7 +12,7 @@ def test_default_is_25(test_client: TestClient): body = test_client.get( '/v1/users/me/preferences', headers=_auth(test_client.first_user.token) ).json() - assert body == {'ranked_list_length': '25'} + assert body == {'ranked_list_length': '25', 'onboarding_completed': False} def test_set_and_read_back(test_client: TestClient): @@ -23,10 +23,30 @@ def test_set_and_read_back(test_client: TestClient): json={'ranked_list_length': 'all'}, ) assert updated.status_code == 200 - assert updated.json() == {'ranked_list_length': 'all'} + assert updated.json() == { + 'ranked_list_length': 'all', + 'onboarding_completed': False, + } fetched = test_client.get('/v1/users/me/preferences', headers=_auth(token)) - assert fetched.json() == {'ranked_list_length': 'all'} + assert fetched.json() == { + 'ranked_list_length': 'all', + 'onboarding_completed': False, + } + + +def test_set_onboarding_completed(test_client: TestClient): + token = test_client.first_user.token + updated = test_client.put( + '/v1/users/me/preferences', + headers=_auth(token), + json={'onboarding_completed': True}, + ) + assert updated.status_code == 200 + assert updated.json() == {'ranked_list_length': '25', 'onboarding_completed': True} + + fetched = test_client.get('/v1/users/me/preferences', headers=_auth(token)) + assert fetched.json() == {'ranked_list_length': '25', 'onboarding_completed': True} def test_invalid_length_rejected(test_client: TestClient): @@ -47,7 +67,7 @@ def test_preferences_are_per_user(test_client: TestClient): other = test_client.get( '/v1/users/me/preferences', headers=_auth(test_client.second_user.token) ).json() - assert other == {'ranked_list_length': '25'} + assert other == {'ranked_list_length': '25', 'onboarding_completed': False} def test_preferences_require_auth(test_client: TestClient):