Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
## Unreleased

### Fixed: a repository row read as a pull request on the Delivered tab (#6459)

- **Why:** found by looking at the rendered tab in a browser rather than at the rows behind it. The Repositories view showed `vivekchand/clawmetry #vivekchand/clawmetry` on every line, and the summary above it read "100 pull requests" over a table of repositories. Both come from one cause: `query_delivery` carries `kind` on the envelope rather than on each row, so the renderer's `!r.kind` test was true for every row and every row was drawn in pull-request form, where the label is the repository followed by `#` and the reference. A repository row's reference IS its name, so the name was printed twice with a `#` wedged between. Six rounds of fixes to this tab were measured against the data; this one was invisible there, because the rows were right and only their rendering was wrong.
- **What:** the row label follows the kind of the view when the row does not carry one of its own. A pull request still reads `owner/name #number`. A repository reads its name. A branch, commit or path reads its reference, under its repository when the row knows one and it is not the same string. The count chip names what the table actually lists, so a Repositories view says repositories, and a branch view says branches.
- **Verified:** 3 browser tests against the shipped template and `app.js` with synthetic fixtures, no live store. Two are red against the released 0.12.927, where the repository label renders `vivekchand/clawmetry #vivekchand/clawmetry` and the branch view says "pull requests"; the third pins that a pull-request row still reads `vivekchand/clawmetry #6450`, which is the half a kind-aware label could quietly break. Confirmed in a real browser against the live node: 100 rows, 0 console errors, 0 page errors, and no `undefined`, `NaN` or unescaped markup in any rendered cell.
- **Limits:** the renderer still has no per-row kind to trust, so a response that mixes kinds in one list would label them all by the view. Nothing produces such a response today, because the store is asked for one kind at a time.

### Fixed: keep Asimov source alerts visible on small screens (#6452)

- Long source alert text now wraps inside its panel, so the full evidence remains readable on mobile. The measured-joint table keeps its horizontal scroll.
Expand Down
31 changes: 27 additions & 4 deletions clawmetry/static/js/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -23435,7 +23435,16 @@ async function loadDelivery() {
// able to see how much of the bill is NOT accounted for here.
chips += chip(t('delivery.unattributed', null, 'not tied to any pull request'),
usd(totals.unattributed_usd), '#d97706');
chips += chip(t('delivery.prs', null, 'pull requests'), rows.length, 'var(--text-primary)');
// The count follows the view: it said "100 pull requests" over a table
// of repositories, which is a number that reads as an answer to a
// question nobody asked.
var countWord = { pr: ['delivery.prs', 'pull requests'],
repo: ['delivery.repos', 'repositories'],
branch: ['delivery.branches', 'branches'],
commit: ['delivery.commits', 'commits'],
file: ['delivery.files', 'files'] }[kind] ||
['delivery.rows', 'rows'];
chips += chip(t(countWord[0], null, countWord[1]), rows.length, 'var(--text-primary)');
chips += chip(t('delivery.sessions', null, 'sessions'),
totals.sessions_attributed || 0, 'var(--text-primary)');
sumEl.innerHTML = chips;
Expand All @@ -23462,9 +23471,23 @@ async function loadDelivery() {
'<th style="padding:9px 12px;font-weight:600;">' + escHtml(t('delivery.col_evidence', null, 'How we know')) + '</th>' +
'</tr></thead><tbody>';
rows.forEach(function(r) {
var label = (r.kind === 'pr' || !r.kind)
? ((r.repo ? escHtml(r.repo) + ' ' : '') + '#' + escHtml(r.ref))
: escHtml(r.ref);
// The row carries no kind of its own: query_delivery puts it on the

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Drift Bot (ClawMetry) — Blueprint: Delivery attribution: which pull request the spend bought, split and never copied

The blueprint specifies that each SessionArtifact model row should have a required 'kind' field (one of pr/repo/branch/commit/file), but the API implementation places 'kind' only on the response envelope, requiring rows to fall back to the view-level kind during rendering.

// envelope, so `!r.kind` was true for EVERY row and every row rendered
// in pull-request form. On the Repositories view that read
// `vivekchand/clawmetry #vivekchand/clawmetry`, because a repo row's
// ref IS its name. Fall back to the view's kind, never to 'pr'.
var rowKind = r.kind || kind || 'pr';
var label;
if (rowKind === 'pr') {
label = (r.repo ? escHtml(r.repo) + ' ' : '') + '#' + escHtml(r.ref);
} else if (rowKind === 'repo') {
label = escHtml(r.ref || r.repo || '');
} else {
// A branch, commit or path, shown under its repository when the row
// knows one and is not already that name.
label = (r.repo && r.repo !== r.ref ? escHtml(r.repo) + ' ' : '') +
escHtml(r.ref || '');
}
var title = r.title ? '<div style="color:var(--text-muted);font-size:11px;margin-top:2px;">' +
escHtml(r.title) + '</div>' : '';
var link = r.url ? '<a href="' + escHtml(r.url) + '" target="_blank" rel="noopener noreferrer" ' +
Expand Down
128 changes: 128 additions & 0 deletions tests/test_delivery_tab_labels_browser.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
"""The Delivered tab's rows must read as what they are.

Found by looking at the rendered tab rather than at its data: the
Repositories view showed `vivekchand/clawmetry #vivekchand/clawmetry` and a
chip reading "100 pull requests" over a table of repositories. Both come
from the same cause: ``query_delivery`` carries ``kind`` on the envelope,
not on each row, so ``!r.kind`` was true for every row and every row was
rendered in pull-request form.

Uses the shipped template and app.js with synthetic API fixtures, never a
live store or the user's browser.
"""
import json
from pathlib import Path

ROOT = Path(__file__).resolve().parents[1]

REPO_ROWS = [
{"repo": "vivekchand/clawmetry", "ref": "vivekchand/clawmetry",
"sessions": 94, "cost_usd": 0, "confidence": 1.0, "confidence_min": 1.0,
"role": "explored", "basis": "pr_reference", "evidence": "git remote -v",
"title": "", "state": "", "url": "", "merged_at": 0},
{"repo": "huggingface/lerobot", "ref": "huggingface/lerobot",
"sessions": 3, "cost_usd": 0, "confidence": 1.0, "confidence_min": 1.0,
"role": "explored", "basis": "pr_reference",
"evidence": "git clone https://github.com/huggingface/lerobot.git",
"title": "", "state": "", "url": "", "merged_at": 0},
]
PR_ROWS = [
{"repo": "vivekchand/clawmetry", "ref": "6450", "sessions": 2,
"cost_usd": 12.5, "confidence": 1.0, "confidence_min": 1.0,
"role": "authored", "basis": "pr_reference",
"evidence": "gh pr view 6450", "title": "A fold", "state": "merged",
"url": "https://github.com/vivekchand/clawmetry/pull/6450",
"merged_at": 0},
]
BRANCH_ROWS = [
{"repo": "", "ref": "feat/case-fold", "sessions": 1, "cost_usd": 0,
"confidence": 1.0, "confidence_min": 1.0, "role": "authored",
"basis": "pr_reference", "evidence": "git push origin feat/case-fold",
"title": "", "state": "", "url": "", "merged_at": 0},
]
TOTALS = {"attributed_usd": 10.0, "unattributed_usd": 2.0,
"sessions_attributed": 3, "artifacts": 3}


def _page(browser, rows_by_kind):
context = browser.new_context(viewport={"width": 1280, "height": 900})
page = context.new_page()
errors = []
page.on("pageerror", lambda e: errors.append(str(e)))
page.on("console",
lambda m: errors.append(m.text) if m.type == "error" else None)
html = ('<!doctype html><html data-theme="dark"><head><meta charset="utf-8">'
'</head><body>'
+ (ROOT / "clawmetry/templates/tabs/delivery.html").read_text()
+ "</body></html>")

def route(request_route):
path = request_route.request.url.split("delivery.test", 1)[-1]
if path == "/":
request_route.fulfill(content_type="text/html", body=html)
return
kind = "pr"
if "kind=" in path:
kind = path.split("kind=", 1)[1].split("&")[0]
body = {"rows": rows_by_kind.get(kind, []), "totals": TOTALS,
"available": True, "kind": kind}
request_route.fulfill(content_type="application/json",
body=json.dumps(body))

context.route("https://delivery.test/**", route)
page.goto("https://delivery.test/")
page.evaluate("""() => {
window.t = (key, _x, fallback) => fallback || key;
window.escHtml = s => String(s === undefined || s === null ? '' : s)
.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;')
.replace(/"/g, '&quot;');
}""")
page.add_script_tag(path=str(ROOT / "clawmetry/static/js/app.js"))
return context, page, errors


def test_a_repository_row_is_not_rendered_as_a_pull_request(_shared_chromium):
context, page, errors = _page(_shared_chromium, {"repo": REPO_ROWS})
try:
page.select_option("#dlv-kind", "repo")
page.wait_for_function(
"() => document.querySelectorAll('#dlv-table tbody tr').length > 0")
first = page.locator("#dlv-table tbody tr td").first.inner_text().strip()
assert first == "vivekchand/clawmetry", first
assert "#" not in page.locator("#dlv-table").inner_text()
# The count chip must name what the table actually lists.
summary = page.locator("#dlv-summary").inner_text()
assert "repositories" in summary, summary
assert "pull requests" not in summary.split("not tied")[-1], summary
assert errors == [], errors
finally:
context.close()


def test_a_pull_request_row_still_reads_repo_then_number(_shared_chromium):
"""The half a kind-aware label could quietly break."""
context, page, errors = _page(_shared_chromium, {"pr": PR_ROWS})
try:
page.evaluate("loadDelivery()")
page.wait_for_function(
"() => document.querySelectorAll('#dlv-table tbody tr').length > 0")
first = page.locator("#dlv-table tbody tr td").first.inner_text()
assert "vivekchand/clawmetry #6450" in first, first
assert "pull requests" in page.locator("#dlv-summary").inner_text()
assert errors == [], errors
finally:
context.close()


def test_a_branch_row_shows_the_branch_and_no_hash(_shared_chromium):
context, page, errors = _page(_shared_chromium, {"branch": BRANCH_ROWS})
try:
page.select_option("#dlv-kind", "branch")
page.wait_for_function(
"() => document.querySelectorAll('#dlv-table tbody tr').length > 0")
first = page.locator("#dlv-table tbody tr td").first.inner_text().strip()
assert first == "feat/case-fold", first
assert "branches" in page.locator("#dlv-summary").inner_text()
assert errors == [], errors
finally:
context.close()
Loading