docs(metrics)!: restate the metric_snapshots comment and pin it to t…

.gitignore

@@ -1,7 +1,7 @@
 docs/*
+# Metric documents are thesis-canonical and belong in the deliverable. Specs, ADRs
+# and internal notes stay local, which the docs/* rule above already handles.
 !docs/metrics/
-docs/metrics/*
-!docs/metrics/author-attribution.md
 .claude/
 .agents/
 .idea

README.md

@@ -53,8 +53,8 @@ a React dashboard with AI-generated insights via local Ollama.
 
 | Variable | Purpose | Default | How to generate |
 |----------|---------|---------|----------------|
-| `JWT_SECRET` | JWT signing key | dev key (**insecure**) | `openssl rand -hex 32` |
-| `ENCRYPTION_KEY` | AES-256-GCM token encryption key (base64) | dev key (**insecure**) | `openssl rand -base64 32` |
+| `JWT_SECRET` | JWT signing key | none — **startup fails** if unset | `openssl rand -hex 32` |
+| `ENCRYPTION_KEY` | AES-256-GCM token encryption key (base64) | none — **startup fails** if unset | `openssl rand -base64 32` |
 | `POSTGRES_PASSWORD` | Database password | `123` (dev only) | choose one |
 | `SMTP_HOST` | SMTP server for password-reset emails | `mailhog` (Docker) / `smtp.gmail.com` (manual) | — |
 | `SMTP_PORT` | SMTP port | `1025` (Docker) / `587` (manual) | — |
@@ -63,6 +63,11 @@ a React dashboard with AI-generated insights via local Ollama.
 | `OLLAMA_BASE_URL` | Ollama server URL | `http://localhost:11434` | — |
 | `COOKIE_SECURE` | Set `true` in production (HTTPS only) | `false` | — |
 
+> `JWT_SECRET` and `ENCRYPTION_KEY` have no defaults. Unset, the application fails to
+> start: `JWT_SECRET` shorter than 32 bytes raises `WeakKeyException`, and an empty
+> `ENCRYPTION_KEY` raises `IllegalArgumentException: Empty key`. Both are thrown during
+> bean creation, so the stack trace names the bean rather than the variable.
+>
 > In production, always set `JWT_SECRET`, `ENCRYPTION_KEY`, and `POSTGRES_PASSWORD`
 > via environment variables or a secrets manager. Never commit real secrets.
 

dev-analytics/.env.example

@@ -1,16 +1,35 @@
 # Copy this file to .env and fill in the values before running docker compose up.
 
-# Required — generate with: openssl rand -hex 32
+# -- Required ------------------------------------------------------------------
+# No defaults. The application fails to start if either is unset.
+# Generate with: openssl rand -hex 32
 JWT_SECRET=
-
-# Required — generate with: openssl rand -base64 32
+# 32 bytes, base64-encoded. Generate with: openssl rand -base64 32
 ENCRYPTION_KEY=
-
-# Required — choose a strong password for the Postgres superuser
+# Choose a strong password for the Postgres superuser
 POSTGRES_PASSWORD=
 
-# Optional — needed only for password-reset emails
-SMTP_HOST=smtp.gmail.com
-SMTP_PORT=587
+# -- Database ------------------------------------------------------------------
+# Unset in Docker: docker-compose.yml points the app at the db service. Set these
+# only when running the backend outside the compose network.
+SPRING_DATASOURCE_URL=
+SPRING_DATASOURCE_USERNAME=
+SPRING_DATASOURCE_PASSWORD=
+
+# -- Email (SMTP) --------------------------------------------------------------
+# Optional - needed only for password-reset emails. Compose defaults to MailHog,
+# viewable at http://localhost:8025.
+SMTP_HOST=
+SMTP_PORT=
 SMTP_USERNAME=
 SMTP_PASSWORD=
+
+# -- Ollama (AI) ---------------------------------------------------------------
+OLLAMA_BASE_URL=
+OLLAMA_MODEL=
+
+# -- App -----------------------------------------------------------------------
+APP_BASE_URL=
+APP_FRONTEND_URL=
+# Set true when serving over HTTPS
+COOKIE_SECURE=

dev-analytics/src/main/java/com/juliashtal/devanalytics/metrics/calc/MetricSnapshotWriter.java

@@ -13,8 +13,7 @@
 
 /**
  * Persists a {@link MetricSnapshot} using the native-SQL upsert guard.
- * Extracted from {@code MetricsService.saveMetric} — logic is unchanged.
- * All {@link MetricCalculator} beans call this service to write their results.
+ * Every {@link MetricCalculator} writes its results through this service.
  */
 @Service
 @RequiredArgsConstructor

dev-analytics/src/main/java/com/juliashtal/devanalytics/metrics/model/MetricSnapshot.java

@@ -44,11 +44,11 @@
  * <p>Invariant: {@code period_from IS NOT NULL AND period_to IS NOT NULL}.</p>
  *
  * <h3>Upsert guard</h3>
- * <p>All writes go through {@code MetricsService.saveMetric}, which uses a native-SQL
+ * <p>All writes go through {@code MetricSnapshotWriter}, which uses a native-SQL
  * {@code findExisting} query with {@code IS NOT DISTINCT FROM} on nullable dimensions
  * ({@code repository_id}, {@code team_id}, {@code period_from}, {@code period_to}) to
- * locate an existing row before inserting. Bypassing {@code saveMetric} will produce
- * duplicate rows that aggregate incorrectly.</p>
+ * locate an existing row before inserting. The table carries no unique key, so bypassing
+ * the writer produces duplicate rows that aggregate incorrectly.</p>
  *
  * <h3>Scope</h3>
  * <p>Personal metrics: {@link #team} is {@code NULL}.

dev-analytics/src/main/java/com/juliashtal/devanalytics/metrics/model/MetricType.java

@@ -44,7 +44,7 @@ public enum MetricType {
     public final boolean inAiContext;
     /** True when the metric represents a daily count to be summed over the period (not averaged). */
     public final boolean dailySum;
-    /** True when the metric is stored with periodFrom/periodTo instead of a date series. */
+    /** True when the metric is recomputed per ISO week; storage shape is a calculator property. */
     public final boolean aggregatePeriod;
 
     MetricType(boolean inAiContext, boolean dailySum, boolean aggregatePeriod) {

dev-analytics/src/main/resources/db/migration/V66__metric_snapshots_comment_refresh.sql

@@ -0,0 +1,27 @@
+-- V66: refresh the metric_snapshots table comment.
+--
+-- V57 restated V40's comment and is itself stale on two counts: it names
+-- MetricsService.saveMetric, which has been extracted to MetricSnapshotWriter, and its
+-- AGGREGATE list omits REVIEW_PARTICIPATION_COUNT and WIP_OPEN_PR_AGE_HOURS_MEDIAN.
+-- COMMENT ON TABLE has no partial form and V57 is frozen, so the comment is restated in full.
+-- Comment-only — no DDL, no data change, and re-running it is a no-op.
+
+COMMENT ON TABLE metric_snapshots IS
+'Persisted metric output. Two structurally distinct row shapes share this table:
+  DAILY (period_from IS NULL, period_to IS NULL):
+    date = calendar day measured; value = single-day figure.
+    Types: DAILY_COMMITS_COUNT, DAILY_COMMITS_AVG_SIZE, DAILY_PR_CREATED,
+           DAILY_PR_MERGED, DAILY_ISSUES_CREATED, DAILY_ISSUES_CLOSED,
+           DAILY_CHURN_RATIO, FOCUS_RATIO_DAYS_TASKS.
+  AGGREGATE (period_from IS NOT NULL, period_to IS NOT NULL):
+    date = snapshot capture date; period_from/period_to = calculation window.
+    Types: PR_LEAD_TIME_HOURS_MEDIAN, ISSUE_LEAD_TIME_HOURS_MEDIAN,
+           PR_FIRST_COMMIT_TO_MERGE_LEAD_TIME_HOURS_MEDIAN,
+           REVIEW_RESPONSE_TIME_HOURS_MEDIAN, REVIEW_PARTICIPATION_COUNT,
+           AFTER_HOURS_COMMIT_RATIO, DEEP_WORK_STREAK_DAYS, KNOWLEDGE_SILO_SCORE,
+           REFACTOR_RATIO, PR_SIZE_COMPLEXITY_SCORE, MERGE_WITHOUT_REVIEW_RATIO,
+           COMMITS_PER_WEEK_AVG, WIP_OPEN_PR_AGE_HOURS_MEDIAN.
+  The lists are exhaustive and disjoint: 8 DAILY + 13 AGGREGATE = 21 MetricType values.
+Storage shape is a property of the calculator, not of MetricType.aggregatePeriod, which
+selects ISO-week window resolution and is true for only five of the AGGREGATE types.
+All writes go through MetricSnapshotWriter (upsert guard). team_id NULL = personal.';

dev-analytics/src/test/java/com/juliashtal/devanalytics/metrics/calc/AggregateStorageShapeDriftTest.java

@@ -63,7 +63,7 @@ class AggregateStorageShapeDriftTest {
      * The metric types stored with {@code periodFrom}/{@code periodTo} — the contract the read
      * paths resolve against, and not derivable from {@code MetricType.aggregatePeriod}.
      */
-    private static final Set<MetricType> EXPECTED_PERIOD_STORED = Set.of(
+    static final Set<MetricType> EXPECTED_PERIOD_STORED = Set.of(
             PR_LEAD_TIME_HOURS_MEDIAN,
             PR_FIRST_COMMIT_TO_MERGE_LEAD_TIME_HOURS_MEDIAN,
             ISSUE_LEAD_TIME_HOURS_MEDIAN,

dev-analytics/src/test/java/com/juliashtal/devanalytics/metrics/calc/MetricSnapshotTableCommentTest.java

@@ -0,0 +1,86 @@
+package com.juliashtal.devanalytics.metrics.calc;
+
+import com.juliashtal.devanalytics.metrics.model.MetricType;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.jdbc.core.JdbcTemplate;
+
+import java.util.Arrays;
+import java.util.EnumSet;
+import java.util.Set;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Pins the {@code metric_snapshots} table comment against the calculators it describes.
+ *
+ * <p>The comment is the schema-level answer to "which shape is this metric stored in". Reading it
+ * back out of {@code pg_description} rather than restating it here is what makes an omission a
+ * build failure.</p>
+ */
+@SpringBootTest
+class MetricSnapshotTableCommentTest {
+
+    @Autowired JdbcTemplate jdbc;
+
+    private String comment;
+
+    @BeforeEach
+    void readComment() {
+        comment = jdbc.queryForObject(
+                "SELECT obj_description('metric_snapshots'::regclass)", String.class);
+        assertThat(comment).as("metric_snapshots must carry a table comment").isNotBlank();
+    }
+
+    @Test
+    void tableComment_aggregateSection_namesExactlyTheTypesCalculatorsStoreWithAPeriod() {
+        assertThat(typesIn(sectionAfter("AGGREGATE")))
+                .isEqualTo(AggregateStorageShapeDriftTest.EXPECTED_PERIOD_STORED);
+    }
+
+    @Test
+    void tableComment_dailySection_namesExactlyTheTypesNotStoredWithAPeriod() {
+        Set<MetricType> expectedDaily = EnumSet.complementOf(
+                EnumSet.copyOf(AggregateStorageShapeDriftTest.EXPECTED_PERIOD_STORED));
+        assertThat(typesIn(sectionBetween("DAILY", "AGGREGATE"))).isEqualTo(expectedDaily);
+    }
+
+    /** An omission is only detectable by exhaustiveness: a missing type is silently absent. */
+    @Test
+    void tableComment_bothSections_accountForEveryMetricType() {
+        Set<MetricType> documented = EnumSet.noneOf(MetricType.class);
+        documented.addAll(typesIn(sectionBetween("DAILY", "AGGREGATE")));
+        documented.addAll(typesIn(sectionAfter("AGGREGATE")));
+        assertThat(documented).containsExactlyInAnyOrder(MetricType.values());
+    }
+
+    /** Half of what V66 corrected was a class name; the type lists above do not cover it. */
+    @Test
+    void tableComment_writerReference_namesTheClassThatActuallyWrites() {
+        assertThat(comment).contains(MetricSnapshotWriter.class.getSimpleName());
+    }
+
+    private Set<MetricType> typesIn(String text) {
+        return Arrays.stream(MetricType.values())
+                .filter(t -> Pattern.compile("\\b" + t.name() + "\\b").matcher(text).find())
+                .collect(Collectors.toCollection(() -> EnumSet.noneOf(MetricType.class)));
+    }
+
+    private String sectionAfter(String marker) {
+        int i = comment.indexOf(marker);
+        assertThat(i).as("comment must contain a %s section", marker).isNotNegative();
+        return comment.substring(i);
+    }
+
+    private String sectionBetween(String start, String end) {
+        int i = comment.indexOf(start);
+        int j = comment.indexOf(end);
+        assertThat(i).as("comment must contain a %s section", start).isNotNegative();
+        assertThat(j).as("comment must contain a %s section", end).isGreaterThan(i);
+        return comment.substring(i, j);
+    }
+}

docs/metrics/knowledge-silo-score.md

@@ -35,7 +35,7 @@ knowledge_silo_score = MAX(share(R)) over all R with total_commits(R) > 0
 - Attribution for numerator: `author_github_id = user.githubUserId` **OR** `lower(author_email) IN user.commitEmails`. The denominator counts every author and is not attributed. See [author-attribution.md](author-attribution.md).
 - Denominator: all commits regardless of author — no email filter.
 - Bot exclusion: `author_name NOT LIKE '%[bot]%'`, applied to both the numerator and the denominator. The score measures how concentrated *human* ownership of a repository is, so automated commits belong in neither term: counting them in the denominator alone would deflate every contributor's share in proportion to how much CI writes to the repo. See [author-attribution.md](author-attribution.md).
-- Snapshots calculated before this policy took effect were computed against an unfiltered denominator and read lower. They are overwritten by the `MetricsService.saveMetric` upsert as each window is recalculated; no migration backfills them.
+- Snapshots calculated before this policy took effect were computed against an unfiltered denominator and read lower. They are overwritten by the `MetricSnapshotWriter` upsert as each window is recalculated; no migration backfills them.
 - Saved as aggregate shape: `periodFrom = fromDate`, `periodTo = toDate`, `repository = null`.
 
 ## Edge cases

docs/metrics/timezone.md

@@ -0,0 +1,121 @@
+# Timezones and Day Boundaries
+
+**Applies to:** every metric in this directory
+**Data source(s):** users.timezone, metric_snapshots.date, metric_coverage.date
+
+This page is the canonical definition of *which clock decides what "today" means*. Each metric
+document states which rule it uses and refers here for the rule itself, so the definition exists
+once rather than in nineteen partial copies. It is the time-side counterpart to
+[author-attribution.md](author-attribution.md).
+
+## Principle
+
+A figure computed from the same input history must not change because the platform was deployed on
+a machine in a different country. Wherever a calendar day has to be derived from an instant, the
+zone is stated explicitly; nothing reads the JVM default.
+
+Two different zones are correct in two different places, and conflating them is the failure this
+page exists to prevent.
+
+| Clock | Zone | Decides |
+|---|---|---|
+| **System clock** | always UTC | when a scheduled job fires, and which day it treats as "yesterday" |
+| **Attribution clock** | the user's `users.timezone` | which of *that user's* calendar days an activity belongs to, and what counts as their working hours |
+
+The system clock answers "what day is it for the platform". The attribution clock answers "what day
+was it for this developer". A developer in Auckland finishes a working day roughly thirteen hours
+before a UTC-only reading agrees that it has finished; both statements are true of different
+questions.
+
+## The system clock
+
+`SystemClock` wraps `Clock.system(ZoneOffset.UTC)` and is the only place a current instant enters
+the application. Its no-argument `today()` and `yesterday()` read UTC.
+
+Every cron-scheduled job declares `zone = "UTC"`, so the firing time is a property of the
+configuration rather than of the deployment:
+
+| Job | Schedule |
+|---|---|
+| `MetricsScheduler` | 01:00 UTC daily — computes yesterday for every user |
+| `TokenCleanupScheduler` | 02:00 UTC daily — deletes expired refresh and reset tokens |
+| `MetricBackfillScheduler` | 03:00 UTC daily — fills gaps in each user's collected history |
+| `MetricsSummaryScheduler` | 08:00 UTC Mondays — generates the weekly AI summary |
+
+Jobs declared with `fixedRate` or `fixedDelay` (`CommitStatsEnrichmentScheduler`,
+`SyncJobTracker`) carry no zone, because an interval has no wall-clock anchor to interpret.
+
+## The attribution clock
+
+`UserZone.of(user)` is the single reading of `users.timezone`. Two metrics and the backfill window
+depend on it:
+
+| Site | Uses the user's zone for |
+|---|---|
+| `AFTER_HOURS_COMMIT_RATIO` | classifying each commit's wall-clock hour and weekday |
+| `MetricBackfillService` | both ends of the window it fills — the earliest activity day, and the last day considered complete |
+| `POST /api/metrics/backfill` | rejecting a requested `to` that is not yet a finished day for the requester |
+
+The backfill guard and the backfill window read the same zone deliberately: otherwise the endpoint
+would refuse a day the nightly job already considered complete, or accept one it did not.
+
+### Resolving the zone
+
+`users.timezone` is `VARCHAR(64) NOT NULL DEFAULT 'Europe/Berlin'`. A user who never opens Settings
+is therefore attributed in **Europe/Berlin**, not UTC — the column default is the product decision,
+and UTC is only a defensive fallback:
+
+| Stored value | Resolves to |
+|---|---|
+| a valid IANA zone id | that zone |
+| unparseable | `ZoneOffset.UTC`, with a warning logged against the user id |
+| null or blank | `ZoneOffset.UTC` — unreachable through the database, which rejects null |
+
+DST is handled by `ZoneId` itself: each instant is offset by the rule in force at that instant, so
+a window spanning a transition is not skewed.
+
+### Changing a timezone does not recompute anything
+
+Unlike changing an identity, editing `users.timezone` does not invalidate stored snapshots. Rows
+already written under the previous zone survive, so an `AFTER_HOURS_COMMIT_RATIO` series can
+contain days classified under two different zones. The value is recomputed only when the day is
+recalculated for another reason.
+
+This is a deliberate asymmetry: an identity change alters *which records belong to the user*, which
+makes every stored figure wrong; a zone change alters only the interpretation of a boundary.
+
+## Storage
+
+`Instant` is used for stored timestamps and at API boundaries; the columns are `timestamptz`
+(migrations `V46`, `V55`), so Postgres preserves the offset and comparisons are unambiguous.
+`LocalDate` is used only for calendar-day windows — `metric_snapshots.date`, `period_from`,
+`period_to` — where the day has already been decided by one of the two clocks above. `LocalDateTime`
+is not used anywhere: it is an instant with the zone silently removed.
+
+## Known divergence: daily bucketing follows the database session zone
+
+The daily metrics bucket rows with `date(<timestamptz column>)` in the repository queries. On
+PostgreSQL that cast resolves in the **session** `TimeZone`, which the JDBC driver sets from the
+JVM default; `hibernate.jdbc.time_zone` is not configured. The day a commit is assigned to
+therefore follows the server's zone, not UTC.
+
+Observed directly against the project database for the instant `2026-03-14 23:30:00+00`:
+
+| JVM / session zone | `date(...)` returns |
+|---|---|
+| `UTC` | `2026-03-14` |
+| `America/Los_Angeles` | `2026-03-14` |
+| `Pacific/Auckland` | `2026-03-15` |
+
+The windows passed into those queries *are* UTC-explicit (`MetricsService` converts `from`/`to` with
+`atStartOfDay(ZoneOffset.UTC)`), so only the bucketing inside the window is affected. The specified
+rule for every daily metric remains UTC day boundaries, as each metric document states; the
+implementation does not yet meet it, and the affected metrics are:
+
+`DAILY_COMMITS_COUNT`, `DAILY_COMMITS_AVG_SIZE`, `DAILY_CHURN_RATIO`, `DAILY_PR_CREATED`,
+`DAILY_PR_MERGED`, `DAILY_ISSUES_CREATED`, `DAILY_ISSUES_CLOSED`, `DEEP_WORK_STREAK_DAYS`,
+`FOCUS_RATIO_DAYS_TASKS`.
+
+Two installations in different zones will disagree about which day a commit made near midnight
+belongs to. Within one installation the bucketing is self-consistent, so a single deployment's
+series is internally comparable.