Skip to content

Commit 9339f03

Browse files
committed
Merge remote-tracking branch 'origin/main' into feat/agentic-e2e-coverage
# Conflicts: # package-lock.json
2 parents b1cba08 + b81bde9 commit 9339f03

34 files changed

Lines changed: 1788 additions & 40 deletions

‎.claude/skills/verify-debatelab/recipes/account.spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ test("timer-run: the speech timer starts, pauses, and resets", async ({ signedIn
7474
await page.getByRole("button", { name: "Pause" }).click();
7575
await expect(page.getByText("PAUSED").first()).toBeVisible();
7676
const paused = await display.textContent();
77+
// eslint-disable-next-line no-restricted-properties -- observation window: a paused timer must not change.
7778
await page.waitForTimeout(1_500);
7879
expect(await display.textContent()).toBe(paused);
7980
await evidence.shot("01-paused");

‎.coderabbit.yaml‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# Merges with the organization settings in the CodeRabbit UI; only the keys
2+
# below are set here.
3+
inheritance: true
4+
knowledge_base:
5+
code_guidelines:
6+
enabled: true
7+
filePatterns:
8+
- "AGENTS.md"
9+
- "CODING_STANDARDS.md"
10+
- "docs/standards/*.md"
11+
reviews:
12+
path_instructions:
13+
- path: "**/*"
14+
instructions: >-
15+
Follow the review steps in CODING_STANDARDS.md and apply the area files
16+
in docs/standards/ that match the changed files. Report a rule violation
17+
as a finding with the rule's link. Skip what lint, typecheck, the test
18+
suites and npm run docs:check already catch.

‎CODING_STANDARDS.md‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
# Coding standards
2+
3+
Review rules for DebateLab diffs. They come from review findings on PRs #5 to #106 that the team accepted and fixed, and each rule links to a thread that produced it. A rule marked "single finding" came up once; weigh it below the rules reviewers raised repeatedly.
4+
5+
## Review steps
6+
7+
1. Map each changed file to its area below and read that area's file. Read [reviews.md](docs/standards/reviews.md) for every PR.
8+
2. Check the diff against every rule in those files. A violation is a finding like any bug: give its severity, `file:line`, and the rule's link.
9+
3. Done when every changed file has been checked against its area file, and the PR body has been checked against the diff.
10+
11+
Skip what lint, typecheck, the test suites, and `npm run docs:check` already catch. Spend the review on what they cannot see.
12+
13+
## Areas
14+
15+
- [Async work and client state](docs/standards/async-and-state.md): stores, pages, and hooks that await anything; stale responses, races with user actions, cancellation, timeouts.
16+
- [Trust boundaries and validation](docs/standards/boundaries.md): `src/lib/ai/`, Edge handlers, anything parsing model or network output; request budgets, failing closed, honest controls, usage and cost records.
17+
- [Tests](docs/standards/tests.md): any test file; tests that fail when the behavior they name breaks.
18+
- [Database and migrations](docs/standards/database.md): `supabase/migrations/`, `supabase/tests/`, privileged database clients.
19+
- [CI, secrets, and privacy](docs/standards/security.md): `.github/workflows/`, credentials, telemetry and error reporting.
20+
- [UI, language, and accessibility](docs/standards/ui.md): components, pages, and `src/locales/`.
21+
- [PR claims and documentation](docs/standards/reviews.md): the PR body, commit messages, and the documents a change must update.
22+
23+
## The rules reviewers raised most
24+
25+
1. A test must fail when the behavior it names breaks. Check both sides of every limit, and exercise behavior rather than matching source text. (about 29 findings)
26+
2. An async result that writes state checks that it is still current when it lands. Read the target of a user action (debate, user, record) when the user acts, before any `await`. (about 17)
27+
3. Validate untrusted data where it enters: model output, Edge responses, persisted browser state, request bodies. Reject or normalize it there, before anything renders or saves it. (about 15)
28+
4. Every user-facing string goes through `t()` with keys in all three catalogs, and every control has an accessible name. (about 15)
29+
5. A timeout or abort stops the work underneath it, not only the promise the caller awaits. (about 9)
30+
6. When a read fails, the answer is "unknown". Persist nothing and report no success on a failed read or write. (6)
31+
7. The owning document changes in the same PR as the behavior it describes. (7)

‎docs/README.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ Start with the smallest document that answers the task instead of loading every
1515
- [Agent guide](../AGENTS.md), current. Always-loaded commands, safety rules, and routing.
1616
- [Root README](../README.md), current. Product overview and quick start.
1717

18+
## Write and review code
19+
20+
- [Coding standards](../CODING_STANDARDS.md), current. Review rules drawn from accepted review findings, split by area under [`standards/`](standards/). Read by reviewers; open an area file when a review finding cites it.
21+
1822
## Set up and develop
1923

2024
- [Development guide](development.md), current. Open for installation, environment behavior, linked worktrees, frontend, backend, database, or iOS workflows.

‎docs/standards/async-and-state.md‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
# Async work and client state
2+
3+
Part of the [coding standards](../../CODING_STANDARDS.md). Race conditions were the second most common finding: a response from earlier work landing after the user moved on.
4+
5+
## Capture the target when the user acts
6+
7+
Read the debate, user, or record ID at the moment of the click, and pass it through the async chain. Do not read a store again after an `await`, because the user may have switched debates or accounts while you waited.
8+
9+
- Clear All read the Current Debate after awaiting auth, so a switch during the wait deleted the other debate's sources ([#72](https://github.com/Programcode/DebateLab/pull/72#discussion_r4112813743)).
10+
- Workspace research saved its sources into whichever personal debate was active when the response arrived ([#72](https://github.com/Programcode/DebateLab/pull/72#discussion_r4112813741)).
11+
- A fallback that reads "the current debate" must respect one-off mode; use `activeDebateId()`, not `currentDebateId` ([#72](https://github.com/Programcode/DebateLab/pull/72#discussion_r4112813737)).
12+
13+
## Stale responses must not overwrite newer state
14+
15+
When a load and a write can overlap, give the write a counter or request key and have the load discard its result if the counter moved while it ran. This is the pattern `debate-store.ts` and the context pins use now.
16+
17+
- Hydration replaced a debate created while it was loading ([#72](https://github.com/Programcode/DebateLab/pull/72#discussion_r4112813746)). A pin load overwrote a pin change made during the read ([#75](https://github.com/Programcode/DebateLab/pull/75#issuecomment-5873019386)).
18+
- A load that started before a dismissal brought the dismissed announcement back ([#19](https://github.com/Programcode/DebateLab/pull/19#discussion_r3896612965)).
19+
- A failed optimistic write undoes only its own change. Restoring a snapshot taken earlier also reverts other writes that succeeded since ([#75](https://github.com/Programcode/DebateLab/pull/75#issuecomment-5873818351)).
20+
- Count against the store's current state, not a value captured at render, so two quick clicks see each other ([#77](https://github.com/Programcode/DebateLab/pull/77#issuecomment-5877394644)).
21+
- Errors tied to one item carry that item's ID and render only beside it ([#19](https://github.com/Programcode/DebateLab/pull/19#discussion_r3896430653)).
22+
23+
## Supersede in-flight work explicitly
24+
25+
Starting new work for the same slot aborts the old request and hides its output. Track ownership with a request identity and check it before every state update in `then`, `catch`, and `finally`.
26+
27+
- A clarify request started while a stream was running; the old stream later appended text and cleared the loading flag ([#9](https://github.com/Programcode/DebateLab/pull/9#discussion_r3861289857)).
28+
- Cancel in-flight work on unmount as well as on clear and load ([#9](https://github.com/Programcode/DebateLab/pull/9#discussion_r3861496792)).
29+
- Undo during a section regeneration was reverted when the regeneration landed. Drop a result if the record it targets changed after the work started ([#89](https://github.com/Programcode/DebateLab/pull/89#issuecomment-5951346940)).
30+
- Key deduplication by a stable identity (user ID), not by a rotating access token ([#65](https://github.com/Programcode/DebateLab/pull/65#discussion_r4082668135)).
31+
32+
## React effects and refs
33+
34+
- Do not use an `active` closure flag in an effect that can return early without registering cleanup; a re-render can leave it `false` forever. Request-key refs already reject stale results ([#13](https://github.com/Programcode/DebateLab/pull/13#discussion_r3893649640)).
35+
- Write refs that track the previous render inside an effect, not during render ([#20](https://github.com/Programcode/DebateLab/pull/20#discussion_r3896378571)).
36+
- Derive view state from the same condition that chooses the rendered branch ([#20](https://github.com/Programcode/DebateLab/pull/20#discussion_r3896378587)).
37+
- One-shot guards must flip after their first use ([#14](https://github.com/Programcode/DebateLab/pull/14#discussion_r3894645041)).
38+
39+
## Auth transitions and persisted state
40+
41+
- Persisted browser state records its owner. On sign-in, clear state whose owner differs or is missing. Do not use "was there a previous user" as the ownership check ([#13 review](https://github.com/Programcode/DebateLab/pull/13#issuecomment-5477866033)).
42+
- Clear only the previous owner's data on an account switch, never the incoming user's ([#12](https://github.com/Programcode/DebateLab/pull/12#discussion_r3893641261)).
43+
- Validate persisted state on every rehydrate. Zustand calls `migrate` only on a version change, so put validation in `merge` ([#13](https://github.com/Programcode/DebateLab/pull/13#discussion_r3893649636)).
44+
- Do not serialize a whole store on every keystroke. Debounce drafts and cap stored history ([#13](https://github.com/Programcode/DebateLab/pull/13#discussion_r3893649632)).
45+
46+
## Timeouts and cancellation
47+
48+
A timeout that rejects the caller's promise but leaves the work running still spends money and CPU.
49+
50+
- Pass one `AbortSignal` through every layer of the work: research, Exa, model calls, body reads ([#5](https://github.com/Programcode/DebateLab/pull/5#discussion_r3835805781), [#9](https://github.com/Programcode/DebateLab/pull/9#discussion_r3861289875)). A `Promise.race` timeout without cancellation leaves a DOCX conversion running ([#63](https://github.com/Programcode/DebateLab/pull/63#discussion_r4081818971)).
51+
- Start the deadline and listen to the caller's signal before the first `await`, including `getSession()` ([#13 review](https://github.com/Programcode/DebateLab/pull/13#issuecomment-5477866033), [#16](https://github.com/Programcode/DebateLab/pull/16#discussion_r3895854139)).
52+
- Every external fetch has a bound, and anything that delays app startup has a fail-open timeout ([#5](https://github.com/Programcode/DebateLab/pull/5#discussion_r3835805765), [#21](https://github.com/Programcode/DebateLab/pull/21#discussion_r3896608554)).
53+
- A promise that wraps a callback API settles on every outcome, including cancel. The Google Picker promise never settled on cancel and left the panel disabled ([#63](https://github.com/Programcode/DebateLab/pull/63#discussion_r4081818959)).
54+
- After a caller abort, normalize the error to `AbortError` so callers do not report it as a failure ([#16](https://github.com/Programcode/DebateLab/pull/16#discussion_r3895854139)).
55+
- Secondary work after the main result (such as source-usage checks) gets its own budget and must not lose a result that already finished ([#69](https://github.com/Programcode/DebateLab/pull/69#discussion_r4093476756)).

‎docs/standards/boundaries.md‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
# Trust boundaries and validation
2+
3+
Part of the [coding standards](../../CODING_STANDARDS.md). The client AI boundary is [`src/lib/ai/client.ts`](../../src/lib/ai/client.ts); the server boundaries are the Edge Function handlers described in [supabase/README.md](../../supabase/README.md).
4+
5+
## Validate at the boundary, then trust the type
6+
7+
Model output, Edge responses, URLs, and request bodies are untrusted until a validator accepts them. Validate the complete shape where the data enters, before rendering, autosave, or a cast.
8+
9+
- Validate the whole output with its schema before saving sources. A count check plus a cast let malformed output through ([#9](https://github.com/Programcode/DebateLab/pull/9#discussion_r3861289852)).
10+
- Normalize optional fields to their primitive types. An object in `qualityLabel` reached React as a child ([#5](https://github.com/Programcode/DebateLab/pull/5#discussion_r3835805775)).
11+
- Use `validateUrl` for every URL you render as a link, and store the value it returns, not the input ([#17](https://github.com/Programcode/DebateLab/pull/17#discussion_r3896309984), [#17 review](https://github.com/Programcode/DebateLab/pull/17#issuecomment-5481176881)).
12+
- Validate raw types before coercing. `Boolean("false")` is `true` ([#29](https://github.com/Programcode/DebateLab/pull/29#discussion_r3937934126)).
13+
- Invalid client input returns a 4xx, never a 500 from a property access on `null` ([#13 review](https://github.com/Programcode/DebateLab/pull/13#issuecomment-5477866033)).
14+
- When the client validates locally, apply the same rules as the server, so a request the client accepted does not fail as a generic error ([#12](https://github.com/Programcode/DebateLab/pull/12#discussion_r3893641258)).
15+
- Send only values the server contract accepts. Local-only source IDs made every AI request fail with 400 ([#66](https://github.com/Programcode/DebateLab/pull/66#discussion_r4085167559)).
16+
- Prefer a typed result over `as` casts at call sites. Unchecked casts hide shape drift ([#18](https://github.com/Programcode/DebateLab/pull/18), single finding).
17+
18+
## Model output can omit what the prompt asked for
19+
20+
The shared JSON rules tell the model to omit a part it cannot fill. Map an omitted field to its documented empty value instead of failing the request ([#12](https://github.com/Programcode/DebateLab/pull/12#discussion_r3893641334)). A response variant that does not match the operation is an error the UI can recover from, not a silent return that leaves the page pending ([#12](https://github.com/Programcode/DebateLab/pull/12#discussion_r3893641293)).
21+
22+
## Size limits are UTF-8 bytes
23+
24+
The Edge limit is on serialized bytes, so budget in bytes, across the whole serialized payload, before transport ([#66](https://github.com/Programcode/DebateLab/pull/66#discussion_r4085167588), [#12](https://github.com/Programcode/DebateLab/pull/12#discussion_r3893641325)).
25+
26+
- Do not silently shorten what the user typed. Reject it with a visible message, or keep the truncation marker so the server can report it ([#66](https://github.com/Programcode/DebateLab/pull/66#discussion_r4085302451)).
27+
- A value exactly at the limit is allowed. Check the boundary with a test on each side ([#12](https://github.com/Programcode/DebateLab/pull/12#discussion_r3893641317)).
28+
- Check `response.ok` before reading a body, and read bounded bodies inside the timeout ([#11](https://github.com/Programcode/DebateLab/pull/11#discussion_r3872004292)).
29+
30+
## Fail closed, and say "unknown" when you do not know
31+
32+
- A failed entitlement read is indeterminate. Treating it as "not Pro" persisted a downgrade of the user's model choice ([#47](https://github.com/Programcode/DebateLab/pull/47#discussion_r4054031767)).
33+
- Check the result of every write before reporting success ([#63](https://github.com/Programcode/DebateLab/pull/63#discussion_r4081818967)). Lint rejects unhandled promises; a `void` in front of one is correct only when the promise cannot reject or handles its own errors and tells the user ([#63 outside diff](https://github.com/Programcode/DebateLab/pull/63)).
34+
- Catch the specific error you expect (a missing file, a missing column) and rethrow the rest ([#5](https://github.com/Programcode/DebateLab/pull/5#discussion_r3835805773)).
35+
- Read provider per-item status before falling back to a generic message ([#11](https://github.com/Programcode/DebateLab/pull/11#discussion_r3872004284)).
36+
- A fix for a permission change covers every write path, not only the one that failed first ([#44](https://github.com/Programcode/DebateLab/pull/44#discussion_r4049229043)).
37+
38+
## Controls must be honest
39+
40+
A visible toggle or setting must change the request or deterministic post-processing. This is an existing team rule that reviewers enforced:
41+
42+
- `useActiveSources` did not change the opponent request ([#11](https://github.com/Programcode/DebateLab/pull/11#discussion_r3872004224)).
43+
- Research instructions were attached when research returned nothing, inviting unsupported citations ([#5](https://github.com/Programcode/DebateLab/pull/5#discussion_r3835805784)).
44+
- A privacy disclosure stays only while the code enforces it ([#22](https://github.com/Programcode/DebateLab/pull/22#discussion_r3896703404)).
45+
- Tier defaults must reach every fallback path, not only a migration ([#47](https://github.com/Programcode/DebateLab/pull/47#discussion_r4054031797)).
46+
47+
## Usage and cost records
48+
49+
Every provider call that can bill gets recorded, including retries, repairs, research sub-calls, and cache-only usage. A request that did not complete records its observed usage with a null cost.
50+
51+
- Cache-only usage returned before pricing ([#5](https://github.com/Programcode/DebateLab/pull/5#discussion_r3835805777)). Repair attempts dropped the failed call's tokens ([#5 review](https://github.com/Programcode/DebateLab/pull/5#issuecomment-5379671525), [#9](https://github.com/Programcode/DebateLab/pull/9#discussion_r3861289886)).
52+
- A stream that errors after its finish frame is incomplete ([#46](https://github.com/Programcode/DebateLab/pull/46#discussion_r4053914209)).
53+
54+
## Keep one copy of a contract
55+
56+
When two code paths implement the same privacy-sensitive payload or validation, move it to one helper. The two copies of the AI failure report had already drifted ([#18](https://github.com/Programcode/DebateLab/pull/18), single finding). Configuration that exists in two places (such as two CSP lists) changes together, with a test that compares them ([#15](https://github.com/Programcode/DebateLab/pull/15#discussion_r3894972424)).

0 commit comments

Comments
 (0)