diff --git a/AGENTS.md b/AGENTS.md index f9e43e53f..ae2ccdaa3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -84,6 +84,8 @@ uv run --all-extras python scripts/dump_schemas.py \ **Code-review detector output** — the `cr-*` detectors defer their `{"findings":[...]}` to `/workspace/tmp/subagent-output/-.json` (via `DeferredOutputMiddleware`, added in `_build_detector_middleware`); the review orchestrator passes those paths to `scripts/findings.py merge` instead of re-typing the JSON. The detector charters are unaware of this — they still just return the structured object. (A detector with no structured response — e.g. one the `LoopBreakerMiddleware` stopped — defers a `.txt` error file instead, which `findings.py merge` counts as a `skipped`/failed detector, never as empty findings.) +**Template comments** — Django's `{# … #}` comment is **single-line only** (the lexer's `tag_re` has no `DOTALL` flag). A `{# … #}` that spans a newline is NOT recognised as a comment: the text renders literally and any `{{ … }}` / `{% … %}` inside it gets evaluated. Use `{% comment %} … {% endcomment %}` for anything multi-line; reserve `{# … #}` for one-liners. + **Icons in templates** — never hand-roll an inline `` for a UI icon. Use `{% load icon_tags %}{% icon "name" "css-classes" %}`; see `DESIGN.md` §Icon System for the mechanism and the icon directory. Exceptions (keep inline): animated spinners, SVGs that need ``/Alpine `:class` on the element itself, and brand/logo `` tags. **Views split by content type** — server-rendered HTML (dashboard pages, forms) lives in `daiv//views.py` as **CBVs** subclassing `View` / `TemplateView` / `ListView` / `UpdateView` with `LoginRequiredMixin` / `AdminRequiredMixin`. JSON endpoints (including those consumed by dashboard JS like autocompletes and the agent-picker catalog) live in `daiv//api/views.py` (or `api/router.py` — both names exist) as a **django-ninja `Router`** with `auth=django_auth` for session callers, registered on the central `NinjaAPI` in `daiv/daiv/api.py` (`api.add_router("/", _router)`). Set `url_name="..."` on each route and reverse via `{% url 'api:' %}` from templates (or pass the URL into JS as an init prop instead of hardcoding `/api/...` paths); see `daiv/automation/api/views.py` + `_agent_picker.html` for the reference pair. diff --git a/daiv/accounts/context_processors.py b/daiv/accounts/context_processors.py index 87883a093..e61e2410d 100644 --- a/daiv/accounts/context_processors.py +++ b/daiv/accounts/context_processors.py @@ -103,18 +103,24 @@ def running_jobs_count(request, user) -> int: def feed_unread_attention_count(user) -> int: - """Count the user's unread Feed rows that still NEED ATTENTION (Story 2.4, Option 1). - - Envelope-aware, unlike the bell's flat unread count: a row counts only when its run still - exists AND its envelope is not ``all-clear``. A ``classifying`` run (terminal but no envelope - yet) counts — it needs attention until it resolves; an unread ``all-clear`` run does not (quiet - by design, so the badge never contradicts the "nothing needs you." seal); a row whose ``Run`` - was deleted does not. Three small queries, all scoped to the user's unread ``RUN_FEED`` rows - (per-user, low volume) — never an unscoped notification-table scan. + """Count the user's unread Feed rows that are still ACTIONABLE (Story 2.4 / Story 3.3). + + Routes the liveness decision through the single shared ``still_actionable`` predicate (AC2), so + the badge can never diverge from the Feed or the (future) Queue. A row counts only when its run + still exists AND ``still_actionable`` holds: an ``all-clear`` envelope is quiet (not counted, so + the badge never contradicts the "nothing needs you." seal); a ``classifying`` run (terminal, no + envelope yet) counts until it resolves; needs-attention / found-issues / failed count; a row + whose ``Run`` was deleted does not; and — new in Story 3.3 — a run whose MR was merged/closed + externally no longer counts (AC4/AC5), while a read failure keeps it counted (AC6, fail-safe). + + All queries are scoped to the user's unread ``RUN_FEED`` rows (per-user, low volume) — never an + unscoped notification-table scan. Runs + envelopes are batched (no N+1); only MR-referencing + runs trigger a live read, and cache-warm reads are free. """ from notifications.choices import EventType from notifications.models import Notification - from sessions.models import EnvelopeStatus, Run, RunEnvelope + from sessions.models import Run, RunEnvelope + from sessions.reconcile import still_actionable source_ids = list( Notification.objects.filter(recipient=user, event_type=EventType.RUN_FEED, read_at__isnull=True).values_list( @@ -123,15 +129,15 @@ def feed_unread_attention_count(user) -> int: ) if not source_ids: return 0 - existing = {str(pk) for pk in Run.objects.filter(pk__in=source_ids).values_list("pk", flat=True)} - all_clear = { - str(run_id) - for run_id in RunEnvelope.objects.filter(run_id__in=source_ids, status=EnvelopeStatus.ALL_CLEAR).values_list( - "run_id", flat=True - ) - } - # classifying (run exists, no envelope) is not in ``all_clear`` → counts. - return sum(1 for source_id in source_ids if source_id in existing and source_id not in all_clear) + runs = {str(run.pk): run for run in Run.objects.filter(pk__in=source_ids)} + # Batch the envelopes once; a missing entry (``None``) is the classifying state passed straight + # into ``still_actionable`` — no per-run re-query. + envelopes = {str(env.run_id): env for env in RunEnvelope.objects.filter(run_id__in=source_ids)} + return sum( + 1 + for source_id in source_ids + if (run := runs.get(str(source_id))) is not None and still_actionable(run, envelopes.get(str(source_id))) + ) def feed_unread_count(request) -> dict[str, Any]: diff --git a/daiv/accounts/locale/pt/LC_MESSAGES/django.po b/daiv/accounts/locale/pt/LC_MESSAGES/django.po index 560436135..731abd68e 100644 --- a/daiv/accounts/locale/pt/LC_MESSAGES/django.po +++ b/daiv/accounts/locale/pt/LC_MESSAGES/django.po @@ -7,7 +7,7 @@ msgid "" msgstr "" "Project-Id-Version: PACKAGE VERSION\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2026-07-19 13:16+0100\n" +"POT-Creation-Date: 2026-07-20 05:23-0500\n" "PO-Revision-Date: 2026-04-04 22:34+0100\n" "Last-Translator: \n" "Language-Team: Portuguese \n" @@ -83,8 +83,24 @@ msgstr "Introduza este código na janela do navegador para concluir a autentica msgid "Your DAIV Sign-In Code" msgstr "O Seu Código de Autenticação DAIV" -msgid "Today" -msgstr "Hoje" +msgid "What this counts" +msgstr "O que isto conta" + +msgid "Changes shipped = merged MRs opened by DAIV, counted from GitLab merges and attributed to you because each one links to one of your runs." +msgstr "Alterações entregues = MRs integrados abertos pelo DAIV, contados a partir das integrações do GitLab e atribuídos a ti porque cada um está ligado a uma das tuas execuções." + +#, python-format +msgid "open MR !%(iid)s on GitLab" +msgstr "abrir MR !%(iid)s no GitLab" + +msgid "view run" +msgstr "ver execução" + +msgid "No changes shipped in this range." +msgstr "Sem alterações entregues neste intervalo." + +msgid "Throughput" +msgstr "Entregas" msgid "Needs-me queue" msgstr "Fila de pendências" @@ -154,9 +170,87 @@ msgstr[1] "%(counter)s execuções sem problemas" msgid "next sweep %(sweep)s" msgstr "próxima verificação %(sweep)s" +#, python-format +msgid "last checked %(checked)s" +msgstr "última verificação %(checked)s" + msgid "No scheduled runs yet." msgstr "Ainda não há execuções agendadas." +msgid "today" +msgstr "hoje" + +msgid "vs yesterday" +msgstr "vs ontem" + +msgid "this week" +msgstr "esta semana" + +msgid "vs last week" +msgstr "vs semana passada" + +msgid "in the last 30 days" +msgstr "nos últimos 30 dias" + +msgid "vs last 30 days" +msgstr "vs últimos 30 dias" + +msgid "in the last 90 days" +msgstr "nos últimos 90 dias" + +msgid "vs last 90 days" +msgstr "vs últimos 90 dias" + +msgid "all time" +msgstr "desde sempre" + +#, python-format +msgid "%(n)s change shipped %(range_label)s" +msgid_plural "%(n)s changes shipped %(range_label)s" +msgstr[0] "%(n)s alteração entregue %(range_label)s" +msgstr[1] "%(n)s alterações entregues %(range_label)s" + +msgid "counted — every merged change" +msgstr "contado — cada alteração integrada" + +msgid "how this is computed" +msgstr "como isto é calculado" + +msgid "shipped since day one" +msgstr "entregues desde o início" + +#, python-format +msgid "~%(n)s dev-hours saved" +msgstr "~%(n)s horas de dev poupadas" + +msgid "est." +msgstr "est." + +#, python-format +msgid "DAIV shipped %(n)s changes %(range)s (%(total)s all-time)" +msgstr "A DAIV entregou %(n)s alterações %(range)s (%(total)s no total)" + +msgid "Copy" +msgstr "Copiar" + +msgid "Copied to clipboard" +msgstr "Copiado para a área de transferência" + +msgid "Copy failed" +msgstr "Falha ao copiar" + +msgid "Copy Slack summary" +msgstr "Copiar resumo para o Slack" + +msgid "Copied" +msgstr "Copiado" + +msgid "No changes shipped yet" +msgstr "Ainda sem alterações entregues" + +msgid "Your throughput appears here once DAIV merges its first change." +msgstr "As tuas entregas aparecem aqui assim que o DAIV fizer o primeiro merge." + msgid "Org impact" msgstr "Impacto da organização" @@ -282,6 +376,9 @@ msgstr "Nenhum utilizador encontrado." msgid "Time range" msgstr "Intervalo de tempo" +msgid "Today" +msgstr "Hoje" + msgid "7 days" msgstr "7 dias" diff --git a/daiv/accounts/templates/accounts/_clickthrough.html b/daiv/accounts/templates/accounts/_clickthrough.html new file mode 100644 index 000000000..98fe9af18 --- /dev/null +++ b/daiv/accounts/templates/accounts/_clickthrough.html @@ -0,0 +1,64 @@ +{% load i18n icon_tags %} +{% comment %} +Generic click-through breakdown panel (Story 3.2) — reused by Epic 4's Queue count (AC9). +HERO-AGNOSTIC: reads ONLY its include params, never a ``hero.``-prefixed context var, so Epic 4 +wires Queue rows in with no rework. + + {% include "accounts/_clickthrough.html" with rows=… number=… how_computed=… panel_id=… %} + +Params: + rows — iterable of row objects. Row contract (each item exposes, as attr or dict key): + · repo_id — repo tag (raw, mono chip) + · merge_request_iid — the MR/PR number (rendered ``!{iid}``, mono) + · title — human title (Geist Sans, truncate-tolerant) + · merged_at — merged timestamp (mono, tabular) + · web_url — best-effort out-link URL; blank → NO out-link, never ``#`` (AC11) + · thread_id — originating session for the same-tab drill-through + number — the count/label this panel is behind (contract param; not force-rendered). + how_computed — truthy renders the "how computed" AD-10 formula disclosure (AC2). + panel_id — the unique id the trigger's ``aria-controls`` points at (a11y, AC10). + +Flat INLINE surface (surface-2 + hairline) — NOT a floating popover, so no overlay shadow. Links +are teal ``accent`` NAVIGATION — never a ``button-*`` action, never a status colour. Icons via +``{% templatetag openblock %} icon {% templatetag closeblock %}`` only (no inline ````). +{% endcomment %} +
+ {% if how_computed %} +
+

+ {% translate "What this counts" %} +

+

+ {% blocktranslate %}Changes shipped = merged MRs opened by DAIV, counted from GitLab merges and attributed to you because each one links to one of your runs.{% endblocktranslate %} +

+
+ {% endif %} +
+ +
+
diff --git a/daiv/accounts/templates/accounts/_console_body.html b/daiv/accounts/templates/accounts/_console_body.html index 86ed44d77..af6faac80 100644 --- a/daiv/accounts/templates/accounts/_console_body.html +++ b/daiv/accounts/templates/accounts/_console_body.html @@ -8,20 +8,19 @@ shimmer skeletons are aria-hidden so assistive tech is not read empty rows. {% endcomment %}
- {# ── Hero ─────────────────────────────────────────────────────────────── #} -

- {% translate "Today" %} + {% translate "Throughput" %}

- + {% include "accounts/_hero.html" %}
{# ── Needs-me queue ───────────────────────────────────────────────────── #} @@ -50,8 +49,8 @@

{% translate "what happened" %} - {# Persistent aria-live container: only its inner fragment swaps on ``feed:seen`` so the live - region survives and the change (including to 0) is announced. #} + {% comment %} Persistent aria-live container: only its inner fragment swaps on ``feed:seen`` so the live + region survives and the change (including to 0) is announced. {% endcomment %} {% include "accounts/_feed_unread_badge.html" %} diff --git a/daiv/accounts/templates/accounts/_feed.html b/daiv/accounts/templates/accounts/_feed.html index e203f780f..e5dffea9e 100644 --- a/daiv/accounts/templates/accounts/_feed.html +++ b/daiv/accounts/templates/accounts/_feed.html @@ -17,6 +17,16 @@ {% include "accounts/_feed_item.html" with item=item %} {% endfor %}

+ {% comment %} Reconcile freshness (Story 3.3, AC8): rendered ONLY alongside the attention list and + ONLY when a live MR-state read actually happened this render (``reconciled_at`` is gated in the + view — review P2 / NFR1). This avoids both duplicating the zero-state's own "last checked" meta + (_feed_zero_state.html) and claiming a source-of-truth check that never occurred. Presentation-only. + {% endcomment %} + {% if reconciled_at %} +

+ {% blocktranslate with checked=reconciled_at|date:"H:i" %}last checked {{ checked }}{% endblocktranslate %} +

+ {% endif %} {% else %} {% include "accounts/_feed_zero_state.html" with zero=feed_zero %} {% endif %} diff --git a/daiv/accounts/templates/accounts/_feed_item.html b/daiv/accounts/templates/accounts/_feed_item.html index 7a970878f..6ffa5f6b8 100644 --- a/daiv/accounts/templates/accounts/_feed_item.html +++ b/daiv/accounts/templates/accounts/_feed_item.html @@ -22,9 +22,9 @@ {% if item.status_slug == "classifying" %}aria-live="polite" aria-busy="true"{% endif %} class="flex items-stretch rounded-md border border-l-2 border-border bg-surface-2{% if item.status_slug == 'classifying' %} border-dashed{% endif %}{% if not item.read_at %} bg-white/[0.02]{% endif %}" style="{% if item.status_slug == 'all-clear' %}opacity: 0.92; border-left-color: var(--color-status-clear);{% else %}{% if item.read_at %}opacity: 0.7;{% endif %}{% if item.accent_var %}border-left-color: var({{ item.accent_var }});{% endif %}{% endif %}"> - {# Only an unmodified left-click marks-seen-then-navigates via htmx; ctrl/cmd/shift/middle-click + {% comment %} Only an unmodified left-click marks-seen-then-navigates via htmx; ctrl/cmd/shift/middle-click fall through to the native ``href`` (open in a new tab) WITHOUT marking seen — restoring 2.3's - plain- behaviour that the bare ``hx-post`` (fires on every click) regressed. #} + plain- behaviour that the bare ``hx-post`` (fires on every click) regressed. {% endcomment %} - {# Dismiss (mark-seen) — resolved-only, unread-only. A seen-state control, NOT an Epic 5 action. + {% comment %} Dismiss (mark-seen) — resolved-only, unread-only. A seen-state control, NOT an Epic 5 action. Single accessible name (aria-label); 44px touch target; inherits the global teal focus ring. The swap replaces this
with its seen version (which has NO dismiss button), so focus would fall to ; after the swap, move focus back onto the swapped-in article (same ``#feed-item-{id}``, made focusable by ``tabindex="-1"``) so keyboard/AT users keep their place - (WCAG 2.4.3). The global ``:focus-visible`` ring supplies the visible indicator. #} + (WCAG 2.4.3). The global ``:focus-visible`` ring supplies the visible indicator. {% endcomment %} {% if not item.read_at and item.status_slug != "classifying" %} + {% if hero.delta is not None %} + {# Delta chip stays INERT — a plain

, never a click target (never wrapped in the trigger). #} +

+ {% if hero.delta > 0 %}+{{ hero.delta }}{% elif hero.delta < 0 %}{{ hero.delta }}{% else %}0{% endif %} {{ delta_label }} +

+ {% endif %} + + + {# Fact caption + the mockup's muted "how this is computed" entry point — toggles the SAME panel. #} +

+ {% translate "counted — every merged change" %} + +

+ + {% comment %} Inline breakdown panel — flat surface below the fact (no overlay shadow), reflows on <768px, + re-renders with #console-main on a range change. One panel serves AC1 (rows) + AC2 (how). {% endcomment %} +
+ {% include "accounts/_clickthrough.html" with rows=hero.this_range_rows number=hero.this_range.total_merges how_computed=True panel_id="hero-breakdown-range" %} +
+ + + {# ── All-time odometer (range-invariant) — click-through trigger ─────────── #} + {% comment %} Its OWN Alpine scope + panel (range-invariant all-time rows). The odometer element is itself the + button (keeps the ``hero-odometer`` testid + the ``.animate-number`` span). ``how_computed`` is + omitted here — the headline panel already carries the AD-10 disclosure. {% endcomment %} +
+ +
+ {% include "accounts/_clickthrough.html" with rows=hero.all_time_rows number=hero.all_time how_computed=False panel_id="hero-breakdown-alltime" %} +
+
+ + {# ── Demoted estimate (D1: OMITTED in v1 — falsy ``estimate`` renders nothing) ── #} + {% if hero.estimate %} +

+ {% blocktranslate with n=hero.estimate %}~{{ n }} dev-hours saved{% endblocktranslate %} + {% translate "est." %} +

+ {% endif %} + + {# ── Copyable Slack one-liner (own overflow-x-auto container; Alpine clipboard, no new JS) ── #} + {% blocktranslate asvar slack_line with n=hero.this_range.total_merges range=range_label total=hero.all_time %}DAIV shipped {{ n }} changes {{ range }} ({{ total }} all-time){% endblocktranslate %} + {{ slack_line|json_script:"hero-slack-line" }} +
+
+ {{ slack_line }} +
+ +
+ +{% else %} +{# Honest empty state (AC13): nothing shipped all-time — a quiet onboarding line, never a fake 0. #} +
+

{% translate "No changes shipped yet" %}

+

{% translate "Your throughput appears here once DAIV merges its first change." %}

+
+{% endif %} diff --git a/daiv/accounts/templates/accounts/dashboard.html b/daiv/accounts/templates/accounts/dashboard.html index aff3bf373..625895c60 100644 --- a/daiv/accounts/templates/accounts/dashboard.html +++ b/daiv/accounts/templates/accounts/dashboard.html @@ -14,32 +14,56 @@ {# Admin-gated console surface toggle, active on "Personal" (the default); absent for non-admins (AC2). #} {% include "accounts/_manager_lens_toggle.html" with surface="personal" %} - {% comment %} - Range-switcher — console-only, static placeholder in 2.1; wired in Epic 3. - Active pill is teal; labels use the mono face + tabular figures. - {% endcomment %} - {% endblock topbar_start %} diff --git a/daiv/accounts/views.py b/daiv/accounts/views.py index a47978142..3483cd3af 100644 --- a/daiv/accounts/views.py +++ b/daiv/accounts/views.py @@ -5,10 +5,11 @@ from django.contrib.auth.mixins import LoginRequiredMixin from django.contrib.messages.views import SuccessMessageMixin from django.db import IntegrityError -from django.db.models import Avg, Count, DurationField, ExpressionWrapper, F, Q, Sum +from django.db.models import Avg, Count, DurationField, Exists, ExpressionWrapper, F, OuterRef, Q, Subquery, Sum from django.http import Http404 from django.shortcuts import redirect, render from django.urls import reverse, reverse_lazy +from django.utils import timezone from django.utils.decorators import method_decorator from django.utils.timezone import localdate from django.views import View @@ -19,6 +20,7 @@ from notifications.choices import EventType from notifications.models import Notification from sessions.models import EnvelopeStatus, Run, RunEnvelope, RunStatus, SessionOrigin +from sessions.reconcile import still_actionable from accounts.context_processors import running_jobs_count from accounts.emails import send_welcome_email @@ -83,7 +85,10 @@ def get_context_data(self, **kwargs): ("all", "All time", None), ] PERIOD_DAYS = {key: days for key, _, days in PERIOD_CHOICES} -DEFAULT_PERIOD = "today" +# The personal console lands on the week (Story 3.1 / D4): a weekly throughput number is +# screenshot-worthy where a daily one is not, and it satisfies the FR-16 "this week" headline. +# ``ManagerLensView`` is unaffected — it passes ``default="all"`` explicitly. +DEFAULT_PERIOD = "7d" def _raw_pct(numerator: int, denominator: int) -> int | None: @@ -117,7 +122,7 @@ def _resolve_period(request, default: str = DEFAULT_PERIOD) -> tuple[str, date | """Resolve the stateless ``?period=`` querystring to a ``(period_key, cutoff_date)`` pair. Falls back to ``default`` for a missing/invalid value — the personal console uses the module - ``DEFAULT_PERIOD`` ("today"), while the org ``ManagerLensView`` passes ``default="all"`` because + ``DEFAULT_PERIOD`` ("7d"), while the org ``ManagerLensView`` passes ``default="all"`` because org velocity is cumulative. ``cutoff_date`` is ``None`` for the "all time" period, today's date for the zero-day "today" period, and ``today - days`` otherwise. Shared by both console surfaces so they honour the same range semantics with no persisted state. @@ -132,15 +137,67 @@ def _resolve_period(request, default: str = DEFAULT_PERIOD) -> tuple[str, date | return period, cutoff_date -def get_velocity_data(cutoff_date: date | None) -> dict | None: - """Aggregate org-wide code-velocity + DAIV-attribution from ``MergeMetric``. +def _shipped_metrics_for(user: User): + """Return the ``MergeMetric`` queryset of merges opened by DAIV **for this user** (AR9 / AD-10). - Org-wide by design — ``MergeMetric.objects.all()`` (never user-scoped, never derived from - ``RunEnvelope`` per AD-10); an optional ``cutoff_date`` restricts to merges on/after that date. - Returns ``None`` when there are zero matching rows so the caller can render an honest cold-load - state instead of a misleading zero reading. Single source of truth for the Manager Lens. + A merge counts as "shipped by DAIV, by this user" iff a ``Run`` in one of the user's own + sessions shares the merge's ``(repo_id, merge_request_iid)`` — the value-based ``Exists`` join + (there is no FK) *is* the attribution predicate, so human-authored MRs (no matching DAIV ``Run``) + are excluded. Uses the literal ``session__user=user`` — never ``Run.objects.by_owner`` / + ``visible_to``, which short-circuit to ``.all()`` for admins and would break personal scope (AC10). """ - merges = MergeMetric.objects.all() + daiv_user_runs = Run.objects.filter( + session__user=user, + merge_request_iid__isnull=False, + repo_id=OuterRef("repo_id"), + merge_request_iid=OuterRef("merge_request_iid"), + ) + return MergeMetric.objects.filter(Exists(daiv_user_runs)) + + +def _hero_breakdown_rows(shipped, user: User, cutoff_date: date | None) -> tuple[list, list]: + """Annotate the exact ``MergeMetric`` rows behind the Hero counts (AC1/AC5, read-only). + + Derived from the SAME ``shipped`` base ``get_velocity_data`` aggregates, so the revealed list + can never diverge from the number (NFR1): a ``.count()`` and this ``.annotate()`` over one base + queryset return the identical set. Each row carries its native ``MergeMetric`` fields + (``repo_id`` / ``merge_request_iid`` / ``title`` / ``merged_at``) plus two annotation-only names + joined from the most-recent attributing ``Run`` in one of the USER's own sessions: + + - ``web_url`` — the already-persisted ``Run.merge_request_web_url`` (no live client, AC6/AC11); + may be ``""`` (blank, best-effort) → the template degrades to the session link. + - ``thread_id`` — the representative session for the drill-through. + + The correlation is the literal ``session__user=user`` scope — never ``by_owner`` / ``visible_to`` + (which broaden to ``.all()`` for admins, AC7). The this-range list applies the IDENTICAL window + filter ``get_velocity_data`` uses (``merged_at__date__gte=cutoff_date`` when dated; unfiltered for + "all"), so it matches the headline count; the unfiltered list matches the odometer (AC8). + """ + attributing_run = Run.objects.filter( + session__user=user, repo_id=OuterRef("repo_id"), merge_request_iid=OuterRef("merge_request_iid") + ).order_by("-created_at", "-pk") # ``-pk`` is a deterministic tiebreak on a created_at collision + rows = shipped.annotate( + # Prefer an attributing run that actually PERSISTED a url: a blank best-effort url on the + # newest re-run must not hide a good url carried by an older run (AC11 degrades to the session + # link only when NO attributing run has one). ``thread_id`` (the session PK) is always present. + web_url=Subquery(attributing_run.exclude(merge_request_web_url="").values("merge_request_web_url")[:1]), + thread_id=Subquery(attributing_run.values("session__thread_id")[:1]), + ).order_by("-merged_at") + this_range_rows = rows.filter(merged_at__date__gte=cutoff_date) if cutoff_date is not None else rows + return list(this_range_rows), list(rows) + + +def get_velocity_data(cutoff_date: date | None, queryset=None) -> dict | None: + """Aggregate code-velocity + DAIV-attribution over a ``MergeMetric`` queryset. + + The single shared counting body (AC11): the Manager Lens passes nothing (defaulting to the + org-wide ``MergeMetric.objects.all()``, never derived from ``RunEnvelope`` per AD-10), while the + personal Hero passes the user-scoped ``_shipped_metrics_for(user)`` queryset — so the org number + and the personal number can never disagree in method. An optional ``cutoff_date`` restricts to + merges on/after that date. Returns ``None`` when there are zero matching rows so the caller can + render an honest cold-load state instead of a misleading zero reading. + """ + merges = MergeMetric.objects.all() if queryset is None else queryset if cutoff_date is not None: merges = merges.filter(merged_at__date__gte=cutoff_date) @@ -320,14 +377,74 @@ def get_context_data(self, **kwargs): # (``ManagerLensView``); every read here is personal-scoped via ``visible_to`` / ``user=``. user = self.request.user context["activity"] = self._get_activity_data(cutoff_date, user) + context["hero"] = self._get_hero_data(user, cutoff_date, period) context["active_api_keys"] = APIKey.objects.filter(user=user, revoked=False).count() context["periods"] = [{"key": key, "label": label} for key, label, _ in PERIOD_CHOICES] context["current_period"] = period context["active_schedules"] = ScheduledJob.objects.filter(user=user, is_enabled=True).count() - context.update(self._get_feed_data(user)) + feed = self._get_feed_data(user) + reconciled = feed.pop("feed_reconciled") + context.update(feed) + # Reconciliation happens at render time (AD-6 live/cached read). Only stamp "last checked" + # when a live MR-state read ACTUALLY occurred this render (>=1 actionable MR run) — a feed + # with nothing to reconcile must not claim a source-of-truth check that never happened + # (review P2 / NFR1). Read-only / presentation-only: passed into context, never stored. + context["reconciled_at"] = timezone.now() if reconciled else None return context + def _get_hero_data(self, user: User, cutoff_date: date | None, period: str) -> dict | None: + """Compose the personal Throughput Hero from the user-scoped ``MergeMetric ⋈ Run`` join. + + Read-only (``.count()`` / ``.aggregate()`` only — AC12): nothing is written on the render + path. Returns ``None`` when the user has shipped nothing all-time so the template renders the + honest empty state rather than a ``0`` dressed as a headline fact (AC13); a genuine "0 this + range" with a non-zero all-time is a true fact and *is* rendered. + + - ``this_range`` — the range-scoped count through the SHARED ``get_velocity_data`` body + (never a forked count, AC11); ``0`` when the window is empty but all-time is not. + - ``delta`` — ``this_range − prev_equal_window`` where the previous window spans the same + number of days as the current [cutoff, today] window (``PERIOD_DAYS[period] + 1``); + ``None`` for the "all time" range (no prior window) so the template hides the chip. + - ``all_time`` — the range-invariant odometer count. + - ``estimate`` — ``None`` in v1 (D1: no defensible dev-hours field exists); the key is kept + so the template's ``{% if hero.estimate %}`` demotion path stays exercisable. + """ + shipped = _shipped_metrics_for(user) + all_time = shipped.count() + if not all_time: + return None + + this_range = get_velocity_data(cutoff_date, queryset=shipped) + this_count = this_range["total_merges"] if this_range else 0 + + delta = None + if period != "all" and cutoff_date is not None: + # The current window ``merged_at__date >= cutoff_date`` is [cutoff, today] *inclusive*, + # spanning ``PERIOD_DAYS[period] + 1`` days ("7d" cutoff = today−7 ⇒ 8 days; "today" ⇒ 1). + # The preceding window must span the SAME number of days, else a flat one-merge-per-day + # cadence reports a spurious +1 (an 8-day current window vs a 7-day previous one). + window_span = (PERIOD_DAYS[period] or 0) + 1 + prev_start = cutoff_date - timedelta(days=window_span) + prev_count = shipped.filter(merged_at__date__gte=prev_start, merged_at__date__lt=cutoff_date).count() + delta = this_count - prev_count + + # Story 3.2 — attach the exact underlying rows behind each number, derived from the SAME + # ``shipped`` base with the identical window filter, so the click-through breakdown always + # reconciles with the count (``len(this_range_rows) == this_count``, ``len(all_time_rows) == + # all_time`` by construction). Read-only annotate/list only — no live client (AC5/AC6/AC11). + this_range_rows, all_time_rows = _hero_breakdown_rows(shipped, user, cutoff_date) + + return { + "this_range": {"total_merges": this_count}, + "period": period, + "delta": delta, + "all_time": all_time, + "estimate": None, + "this_range_rows": this_range_rows, + "all_time_rows": all_time_rows, + } + def _get_feed_data(self, user: User) -> dict: """Build the console Feed: RUN_FEED rows newest-first, each rendered from its live envelope. @@ -348,12 +465,27 @@ def _get_feed_data(self, user: User) -> dict: items: list[dict] = [] in_flight_ids: list[str] = [] latest_finished = None + feed_reconciled = False for notification in notifications: run = runs_by_id.get(notification.source_id) if run is None: # The run was deleted out from under a stale Feed row; skip it rather than 500. continue - item = build_feed_item(run, notification, envelope=envelopes_by_run.get(str(run.id))) + envelope = envelopes_by_run.get(str(run.id)) + # Reconcile against live MR state (Story 3.3, AC4/AC5). A run that WAS actionable but + # whose MR resolved externally (merged/closed) leaves the surface — it no longer counts + # toward attention nor renders as awaiting. An ``all-clear`` run (never actionable) is + # NOT dropped; it stays as a quiet card, exactly as before. A read failure keeps the item + # visible (AC6, fail-safe) since ``still_actionable`` then resolves to actionable. + classification_actionable = envelope is None or envelope.is_actionable + # A live MR-state read is performed iff the run is (classification-)actionable AND + # references an MR (see reconcile.still_actionable). Track it so the view stamps + # "last checked" only when a real reconciliation happened this render (review P2 / NFR1). + if classification_actionable and run.merge_request_iid is not None: + feed_reconciled = True + if classification_actionable and not still_actionable(run, envelope): + continue + item = build_feed_item(run, notification, envelope=envelope) items.append(item) if item["status_slug"] == "classifying": in_flight_ids.append(str(run.id)) @@ -389,26 +521,28 @@ def _get_feed_data(self, user: User) -> dict: "feed_in_flight_ids": ",".join(in_flight_ids), "feed_has_attention": has_attention, "feed_zero": zero, + "feed_reconciled": feed_reconciled, } @staticmethod def _feed_attention_summary(feed_qs) -> tuple[bool, int]: """Return ``(has_attention, all_clear_count)`` over the FULL feed set. - ``has_attention`` is True when any feed run is classifying (no envelope yet) or has a - non-all-clear envelope. Called only when the rendered page is entirely all-clear, to avoid a - false zero-state seal that would hide a finding paged past ``FEED_PAGE_SIZE`` and to count - all-clear runs across the whole feed (not just the page). + ``has_attention`` routes every existing feed run through the shared ``still_actionable`` + predicate (Story 3.3, AC2) — a classifying run, a non-all-clear envelope, or an open/unknown + live MR read all mean attention; an all-clear envelope or an externally-resolved MR does not + — so the zero-state seal decision can never diverge from the badge or the per-item render. + Called only when the rendered page is entirely quiet, to avoid a false seal that would hide a + finding paged past ``FEED_PAGE_SIZE`` and to count all-clear runs across the whole feed. """ run_ids = list(feed_qs.values_list("source_id", flat=True)) if not run_ids: return False, 0 - statuses = list(RunEnvelope.objects.filter(run_id__in=run_ids).values_list("status", flat=True)) - all_clear_count = sum(1 for status in statuses if status == EnvelopeStatus.ALL_CLEAR) - # Fewer envelopes than feed runs ⇒ at least one run is still classifying (no envelope) — don't seal. - classifying = len(statuses) < len(run_ids) - has_non_all_clear = any(status != EnvelopeStatus.ALL_CLEAR for status in statuses) - return (classifying or has_non_all_clear), all_clear_count + runs_by_id = Run.objects.filter(pk__in=run_ids).in_bulk() + envelopes_by_run = {str(env.run_id): env for env in RunEnvelope.objects.filter(run_id__in=run_ids)} + all_clear_count = sum(1 for env in envelopes_by_run.values() if env.status == EnvelopeStatus.ALL_CLEAR) + has_attention = any(still_actionable(run, envelopes_by_run.get(str(run.id))) for run in runs_by_id.values()) + return has_attention, all_clear_count @staticmethod def _next_sweep(user: User): diff --git a/daiv/codebase/base.py b/daiv/codebase/base.py index 764336dfb..1a26c0369 100644 --- a/daiv/codebase/base.py +++ b/daiv/codebase/base.py @@ -14,6 +14,31 @@ class GitPlatform(StrEnum): SWE = "swe" +class MergeRequestState(StrEnum): + """Live lifecycle state of a merge/pull request, normalized across platforms. + + Presentation-only (AC1): read live from the platform for render-time reconciliation and + never persisted — so this is a plain ``StrEnum``, NOT a ``models.TextChoices`` with a + DB ``CheckConstraint``. The ``resolved()`` / ``live()`` groupings mirror the + ``RunStatus.terminal()`` classmethod idiom (AR14), returning ``frozenset[str]``. + """ + + OPEN = "open" + MERGED = "merged" + CLOSED = "closed" + DRAFT = "draft" + + @classmethod + def resolved(cls) -> frozenset[str]: + """States that mean the MR was settled externally — an item in one leaves the console.""" + return frozenset({cls.MERGED, cls.CLOSED}) + + @classmethod + def live(cls) -> frozenset[str]: + """States that mean the MR is still open work — an item in one stays on the console.""" + return frozenset({cls.OPEN, cls.DRAFT}) + + class Scope(StrEnum): """ Scope of the conversation. diff --git a/daiv/codebase/clients/base.py b/daiv/codebase/clients/base.py index 25ead1c1d..77a4943ce 100644 --- a/daiv/codebase/clients/base.py +++ b/daiv/codebase/clients/base.py @@ -20,6 +20,7 @@ MergeRequest, MergeRequestCommit, MergeRequestDiffStats, + MergeRequestState, RepoAccessLevel, RepoMember, Repository, @@ -419,6 +420,24 @@ def get_bot_commit_email(self) -> str: def get_merge_request(self, repo_id: str, merge_request_id: int) -> MergeRequest: pass + @abc.abstractmethod + def get_merge_request_state(self, repo_id: str, merge_request_id: int) -> MergeRequestState: + """Read the live lifecycle state (open / merged / closed / draft) of a merge/pull request. + + Presentation-only trust-baseline read (AC1): returns the current platform state so the + console can reconcile against source of truth at render time. Providers RAISE their native + API error on failure (``GitlabError`` / ``GithubException``); the cached wrapper in + :mod:`codebase.mr_state` owns the fail-safe (AC6), not this method. + + Args: + repo_id: The repository ID. + merge_request_id: The merge request IID (GitLab) or pull request number (GitHub). + + Returns: + The live :class:`~codebase.base.MergeRequestState`. + """ + pass + @abc.abstractmethod def get_merge_request_by_branches( self, repo_id: str, source_branch: str, target_branch: str diff --git a/daiv/codebase/clients/github/api/callbacks.py b/daiv/codebase/clients/github/api/callbacks.py index 84996f488..93c5788e5 100644 --- a/daiv/codebase/clients/github/api/callbacks.py +++ b/daiv/codebase/clients/github/api/callbacks.py @@ -2,6 +2,8 @@ from functools import cached_property from typing import Any, Literal +from django.core.cache import cache + from asgiref.sync import sync_to_async from github.GithubException import GithubException from sandbox_envs.services import resolve_env_for_run @@ -13,6 +15,7 @@ from codebase.base import Scope from codebase.clients import RepoClient from codebase.clients.base import Emoji +from codebase.mr_state import _mr_state_cache_key from codebase.repo_config import RepositoryConfig from codebase.tasks import address_issue_task, address_mr_comments_task from codebase.utils import compute_thread_id, note_mentions_daiv @@ -319,6 +322,10 @@ async def process_callback(self): merged_at=self.pull_request.merged_at or "", platform="github", ) + # Eagerly drop the cached live MR state so the merged PR leaves the console near-instantly + # rather than after the TTL (AC7). Pure cache side effect — no accept-gate change, no new + # stored state; the same posture as ``PushCallback`` invalidating the repo-config cache. + cache.delete(_mr_state_cache_key(self.repository.full_name, self.pull_request.number)) class PushCallback(GitHubCallback): diff --git a/daiv/codebase/clients/github/client.py b/daiv/codebase/clients/github/client.py index 37413acf4..1c88f21f5 100644 --- a/daiv/codebase/clients/github/client.py +++ b/daiv/codebase/clients/github/client.py @@ -20,6 +20,7 @@ MergeRequest, MergeRequestCommit, MergeRequestDiffStats, + MergeRequestState, Note, NoteableType, NoteDiffPosition, @@ -755,6 +756,24 @@ def get_merge_request(self, repo_id: str, merge_request_id: int) -> MergeRequest author=User(id=mr.user.id, username=mr.user.login, name=mr.user.name), ) + def get_merge_request_state(self, repo_id: str, merge_request_id: int) -> MergeRequestState: + """Read the live lifecycle state of a pull request (AC1, AC5). + + GitHub layers ``merged`` / ``draft`` booleans on top of ``state ∈ {open, closed}``: a merged + PR is ``MERGED``; a non-merged closed PR is ``CLOSED``; an open draft is ``DRAFT``; else + ``OPEN``. Raises ``GithubException`` on API failure — the cached wrapper in + :mod:`codebase.mr_state` owns the fail-safe. + """ + repo = self.client.get_repo(repo_id, lazy=True) + pr = repo.get_pull(merge_request_id) + if pr.merged: + return MergeRequestState.MERGED + if pr.state == "closed": + return MergeRequestState.CLOSED + if pr.draft: + return MergeRequestState.DRAFT + return MergeRequestState.OPEN + def get_merge_request_comment(self, repo_id: str, merge_request_id: int, comment_id: str) -> Discussion: """ Get a comment from a merge request. diff --git a/daiv/codebase/clients/gitlab/api/callbacks.py b/daiv/codebase/clients/gitlab/api/callbacks.py index eee61dec1..921ea07d2 100644 --- a/daiv/codebase/clients/gitlab/api/callbacks.py +++ b/daiv/codebase/clients/gitlab/api/callbacks.py @@ -2,6 +2,8 @@ from functools import cached_property from typing import Any, Literal +from django.core.cache import cache + from gitlab.exceptions import GitlabError from sandbox_envs.services import resolve_env_for_run from sessions.models import SessionOrigin @@ -12,6 +14,7 @@ from codebase.base import Scope from codebase.clients import RepoClient from codebase.clients.base import Emoji +from codebase.mr_state import _mr_state_cache_key from codebase.repo_config import RepositoryConfig from codebase.tasks import address_issue_task, address_mr_comments_task from codebase.utils import compute_thread_id, note_mentions_daiv @@ -337,6 +340,10 @@ async def process_callback(self): merged_at=self.object_attributes.merged_at or "", platform="gitlab", ) + # Eagerly drop the cached live MR state so the merged MR leaves the console near-instantly + # rather than after the TTL (AC7). Pure cache side effect — no accept-gate change, no new + # stored state; the same posture as ``PushCallback`` invalidating the repo-config cache. + cache.delete(_mr_state_cache_key(self.project.path_with_namespace, self.object_attributes.iid)) class PushCallback(BaseCallback): diff --git a/daiv/codebase/clients/gitlab/client.py b/daiv/codebase/clients/gitlab/client.py index c81c35485..5d86809fd 100644 --- a/daiv/codebase/clients/gitlab/client.py +++ b/daiv/codebase/clients/gitlab/client.py @@ -23,6 +23,7 @@ MergeRequest, MergeRequestCommit, MergeRequestDiffStats, + MergeRequestState, Note, NoteDiffPosition, NotePosition, @@ -901,6 +902,28 @@ def get_merge_request(self, repo_id: str, merge_request_id: int) -> MergeRequest author=User(id=mr.author.get("id"), username=mr.author.get("username"), name=mr.author.get("name")), ) + def get_merge_request_state(self, repo_id: str, merge_request_id: int) -> MergeRequestState: + """Read the live lifecycle state of a merge request (AC1, AC5). + + Maps GitLab's raw ``state`` (``opened`` / ``merged`` / ``closed`` / ``locked``) plus the + ``work_in_progress`` flag (the same flag ``_serialize_merge_request`` maps to ``draft``) onto + the normalized :class:`MergeRequestState`: ``locked`` is TRANSIENT (GitLab locks an MR while a + merge is being processed and reverts to ``opened`` if it fails), so it is NOT a confirmed + resolution — it maps to ``OPEN`` and the item stays visible until a confirmed ``merged`` / + ``closed`` (AC6, under-claim never over-claim). An ``opened`` WIP MR is a ``DRAFT``. Raises + ``GitlabError`` on API failure — the cached wrapper in :mod:`codebase.mr_state` owns the + fail-safe. + """ + project = self.client.projects.get(repo_id, lazy=True) + mr = project.mergerequests.get(merge_request_id) + if mr.state == "merged": + return MergeRequestState.MERGED + if mr.state == "closed": + return MergeRequestState.CLOSED + if mr.work_in_progress: + return MergeRequestState.DRAFT + return MergeRequestState.OPEN + def get_merge_request_comment(self, repo_id: str, merge_request_id: int, comment_id: str) -> Discussion: """ Get a comment from a merge request. diff --git a/daiv/codebase/clients/swe.py b/daiv/codebase/clients/swe.py index 517c0dc99..50461090b 100644 --- a/daiv/codebase/clients/swe.py +++ b/daiv/codebase/clients/swe.py @@ -17,6 +17,7 @@ MergeRequest, MergeRequestCommit, MergeRequestDiffStats, + MergeRequestState, RepoMember, Repository, User, @@ -335,6 +336,15 @@ def get_merge_request(self, repo_id: str, merge_request_id: int) -> MergeRequest """Not supported for SWE client.""" raise NotImplementedError("SWERepoClient does not support merge requests") + def get_merge_request_state(self, repo_id: str, merge_request_id: int) -> MergeRequestState: + """SWE runs have no live MR lifecycle: report ``OPEN`` (unresolved → keep visible, AC6). + + Returning a benign default rather than raising keeps AC6's fail-safe intact without the + cached wrapper logging an exception on every reconcile of a SWE install — the same graceful + posture as :meth:`get_merge_request_by_branches` returning ``None``. Genuine GitLab/GitHub + API failures still raise from their clients and are caught by the wrapper.""" + return MergeRequestState.OPEN + def get_merge_request_by_branches( self, repo_id: str, source_branch: str, target_branch: str ) -> MergeRequest | None: diff --git a/daiv/codebase/mr_state.py b/daiv/codebase/mr_state.py new file mode 100644 index 000000000..4cfc7fe49 --- /dev/null +++ b/daiv/codebase/mr_state.py @@ -0,0 +1,65 @@ +from __future__ import annotations + +import logging +from typing import TYPE_CHECKING + +from django.core.cache import cache + +from codebase.base import MergeRequestState + +if TYPE_CHECKING: + from collections.abc import Iterable + +logger = logging.getLogger("daiv.clients") + +MR_STATE_CACHE_KEY_PREFIX = "mr_state:" +# Short TTL (W2 render-latency/cache decision): a refresh reflects reality within this window. +# Eager webhook invalidation (codebase/clients/{gitlab,github}/api/callbacks.py) is the primary +# freshness path; this TTL is the fallback for a close-without-merge or a missed webhook. +MR_STATE_CACHE_TTL = 60 # seconds + + +def _mr_state_cache_key(repo_id: str, iid: int) -> str: + """Build the cache key for one MR's live state. + + The single source of the key (AC7), used by BOTH the read-through below and the webhook + invalidation ``cache.delete`` — a divergent key would leave a merged MR cached-stale until the + TTL, so the read and the invalidation must never drift. + """ + return f"{MR_STATE_CACHE_KEY_PREFIX}{repo_id}:{iid}" + + +def get_merge_request_state(repo_id: str, iid: int) -> MergeRequestState: + """Cached read-through for one MR's live lifecycle state (AC3, AC6, AC7). + + Cache-first, mirroring the ``codebase/repo_config.py`` idiom: a warm key returns the rehydrated + enum with NO client call; a cold key calls the provider and caches the enum ``.value`` (a + ``str``) for :data:`MR_STATE_CACHE_TTL`. This wrapper OWNS the fail-safe: any exception, timeout, + or unconfigured client resolves to :attr:`MergeRequestState.OPEN` (logged at ``warning``) so a + read failure keeps the item visible (NFR1) and never raises up into a render path. + """ + key = _mr_state_cache_key(repo_id, iid) + try: + if (cached := cache.get(key)) is not None: + return MergeRequestState(cached) + + # Lazy import: keep this module importable without pulling the whole client graph, and + # let the mocked factory take over in tests. + from codebase.clients import RepoClient + + state = RepoClient.create_instance().get_merge_request_state(repo_id, merge_request_id=iid) + cache.set(key, state.value, MR_STATE_CACHE_TTL) + return state + except Exception: + logger.warning("Live MR-state read failed for %s!%s; keeping item visible", repo_id, iid, exc_info=True) + return MergeRequestState.OPEN + + +def get_merge_request_states(repo_id: str, iids: Iterable[int]) -> dict[int, MergeRequestState]: + """Batch entry point: resolve many MRs' live states in one call (AC7). + + Loops the per-item cached read (warm items cost nothing), so Epic 4's Needs-me Queue can + reconcile a list of MRs without N cold sequential round-trips. Each item is independently + fail-safe (a per-item failure resolves to ``OPEN``). + """ + return {iid: get_merge_request_state(repo_id, iid) for iid in iids} diff --git a/daiv/sessions/locale/pt/LC_MESSAGES/django.po b/daiv/sessions/locale/pt/LC_MESSAGES/django.po index cac0ad1a6..198285196 100644 --- a/daiv/sessions/locale/pt/LC_MESSAGES/django.po +++ b/daiv/sessions/locale/pt/LC_MESSAGES/django.po @@ -443,3 +443,9 @@ msgstr "Mostrar mais" msgid "Show less" msgstr "Mostrar menos" + +msgid "Download report" +msgstr "Descarregar relatório" + +msgid "Download report from %(when)s" +msgstr "Descarregar relatório de %(when)s" diff --git a/daiv/sessions/reconcile.py b/daiv/sessions/reconcile.py new file mode 100644 index 000000000..01eb2314e --- /dev/null +++ b/daiv/sessions/reconcile.py @@ -0,0 +1,46 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +from codebase.base import MergeRequestState +from sessions.models import RunEnvelope + +if TYPE_CHECKING: + from sessions.models import Run + +# Sentinel distinguishing "caller passed no envelope, look it up" (``_UNSET``) from "caller asserts +# there is no envelope — the run is still classifying" (an explicit ``None``). Letting ``None`` mean +# classifying lets the badge/Feed pass their already-batched per-run envelope directly (``None`` for +# a run with no envelope) with no re-query, so no N+1 is introduced. Typed ``Any`` (matching the +# house sentinel idiom) so the default is assignable to the ``RunEnvelope | None`` parameter. +_UNSET: Any = object() + + +def still_actionable(run: Run, envelope: RunEnvelope | None = _UNSET) -> bool: + """The single shared liveness predicate (AC2) — read-only (AC3), fail-safe (AC6). + + Composes two checks and NEVER writes (no ``.save()`` / ``aupdate`` / signal): + + 1. **Classification actionability.** ``envelope is _UNSET`` → look it up via + ``RunEnvelope.objects.for_run(run)``. An explicit ``None`` (or a resolved ``None``) means the + run is still *classifying* → actionable until it resolves. Otherwise defer to + ``envelope.is_actionable`` (an ``all-clear`` envelope is not actionable). Because + ``RunEnvelope.clean()`` enforces ``is_actionable ⟺ status != all-clear`` for every persisted + envelope, this preserves the badge's existing counted cases. + 2. **Live MR resolution.** Only when the item references an MR (``run.merge_request_iid`` is set) + do we consult the Task-2 cached live read: a confirmed ``MERGED`` / ``CLOSED`` state means the + MR was resolved externally → *not* actionable (AC4/AC5); ``OPEN`` / ``DRAFT``, an unknown + state, or a failed read keeps it actionable (AC6, the fail-safe: an item leaves only on a + confirmed resolution). A run with no ``merge_request_iid`` skips the live read entirely. + """ + env = RunEnvelope.objects.for_run(run) if envelope is _UNSET else envelope + actionable = True if env is None else env.is_actionable + if not actionable or run.merge_request_iid is None: + return actionable + # Lazy import: keeps the sessions <-> codebase dependency acyclic at import time. + from codebase.mr_state import get_merge_request_state + + # ``actionable`` is necessarily True here (the guard above returned otherwise), so liveness is + # decided solely by the MR state: a confirmed resolution drops the item, anything else keeps it. + state = get_merge_request_state(run.repo_id, run.merge_request_iid) + return state not in MergeRequestState.resolved() diff --git a/daiv/sessions/templates/sessions/session_detail.html b/daiv/sessions/templates/sessions/session_detail.html index 7291517dd..481508403 100644 --- a/daiv/sessions/templates/sessions/session_detail.html +++ b/daiv/sessions/templates/sessions/session_detail.html @@ -84,7 +84,20 @@ {% if expired %}
{% translate "This session's state has expired. Start a new session to continue." %} - {% translate "New session" %} + {% comment %} AC12: the transcript is gone with the checkpoint, but ``Run.response_text`` is persisted — + surface a link to the durable prose report for every successful run so the drill-through + never dead-ends. ``session_run_download_md`` serves it verbatim from the DB. {% endcomment %} +
+ {% for run in runs %} + {% if run.status == "SUCCESSFUL" and run.response_text %} + {# Label each report with its run time so multiple runs are distinguishable (a11y). #} + {% translate "Download report" %} {{ run.created_at|date:"Y-m-d H:i" }} + {% endif %} + {% endfor %} + {% translate "New session" %} +
{% endif %} diff --git a/tests/unit_tests/accounts/test_context_processors.py b/tests/unit_tests/accounts/test_context_processors.py index d6d1c0066..3ef8acac4 100644 --- a/tests/unit_tests/accounts/test_context_processors.py +++ b/tests/unit_tests/accounts/test_context_processors.py @@ -187,6 +187,8 @@ def rf(): def _make_feed_run(user, *, envelope_status=None, repo_id="group/project"): """Build a SCHEDULE run for ``user``, optionally with a ``RunEnvelope`` of the given status.""" + from sessions.envelopes import build_actionable_item + session = Session.objects.create( thread_id=str(uuid.uuid4()), origin=SessionOrigin.SCHEDULE, repo_id=repo_id, user=user ) @@ -199,7 +201,14 @@ def _make_feed_run(user, *, envelope_status=None, repo_id="group/project"): finished_at=timezone.now(), ) if envelope_status is not None: - RunEnvelope.objects.create(run=run, status=envelope_status) + # A found-issues envelope MUST carry ≥1 actionable item (RunEnvelope.clean() / FR-5), so its + # ``is_actionable`` is True — the invariant the badge's predicate routing relies on. + actionable = ( + [build_actionable_item(id="1", kind="bug", label="issue", ref="a.py")] + if envelope_status == EnvelopeStatus.FOUND_ISSUES + else [] + ) + RunEnvelope.objects.create(run=run, status=envelope_status, actionable=actionable) return run @@ -320,3 +329,68 @@ def test_bell_mark_all_read_leaves_feed_count(self, member_user): _feed_notif(member_user, run.pk) Notification.mark_all_read_for(member_user, exclude_event_types=(EventType.RUN_FEED,)) assert feed_unread_attention_count(member_user) == 1 + + +def _make_mr_feed_run(user, *, envelope_status, merge_request_iid, repo_id="group/project"): + """An attention run that references an MR (so the badge reconciles it against live MR state).""" + from sessions.envelopes import build_actionable_item + + session = Session.objects.create( + thread_id=str(uuid.uuid4()), origin=SessionOrigin.MR_WEBHOOK, repo_id=repo_id, user=user + ) + run = Run.objects.create( + session=session, + trigger_type=SessionOrigin.MR_WEBHOOK, + repo_id=repo_id, + status=RunStatus.SUCCESSFUL, + user=user, + merge_request_iid=merge_request_iid, + finished_at=timezone.now(), + ) + actionable = ( + [build_actionable_item(id="1", kind="bug", label="issue", ref="a.py")] + if envelope_status == EnvelopeStatus.FOUND_ISSUES + else [] + ) + RunEnvelope.objects.create(run=run, status=envelope_status, actionable=actionable) + return run + + +@pytest.mark.django_db +class TestFeedUnreadAttentionCountReconcile: + """Story 3.3 — the badge routes liveness through ``still_actionable`` (AC2, AC4, AC5, AC6).""" + + _LIVE_READ = "codebase.mr_state.get_merge_request_state" + + def test_externally_merged_mr_run_is_excluded(self, member_user): + from codebase.base import MergeRequestState + + run = _make_mr_feed_run(member_user, envelope_status=EnvelopeStatus.NEEDS_ATTENTION, merge_request_iid=7) + _feed_notif(member_user, run.pk) + with patch(self._LIVE_READ, return_value=MergeRequestState.MERGED): + assert feed_unread_attention_count(member_user) == 0 + + def test_open_mr_run_is_counted(self, member_user): + from codebase.base import MergeRequestState + + run = _make_mr_feed_run(member_user, envelope_status=EnvelopeStatus.FOUND_ISSUES, merge_request_iid=7) + _feed_notif(member_user, run.pk) + with patch(self._LIVE_READ, return_value=MergeRequestState.OPEN): + assert feed_unread_attention_count(member_user) == 1 + + def test_read_failure_keeps_the_run_counted(self, member_user): + # The wrapper resolves a failed read to OPEN → the item stays counted (AC6, fail-safe). + from codebase.base import MergeRequestState + + run = _make_mr_feed_run(member_user, envelope_status=EnvelopeStatus.NEEDS_ATTENTION, merge_request_iid=7) + _feed_notif(member_user, run.pk) + with patch(self._LIVE_READ, return_value=MergeRequestState.OPEN): + assert feed_unread_attention_count(member_user) == 1 + + def test_all_clear_mr_run_never_triggers_a_live_read(self, member_user): + # An all-clear run is quiet regardless of its MR: no live read, not counted. + run = _make_mr_feed_run(member_user, envelope_status=EnvelopeStatus.ALL_CLEAR, merge_request_iid=7) + _feed_notif(member_user, run.pk) + with patch(self._LIVE_READ) as read: + assert feed_unread_attention_count(member_user) == 0 + read.assert_not_called() diff --git a/tests/unit_tests/accounts/test_dashboard_console.py b/tests/unit_tests/accounts/test_dashboard_console.py index 6bf795f4e..6466ed0f8 100644 --- a/tests/unit_tests/accounts/test_dashboard_console.py +++ b/tests/unit_tests/accounts/test_dashboard_console.py @@ -6,17 +6,24 @@ job-creation launcher in the console content. """ +import uuid from datetime import timedelta from pathlib import Path +from unittest.mock import patch -from django.template.loader import get_template +from django.template.loader import get_template, render_to_string from django.test import Client from django.urls import reverse from django.utils import timezone import pytest +from notifications.choices import EventType +from notifications.models import Notification +from sessions.models import EnvelopeStatus, Run, RunEnvelope, RunStatus, Session, SessionOrigin from accounts.models import Role +from accounts.views import get_velocity_data +from codebase.clients import RepoClient from codebase.models import MergeMetric, PlatformType # Repo root: tests/unit_tests/accounts/ -> parents[3]. @@ -24,7 +31,7 @@ INPUT_CSS = REPO_ROOT / "daiv" / "static_src" / "css" / "input.css" -def _make_merge_metric(*, iid=1, daiv_commits=1, total_commits=2, lines_added=10, lines_removed=3): +def _make_merge_metric(*, iid=1, daiv_commits=1, total_commits=2, lines_added=10, lines_removed=3, title=""): """Create a ``MergeMetric`` row directly (no factory exists). Fills the required non-default fields (``merged_at``/``target_branch``/``source_branch``/ @@ -34,6 +41,7 @@ def _make_merge_metric(*, iid=1, daiv_commits=1, total_commits=2, lines_added=10 return MergeMetric.objects.create( repo_id="daiv/test", merge_request_iid=iid, + title=title, lines_added=lines_added, lines_removed=lines_removed, total_commits=total_commits, @@ -395,3 +403,680 @@ def test_lens_strings_are_translated(self): assert '{% translate "MRs involving DAIV" %}' in src # The cold-load aria-busy label (single-quoted inside the double-quoted attribute). assert "{% translate 'Computing org impact…' %}" in src + + +# --------------------------------------------------------------------------- +# Story 3.1 — The personal Throughput Hero + wired range switcher +# --------------------------------------------------------------------------- + +# ``merged_at`` is *now* in ``_make_merge_metric``; distinct ``iid`` keeps the +# (repo_id, merge_request_iid, platform) uniqueness happy. The Hero counts a merge only when a +# ``Run`` in one of the USER's own sessions shares its (repo_id, merge_request_iid) — the value-based +# ``MergeMetric ⋈ Run`` attribution join. This helper builds that owning Session+Run inline (no +# factory exists), mirroring how ``_make_merge_metric`` builds the MergeMetric directly. + + +def _make_shipping_run(user, *, iid, repo_id="daiv/test", web_url=""): + """Create a Session owned by ``user`` + a Run sharing ``(repo_id, iid)`` — the DAIV-opened, + by-this-user attribution the Hero join asserts. Pairs with a ``_make_merge_metric(iid=iid)``. + + ``web_url`` carries onto ``Run.merge_request_web_url`` (the best-effort MR out-link the 3.2 + breakdown surfaces through the join; blank exercises the AC11 degrade-to-session-link path).""" + session = Session.objects.create( + thread_id=str(uuid.uuid4()), origin=SessionOrigin.MR_WEBHOOK, repo_id=repo_id, user=user + ) + return Run.objects.create( + session=session, + trigger_type=SessionOrigin.MR_WEBHOOK, + status=RunStatus.SUCCESSFUL, + repo_id=repo_id, + merge_request_iid=iid, + merge_request_web_url=web_url, + ) + + +@pytest.mark.django_db +class TestHeroCountAndAttribution: + """AC2/AC3: 'changes shipped' = merged MRs opened by DAIV for THIS user (MergeMetric ⋈ Run).""" + + def test_counts_only_merges_with_a_matching_user_run(self, member_client, member_user): + # iid=1 → member's own run (counted); iid=2 → human MR, no run (excluded). + _make_merge_metric(iid=1) + _make_shipping_run(member_user, iid=1) + _make_merge_metric(iid=2) # no run → human-authored → excluded + + response = member_client.get(reverse("dashboard")) + assert response.status_code == 200 + hero = response.context["hero"] + assert hero is not None + assert hero["all_time"] == 1 + assert hero["this_range"]["total_merges"] == 1 + assert b'data-testid="hero-headline"' in response.content + + def test_another_users_merge_is_excluded(self, member_client, member_user, admin_user): + # A merge matched only by ANOTHER user's run must not count for member. + _make_merge_metric(iid=3) + _make_shipping_run(admin_user, iid=3) + + response = member_client.get(reverse("dashboard")) + # member has zero matched merges → honest empty state, not a 0-headline. + assert response.context["hero"] is None + assert b'data-testid="hero-empty"' in response.content + + +@pytest.mark.django_db +class TestHeroAdminPersonalScope: + """AC10: the admin Hero counts only the admin's OWN merges — never the by_owner/.all() short-circuit.""" + + def test_admin_hero_is_personal_not_org_wide(self, admin_client, admin_user, member_user): + _make_merge_metric(iid=1) + _make_shipping_run(admin_user, iid=1) # admin's own → counted + _make_merge_metric(iid=2) + _make_shipping_run(member_user, iid=2) # another user's → excluded for admin + _make_merge_metric(iid=3) # human MR, no run → excluded + + response = admin_client.get(reverse("dashboard")) + hero = response.context["hero"] + assert hero is not None + # Personal scope: only the admin's own 1 merge, NOT the org-wide 3 (or 2). + assert hero["all_time"] == 1 + + def test_personal_by_default_still_holds_with_hero(self, admin_client, admin_user): + # AC10 belt-and-braces: even with a live personal Hero, the default carries no org content. + _make_merge_metric(iid=1) + _make_shipping_run(admin_user, iid=1) + response = admin_client.get(reverse("dashboard")) + assert "velocity" not in response.context + assert "total_users" not in response.context + assert b'data-testid="manager-lens"' not in response.content + + +@pytest.mark.django_db +class TestHeroRangeScopingAndDelta: + """AC1/AC6 + D3/D4: range scopes the headline, delta vs the prior equal window, hidden for 'all'.""" + + def _seed_windows(self, user): + # this-week ×2 (now), prev-window ×1 (10d ago), older ×1 (40d ago); all owned by ``user``. + for iid in (1, 2, 3, 4): + _make_merge_metric(iid=iid) + _make_shipping_run(user, iid=iid) + MergeMetric.objects.filter(merge_request_iid=3).update(merged_at=timezone.now() - timedelta(days=10)) + MergeMetric.objects.filter(merge_request_iid=4).update(merged_at=timezone.now() - timedelta(days=40)) + + def test_default_view_resolves_to_7d_this_week_with_delta(self, member_client, member_user): + self._seed_windows(member_user) + response = member_client.get(reverse("dashboard")) # no ?period= → D4 default + assert response.context["current_period"] == "7d" + hero = response.context["hero"] + assert hero["this_range"]["total_merges"] == 2 # the two this-week merges + assert hero["delta"] == 1 # 2 this week − 1 in the preceding 7-day window + assert hero["all_time"] == 4 + assert b"this week" in response.content + assert b'data-testid="hero-delta"' in response.content + + def test_delta_uses_equal_length_windows(self, member_client, member_user): + # AC1/D3 regression: the current "7d" window is [today−7, today] = 8 inclusive days, so the + # preceding window must also span 8 days. With a flat one-merge-per-day cadence a 7-day + # previous window would report a spurious +1; an equal 8-day window reports 0. + for offset in range(16): # today, today−1, … today−15 (one merge per day) + iid = offset + 1 + _make_merge_metric(iid=iid) + _make_shipping_run(member_user, iid=iid) + MergeMetric.objects.filter(merge_request_iid=iid).update(merged_at=timezone.now() - timedelta(days=offset)) + hero = member_client.get(reverse("dashboard"), {"period": "7d"}).context["hero"] + assert hero["this_range"]["total_merges"] == 8 # offsets 0..7 + assert hero["delta"] == 0 # preceding equal 8-day window (offsets 8..15) also holds 8 + + def test_30d_range_label_and_scope(self, member_client, member_user): + self._seed_windows(member_user) + response = member_client.get(reverse("dashboard"), {"period": "30d"}) + hero = response.context["hero"] + assert hero["this_range"]["total_merges"] == 3 # excludes the 40-day-old merge + assert b"in the last 30 days" in response.content + + def test_all_time_range_has_no_delta_chip(self, member_client, member_user): + self._seed_windows(member_user) + response = member_client.get(reverse("dashboard"), {"period": "all"}) + hero = response.context["hero"] + assert hero["this_range"]["total_merges"] == 4 # every merge + assert hero["delta"] is None + assert b'data-testid="hero-delta"' not in response.content + + def test_htmx_fragment_rescopes_hero(self, member_client, member_user): + self._seed_windows(member_user) + response = member_client.get(reverse("dashboard"), {"period": "all"}, HTTP_HX_REQUEST="true") + assert response.status_code == 200 + assert b'data-testid="hero-headline"' in response.content + assert b'data-testid="app-sidebar"' not in response.content # fragment, not the shell + assert response.context["hero"]["this_range"]["total_merges"] == 4 + + +@pytest.mark.django_db +class TestHeroOdometerInvariance: + """AC1: the all-time odometer is invariant to the selected range.""" + + def test_all_time_identical_across_ranges(self, member_client, member_user): + for iid in (1, 2, 3): + _make_merge_metric(iid=iid) + _make_shipping_run(member_user, iid=iid) + MergeMetric.objects.filter(merge_request_iid=3).update(merged_at=timezone.now() - timedelta(days=40)) + + seven = member_client.get(reverse("dashboard"), {"period": "7d"}).context["hero"]["all_time"] + thirty = member_client.get(reverse("dashboard"), {"period": "30d"}).context["hero"]["all_time"] + assert seven == thirty == 3 + + +@pytest.mark.django_db +class TestHeroDefaultPeriod: + """D4: the personal console default period is now '7d' (was 'today').""" + + def test_default_period_is_7d(self, member_client): + response = member_client.get(reverse("dashboard")) + assert response.context["current_period"] == "7d" + + +class TestHeroEstimateDemotion: + """AC4/D1: no estimate line in v1; a truthy estimate renders demoted (never fact-styled).""" + + def _render(self, *, estimate): + hero = {"this_range": {"total_merges": 5}, "period": "7d", "delta": 1, "all_time": 100, "estimate": estimate} + return render_to_string("accounts/_hero.html", {"hero": hero}) + + def test_v1_estimate_none_renders_no_estimate_line(self): + html = self._render(estimate=None) + assert 'data-testid="hero-estimate"' not in html + + def test_truthy_estimate_is_visually_demoted(self): + html = self._render(estimate=48) + assert 'data-testid="hero-estimate"' in html + start = html.index('data-testid="hero-estimate"') + block = html[start : html.index("

", start)] + assert "~48 dev-hours saved" in html + # Demotion: italic + faint + dotted underline + an ``est.`` tag ... + assert "italic" in block + assert "text-text-faint" in block + assert "decoration-dotted" in block + assert "est." in block + # ... and NEVER the fact styling (solid white display mono). + assert "text-text-strong" not in block + + +@pytest.mark.django_db +class TestHeroSlackLine: + """AC5: the copyable Slack one-liner sits in its own overflow-x-auto inset with a Copy control.""" + + def test_slack_line_overflow_and_copy_control(self, member_client, member_user): + _make_merge_metric(iid=1) + _make_shipping_run(member_user, iid=1) + response = member_client.get(reverse("dashboard")) + content = response.content.decode() + assert 'data-testid="hero-slack"' in content + # The inset owns its overflow container so a long line never scrolls the body. + slack_start = content.index('data-testid="hero-slack"') + assert "overflow-x-auto" in content[slack_start - 120 : slack_start + 120] + assert 'data-testid="hero-copy"' in content + # The clipboard idiom's json_script payload is present with the referenced id. + assert 'id="hero-slack-line"' in content + assert "DAIV shipped 1 changes this week (1 all-time)" in content + + +@pytest.mark.django_db +class TestHeroEmptyState: + """AC13: nothing shipped all-time → honest empty state, never a 0-headline fact.""" + + def test_empty_hero_is_honest(self, member_client): + response = member_client.get(reverse("dashboard")) + assert response.context["hero"] is None + assert b'data-testid="hero-empty"' in response.content + assert b"No changes shipped yet" in response.content + # No 0 dressed up as a headline fact. + assert b'data-testid="hero-headline"' not in response.content + + +@pytest.mark.django_db +class TestGetVelocityDataQuerysetRefactor: + """AC11: one shared counting body — the queryset arg scopes it; the default stays org-wide.""" + + def test_default_is_org_wide_and_queryset_scopes(self, member_user): + _make_merge_metric(iid=1) + _make_merge_metric(iid=2) + # Default (no queryset) aggregates every MergeMetric — protects the org-wide ManagerLensView. + assert get_velocity_data(None)["total_merges"] == 2 + # A passed subset aggregates only that subset — the personal Hero's user-scoped path. + subset = MergeMetric.objects.filter(merge_request_iid=1) + assert get_velocity_data(None, queryset=subset)["total_merges"] == 1 + + def test_empty_queryset_returns_none(self): + assert get_velocity_data(None, queryset=MergeMetric.objects.none()) is None + + +@pytest.mark.django_db +class TestHeroReadOnly: + """AC12: rendering the Hero performs read-only queries — no row is created or mutated.""" + + def test_render_writes_nothing(self, member_client, member_user): + _make_merge_metric(iid=1) + _make_shipping_run(member_user, iid=1) + before = (MergeMetric.objects.count(), Run.objects.count(), Session.objects.count()) + member_client.get(reverse("dashboard")) + member_client.get(reverse("dashboard"), {"period": "all"}) + after = (MergeMetric.objects.count(), Run.objects.count(), Session.objects.count()) + assert before == after + + +class TestHeroI18n: + """AC9: every new Hero string is {% translate %}/{% blocktranslate %}-wrapped (no bare literal).""" + + def test_hero_strings_are_translated(self): + src = Path(get_template("accounts/_hero.html").origin.name).read_text(encoding="utf-8") + assert "{% blocktranslate count n=hero.this_range.total_merges %}" in src + assert "changes shipped" in src + assert '{% translate "shipped since day one" %}' in src + assert '{% translate "est." %}' in src + assert "DAIV shipped" in src # the locked Slack line (D2), blocktranslate asvar + assert "{% translate 'Copy' %}" in src + assert '{% translate "No changes shipped yet" %}' in src + # A range-adaptive label is wrapped too (the ``as`` form). + assert '{% translate "this week" as range_label %}' in src + + def test_hero_eyebrow_is_range_agnostic_and_mounts_the_partial(self): + # The reconciled eyebrow can no longer hard-code "Today" and contradict the active range, + # the live partial is mounted, and the hero section no longer carries the placeholder + # aria-busy (its content is live now). + src = Path(get_template("accounts/_console_body.html").origin.name).read_text(encoding="utf-8") + assert '{% translate "Throughput" %}' in src + assert '{% include "accounts/_hero.html" %}' in src + assert 'data-testid="console-hero" aria-labelledby="console-hero-heading" aria-busy="true"' not in src + + +# --------------------------------------------------------------------------- +# Story 3.2 — Click-through to source (the auditable-in-place breakdown) +# --------------------------------------------------------------------------- + + +@pytest.mark.django_db +class TestBreakdownReconcilesExactly: + """AC1/AC5 (NFR1, load-bearing): the revealed rows are exactly the rows the count summed.""" + + def test_this_range_rows_equal_headline_count_and_exclude_the_rest(self, member_client, member_user, admin_user): + # K=3 matched, in-range (now) → counted this range AND all-time. + for iid in (1, 2, 3): + _make_merge_metric(iid=iid) + _make_shipping_run(member_user, iid=iid) + # matched but out-of-range (40d ago) → excluded this range, present all-time. + _make_merge_metric(iid=4) + _make_shipping_run(member_user, iid=4) + MergeMetric.objects.filter(merge_request_iid=4).update(merged_at=timezone.now() - timedelta(days=40)) + # matched by ANOTHER user's run → excluded from both (personal scope). + _make_merge_metric(iid=5) + _make_shipping_run(admin_user, iid=5) + # human MR, no matching run → excluded from both (attribution join). + _make_merge_metric(iid=6) + + response = member_client.get(reverse("dashboard")) # 7d default + hero = response.context["hero"] + # Reconcile-exactly by construction: len(this_range_rows) == the headline count. + assert hero["this_range"]["total_merges"] == 3 + assert len(hero["this_range_rows"]) == 3 + # all-time carries the out-of-range matched row too, still excludes other-user/human. + assert hero["all_time"] == 4 + assert len(hero["all_time_rows"]) == 4 + + range_iids = {row.merge_request_iid for row in hero["this_range_rows"]} + assert range_iids == {1, 2, 3} + all_time_iids = {row.merge_request_iid for row in hero["all_time_rows"]} + assert all_time_iids == {1, 2, 3, 4} + # Excluded rows appear in neither breakdown. + assert 5 not in all_time_iids + assert 6 not in all_time_iids + # The rendered panel carries one row per underlying merge. + assert b'data-testid="hero-breakdown"' in response.content + assert b'data-testid="breakdown-row"' in response.content + + def test_rows_carry_the_row_contract_fields(self, member_client, member_user): + _make_merge_metric(iid=7, title="Fix the flaky auth test") + _make_shipping_run(member_user, iid=7, web_url="https://gitlab.example.com/daiv/test/-/merge_requests/7") + hero = member_client.get(reverse("dashboard")).context["hero"] + row = hero["this_range_rows"][0] + assert row.repo_id == "daiv/test" + assert row.merge_request_iid == 7 + assert row.title == "Fix the flaky auth test" + assert row.merged_at is not None + assert row.web_url == "https://gitlab.example.com/daiv/test/-/merge_requests/7" + assert row.thread_id # a representative session for the drill-through + + +@pytest.mark.django_db +class TestBreakdownAdminPersonalScope: + """AC7: the admin breakdown lists ONLY the admin's own rows — never the by_owner/.all() path.""" + + def test_admin_breakdown_is_personal_not_org_wide(self, admin_client, admin_user, member_user): + _make_merge_metric(iid=1) + _make_shipping_run(admin_user, iid=1) # admin's own → listed + _make_merge_metric(iid=2) + _make_shipping_run(member_user, iid=2) # another user's → excluded for admin + _make_merge_metric(iid=3) # human MR, no run → excluded + + hero = admin_client.get(reverse("dashboard"), {"period": "all"}).context["hero"] + assert len(hero["all_time_rows"]) == 1 + assert hero["all_time_rows"][0].merge_request_iid == 1 + assert len(hero["this_range_rows"]) == 1 + assert hero["this_range_rows"][0].merge_request_iid == 1 + + +@pytest.mark.django_db +class TestBreakdownRangeAndOdometer: + """AC8: headline rows track the active range; odometer rows are range-invariant.""" + + def _seed(self, user): + # two this-week (now) + one 20 days ago; all owned by ``user``. + for iid in (1, 2, 3): + _make_merge_metric(iid=iid) + _make_shipping_run(user, iid=iid) + MergeMetric.objects.filter(merge_request_iid=3).update(merged_at=timezone.now() - timedelta(days=20)) + + def test_headline_rows_match_counted_range(self, member_client, member_user): + self._seed(member_user) + seven = member_client.get(reverse("dashboard"), {"period": "7d"}).context["hero"] + assert len(seven["this_range_rows"]) == seven["this_range"]["total_merges"] == 2 + thirty = member_client.get(reverse("dashboard"), {"period": "30d"}).context["hero"] + assert len(thirty["this_range_rows"]) == thirty["this_range"]["total_merges"] == 3 + + def test_odometer_rows_identical_across_ranges(self, member_client, member_user): + self._seed(member_user) + seven = member_client.get(reverse("dashboard"), {"period": "7d"}).context["hero"] + thirty = member_client.get(reverse("dashboard"), {"period": "30d"}).context["hero"] + assert len(seven["all_time_rows"]) == len(thirty["all_time_rows"]) == 3 + + def test_htmx_fragment_carries_the_rescoped_breakdown(self, member_client, member_user): + self._seed(member_user) + response = member_client.get(reverse("dashboard"), {"period": "7d"}, HTTP_HX_REQUEST="true") + assert response.status_code == 200 + assert b'data-testid="app-sidebar"' not in response.content # fragment, not the shell + assert b'data-testid="hero-breakdown"' in response.content + assert len(response.context["hero"]["this_range_rows"]) == 2 + + def test_empty_range_shows_honest_empty_state(self, member_client, member_user): + # A non-zero all-time but nothing in the active range must not open a blank panel: the + # breakdown renders an honest "nothing here" row rather than an empty reveal (Edge-A). + _make_merge_metric(iid=1) + _make_shipping_run(member_user, iid=1) + MergeMetric.objects.filter(merge_request_iid=1).update(merged_at=timezone.now() - timedelta(days=20)) + response = member_client.get(reverse("dashboard"), {"period": "7d"}) + ctx = response.context["hero"] + assert ctx["all_time"] == 1 + assert len(ctx["this_range_rows"]) == 0 + assert b"No changes shipped in this range." in response.content + + +@pytest.mark.django_db +class TestBreakdownHowComputed: + """AC2: an honest AD-10 disclosure — no envelope category error, no NON-GOAL promises.""" + + def test_how_computed_present_and_honest(self, member_client, member_user): + _make_merge_metric(iid=1) + _make_shipping_run(member_user, iid=1) + # Use the console-body fragment (no shell chrome), so the envelope guard reads the console + # content only — the shared sidebar renders an unrelated ``{% icon "envelope" %}`` glyph. + content = member_client.get(reverse("dashboard"), HTTP_HX_REQUEST="true").content + assert b'data-testid="hero-howcomputed"' in content + assert b"Changes shipped = merged MRs opened by DAIV" in content + # The Hero never reads RunEnvelope — the word must not appear in the disclosure (category error). + assert b"RunEnvelope" not in content + assert b"envelope" not in content + # No NON-GOAL metric is promised (diff-survival / clean-vs-edited / sparkline). + for word in (b"diff-survival", b"sparkline", b"clean-vs", b"dev-hours"): + assert word not in content + + +@pytest.mark.django_db +class TestBreakdownMrLinkHonesty: + """AC11/AC6: link out via the persisted URL only; blank degrades; no live client on render.""" + + def test_web_url_links_out_blank_degrades_to_session(self, member_client, member_user): + _make_merge_metric(iid=1) + run_linked = _make_shipping_run( + member_user, iid=1, web_url="https://gitlab.example.com/daiv/test/-/merge_requests/1" + ) + _make_merge_metric(iid=2) + run_blank = _make_shipping_run(member_user, iid=2, web_url="") # blank → no out-link + + content = member_client.get(reverse("dashboard"), {"period": "all"}).content.decode() + # The set URL renders a real out-link opening in a new tab. + assert 'href="https://gitlab.example.com/daiv/test/-/merge_requests/1"' in content + assert 'target="_blank"' in content + # Never a broken/placeholder link when the URL is blank. + assert 'href="#"' not in content + # Both rows still carry a same-tab session drill-through. + for run in (run_linked, run_blank): + assert reverse("session_detail", kwargs={"thread_id": run.session_id}) in content + + def test_no_repo_client_instantiated_on_render(self, member_client, member_user): + _make_merge_metric(iid=1) + _make_shipping_run(member_user, iid=1, web_url="https://gitlab.example.com/daiv/test/-/merge_requests/1") + with patch.object(RepoClient, "create_instance") as mock_create_instance: + member_client.get(reverse("dashboard")) + member_client.get(reverse("dashboard"), {"period": "all"}) + mock_create_instance.assert_not_called() + + def test_prefers_a_run_with_a_url_over_a_newer_blank_one(self, member_client, member_user): + # A merge attributed by two of the user's runs: the OLDER persisted the url, a newer re-run + # left it blank (the field default). The good url must win — a blank newest run must not hide + # an out-link that genuinely exists (AC11 degrades only when NO attributing run has a url). + _make_merge_metric(iid=1) + _make_shipping_run(member_user, iid=1, web_url="https://gitlab.example.com/daiv/test/-/merge_requests/1") + _make_shipping_run(member_user, iid=1, web_url="") # newer re-run, blank url + content = member_client.get(reverse("dashboard"), {"period": "all"}).content.decode() + assert 'href="https://gitlab.example.com/daiv/test/-/merge_requests/1"' in content + + +@pytest.mark.django_db +class TestBreakdownA11y: + """AC10: the trigger is a native