familienarchiv/COLLABORATING.md

# Collaboration Rules

How we work together on this project.

## Honesty and Objectivity

Evaluate all suggestions on their technical merits. No sycophancy — if something doesn't make sense, doesn't align with best practices, or could be improved, say so directly and constructively. Technical accuracy and project quality take precedence over being agreeable.

## Core Workflow: Research → Plan → Implement → Validate

Every non-trivial feature or bug fix follows this sequence:

1. **Research** — Read the relevant code. Understand existing patterns before touching anything.
2. **Plan** — Write a plan to `/.agent/current-plan.md` and align with the user before writing code. Update the plan as work progresses.
3. **Implement** — Use Red/Green TDD (see below).
4. **Validate** — Run formatters, linters, and tests after every implementation step.

Never start writing code without having read the relevant files first.

## Red/Green TDD

All new behavior is driven by tests written **before** the implementation. The cycle is:

1. **Red** — Write a test that captures the requirement. Run it and confirm it fails. A test that passes before the implementation is written is not testing anything real.
2. **Green** — Write the minimum production code needed to make the test pass. No more.
3. **Refactor** — Clean up the implementation (names, structure, duplication) while keeping the test green.
4. **Commit** — The test and implementation ship together in a single logical commit.

Repeat for each new behavior.

### What level of test to write

| Scenario | Test type |
|---|---|
| Business logic, calculations, service rules | Unit test (`DocumentServiceTest`, etc.) |
| HTTP contract, request validation, error codes | Controller slice test (`@WebMvcTest`) |
| Full user-facing behavior, navigation, forms | E2E Playwright spec |

### Rules

- Never write production code without a failing test that requires it.
- Keep the Green step minimal — resist adding "obvious" extras that have no test yet.
- The Refactor step must not change behavior — if a test breaks, the refactor introduced a bug.
- If a bug is reported with no test, write the failing test first, then fix it.

## User Journeys & E2E Acceptance Criteria

Every `feature` issue must include two sections before any implementation begins:

### 1. User Journey

A plain-prose description of the steps a user takes to get value from the feature. Written from the user's perspective, not the implementation's:

> User opens a document, clicks "History", sees a chronological list of changes with editor name and timestamp. Clicking a row expands the old vs. new values.

This makes the scope concrete and prevents scope creep — anything not in the journey is out of scope for the issue.

### 2. E2E Scenarios

One or more acceptance criteria written as Playwright-ready scenarios. These become the outermost Red test in the TDD cycle — no feature is considered done until all its E2E scenarios pass:

```
Scenario: View edit history of a document
  Given I am on a document detail page
  When I click the "History" tab
  Then I see at least one revision entry
  And each entry shows the editor's name and a timestamp
```

Use this format consistently. It maps directly to `test.describe` / `test` blocks in the Playwright spec.

### Where this fits in the workflow

```
Issue (Journey + Scenarios) → Red E2E test → Implementation → Green
```

The scenarios in the issue are the contract. Write them before planning, treat them as failing tests from day one.

---

## Issue Tracking (Gitea)

All work is tracked in **Gitea** at `http://192.168.178.71:3005` (repo `marcel/familienarchiv`). Never use todo files or CLAUDE.md notes as a substitute.

Create an issue whenever work is identified that isn't being done in the current session.

### Issue title formats

**`feature` label** — user story format:
```
As a [role] I want [capability] so/because [reason]
```
Examples:
- "As a user I want to search documents so I can find a specific document faster"
- "As an admin I want to add a new user so I don't have to restart the server"

**`bug` label** — user-facing impact, not the technical cause:
```
[What breaks] when [trigger]
```
Examples:
- "Document list shows blank page when no results found"
- "Upload fails silently when file exceeds 50MB"

**`devops` label** — infrastructure, CI/CD, deployment, tooling:
- "Fix CI checkout failing due to unresolvable hostname"
- "Add E2E test seed data for runner"

### Priority labels

- `priority: high` — blocking or urgent
- `priority: medium` — normal
- `priority: low` — nice to have

### Other labels

- `needs-discussion` — decision needed before work starts
- `wontfix` — acknowledged, not addressing

## Branching and Pull Requests

All changes go through a branch and a pull request — never commit directly to `main`.

### Branch naming

```
<type>/<short-description>
```

Examples:
- `feat/received-documents-person-page`
- `fix/tag-filter-url-sync`
- `devops/docker-compose-v2`

### Workflow

1. Create a branch from `main` before writing any code.
2. Commit work to that branch.
3. Open a pull request on Gitea targeting `main` for review.
4. Wait for approval before merging.

The PR description should reference the related issue (`Closes #n` or `Refs #n`).

## Commit Messages

Every commit must reference the relevant Gitea issue.

- `Closes #12` — commit fully resolves the issue (Gitea auto-closes it)
- `Refs #12` — commit is related but doesn't fully close the issue

Place the reference at the end of the commit body:

```
feat: add person typeahead to document edit form

Closes #7
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
```

### Atomic commits

Each commit must do exactly one logical thing. Never bundle multiple unrelated changes into a single commit, even if they are small.

**Wrong** — three changes in one commit:
```
fix(e2e+i18n): add missing DE translation, fix test selectors, fix lang switching
```

**Right** — three separate commits:
```
fix(i18n): add missing person_btn_conversations DE translation
fix(e2e): exclude /persons/new from person link selector
fix(e2e): clear locale cookie when switching back to base language
```

When in doubt, commit more often rather than less.

## Code Style

See [CODESTYLE.md](./CODESTYLE.md) for the full guide: Clean Code (Uncle Bob), DRY/KISS trade-offs, and SOLID principles applied to this stack.

Quick reminders:
- Pure functions over stateful helpers where possible
- No premature abstractions — KISS beats DRY
- No backwards-compatibility shims for code that has no callers
- Validate at system boundaries only (user input, external APIs)

## Frontend Domain Boundaries

The frontend mirrors the backend's package-by-domain structure. Each Tier-1 folder under `src/lib/` is a domain with a hard import boundary:

```
document  person  tag  user  geschichte  notification  ocr
activity  conversation  shared
```

The `boundaries/dependencies` ESLint rule enforces this. The full allow-list lives in `frontend/eslint.config.js`. The rule fires at error severity and blocks `npm run lint`.

### Allowed cross-domain imports

| From | May import from |
|---|---|
| `document` | `shared`, `person`, `tag`, `ocr`, `activity`, `conversation` |
| `geschichte` | `shared`, `person`, `document` |
| `ocr` | `shared`, `document` |
| `activity` | `shared`, `notification` |
| `person`, `tag`, `user`, `notification`, `conversation` | `shared` only |
| `shared` | `shared` only |
| `routes` | any domain |

### When you need to cross a boundary

1. **Move the code to `$lib/shared/`** — the correct fix when the code is truly generic (a UI primitive, a pure utility, a formatting helper).
2. **Add an explicit rule** — if a cross-domain dependency is architecturally justified (e.g., `document` importing `PersonTypeahead`), add the allow entry to `eslint.config.js` with a comment explaining the reason.
3. **Use `// eslint-disable-next-line boundaries/dependencies`** — last resort, only for cases where neither option is practical. Leave a comment explaining why.

### Verifying the rule works

```bash
npm run lint:boundary-demo   # exits 1 — shows the rule firing on a deliberate tag→person violation
```

The fixture lives at `src/lib/tag/__fixtures__/cross-domain.fixture.ts` and is excluded from `npm run lint` via `--ignore-pattern`.