From b69f9a8132c052506ea4a6d7ff3f32f0a20e467d Mon Sep 17 00:00:00 2001 From: Pratik Bhavsar Date: Fri, 6 Nov 2026 12:42:49 +0530 Subject: [PATCH 1/6] feat(git): label commit work types from message details and file kinds Commits without a conventional prefix were labeled from the subject alone (58% agreement with their authors' own type). The model now also sees the first body line and a content-free count of the kinds of files the commit changed ("Changed files: 3 code, 1 tests (+120 / -30 lines)."); paths and names never leave the Git scan. Held-out agreement rose from 57.5% to 72.0% (67% vs 39% for commits with no body). Question version c2 relabels once. A conventional prefix (feat:, fix(ui):, docs:, chore:, ...) is now taken as the label directly, with no model call or queue slot. Also from #139: MCP git_stats returns the estimated work-type split (summary and per day, enum keys, counts only), and a failed rev-list in the backfill walk has a test. Co-Authored-By: Claude Code --- AGENTS.md | 2 +- README.md | 3 +- dashboard/markup/git-learn-tools.html | 2 +- specs/AGENTS.md | 2 +- specs/ARCHITECTURE.md | 2 +- specs/SECURITY.md | 6 +- specs/USER_GUIDE.md | 7 +- specs/WORK_INSIGHTS.md | 60 ++++++++++----- tests/test_git_delivery.py | 104 ++++++++++++++++++++------ tests/test_mcp_delivery.py | 20 +++++ tests/test_work_insights.py | 44 ++++++++++- token_meter/mcp/delivery.py | 34 +++++++++ token_meter/mcp/schema.py | 8 ++ token_meter/services/git_delivery.py | 46 ++++++++++-- token_meter/services/work_insights.py | 64 ++++++++++++++-- token_meter_mcp.py | 5 +- 16 files changed, 341 insertions(+), 68 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 1add2b1..f1913c0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -81,7 +81,7 @@ Token Meter reads local agent traces, calculates clearly labeled usage estimates - Keep `meter.py` and `token_meter_mcp.py` on the Python standard library. - Keep the dashboard local-only; do not add hosted assets, analytics, or telemetry. - Never output, commit, persist, or transmit prompts, responses, reasoning, tool contents, credentials, account data, or raw traces. -- Work insights are the only sanctioned consumer of typed user-turn text, the preceding assistant-reply tail (600 characters), per request, the basenames of files the agent changed, and the subjects (200 characters) of the user's own pushed commits for Git work types: they stay in the bounded in-memory queue, are sent only to the validated loopback Ollama URL, and only salted keys, labels, and content-free counts (costs, tool and command kinds, file categories) persist. Do not add other consumers. +- Work insights are the only sanctioned consumer of typed user-turn text, the preceding assistant-reply tail (600 characters), per request, the basenames of files the agent changed, and, for Git work types, the subject and first body line (200 characters each) of the user's own pushed commits with a content-free count of the kinds of files each changed: they stay in the bounded in-memory queue, are sent only to the validated loopback Ollama URL, and only salted keys, labels, and content-free counts (costs, tool and command kinds, file categories) persist. Do not add other consumers. - Provider-account requests must remain narrow, bounded, sanitized, and read-only. - Cursor databases, transcripts, and request logs are read-only inputs. - Label estimates as estimates. Unavailable evidence must not become a measured zero. diff --git a/README.md b/README.md index 93c6542..f87b723 100644 --- a/README.md +++ b/README.md @@ -257,7 +257,8 @@ skills, so the package targets Claude Code. `~/.token-meter/mcp-evidence-cache.json`, never paths, titles, or text. It is safe to delete. - Costs are estimates. Codex uses public API rates, which can differ from subscription billing. - Work insights, when on, send typed prompts, the end of the reply before each one, - changed file names, and your pushed commits' subject lines only to the local Ollama; + changed file names, and your pushed commits' messages (subject and first body line) with + counts of the kinds of files they changed only to the local Ollama; Token Meter stores labels and counts, never text. Full details: [User guide](specs/USER_GUIDE.md#data-and-evidence) · diff --git a/dashboard/markup/git-learn-tools.html b/dashboard/markup/git-learn-tools.html index b9b7a9c..c4b6ace 100644 --- a/dashboard/markup/git-learn-tools.html +++ b/dashboard/markup/git-learn-tools.html @@ -46,7 +46,7 @@
-

Work pushed by type

Reading labels…

+

Work pushed by type

Reading labels…

diff --git a/specs/AGENTS.md b/specs/AGENTS.md index d94fd54..92c7d00 100644 --- a/specs/AGENTS.md +++ b/specs/AGENTS.md @@ -79,7 +79,7 @@ Token Meter reads local agent traces, calculates clearly labeled usage estimates - Keep `meter.py` and `token_meter_mcp.py` on the Python standard library. - Keep the dashboard local-only; do not add hosted assets, analytics, or telemetry. - Never output, commit, persist, or transmit prompts, responses, reasoning, tool contents, credentials, account data, or raw traces. -- Work insights are the only sanctioned consumer of typed user-turn text, the preceding assistant-reply tail (600 characters), per request, the basenames of files the agent changed, and the subjects (200 characters) of the user's own pushed commits for Git work types: they stay in the bounded in-memory queue, are sent only to the validated loopback Ollama URL, and only salted keys, labels, and content-free counts (costs, tool and command kinds, file categories) persist. Do not add other consumers. +- Work insights are the only sanctioned consumer of typed user-turn text, the preceding assistant-reply tail (600 characters), per request, the basenames of files the agent changed, and, for Git work types, the subject and first body line (200 characters each) of the user's own pushed commits with a content-free count of the kinds of files each changed: they stay in the bounded in-memory queue, are sent only to the validated loopback Ollama URL, and only salted keys, labels, and content-free counts (costs, tool and command kinds, file categories) persist. Do not add other consumers. - Provider-account requests must remain narrow, bounded, sanitized, and read-only. - Cursor databases, transcripts, and request logs are read-only inputs. - Label estimates as estimates. Unavailable evidence must not become a measured zero. diff --git a/specs/ARCHITECTURE.md b/specs/ARCHITECTURE.md index f33ba35..6f56a27 100644 --- a/specs/ARCHITECTURE.md +++ b/specs/ARCHITECTURE.md @@ -714,7 +714,7 @@ No public HTTP, native, MCP, or telemetry projection may contain: - unbounded trace rows or provider-controlled payloads. Work insights read typed user-turn text, the preceding assistant-reply tail, and -the subjects of the user's own pushed commits (Git work types) only in process memory, only when the user enables them, and send them only to +the subject, first body line, and changed-file kind counts of the user's own pushed commits (Git work types) only in process memory, only when the user enables them, and send them only to a loopback (`127.0.0.1`, `::1`; `localhost` pinned to `127.0.0.1`) plain-HTTP Ollama endpoint with redirects refused; remote or cloud Ollama models are rejected by a local `/api/tags` check before every model request. The ledger, logs, and diff --git a/specs/SECURITY.md b/specs/SECURITY.md index 046f712..187febd 100644 --- a/specs/SECURITY.md +++ b/specs/SECURITY.md @@ -44,8 +44,10 @@ The Git page is a local-only Git reader. It inspects bounded remote-tracking ref for successful-push markers and never fetches, pulls, pushes, updates refs, or contacts a remote. Its SQLite ledger stores salted repository/object keys, timestamped clear baselines, numeric text-line totals, coverage state, and enum -commit work-type labels only. With Work insights on, a commit subject is read into -the Work insights in-memory queue and sent only to the loopback model; it is never stored. +commit work-type labels only. With Work insights on, a commit's subject and first +body line, with a count of the kinds of files it changed (never paths or names), are +read into the Work insights in-memory queue and sent only to the loopback model; they +are never stored. Public project suffixes use the same per-machine salt. It must not persist or project paths, remote/ref names, identities, messages, filenames, diff content, raw object IDs, subprocess output, or exceptions. diff --git a/specs/USER_GUIDE.md b/specs/USER_GUIDE.md index d9e66bd..9657e17 100644 --- a/specs/USER_GUIDE.md +++ b/specs/USER_GUIDE.md @@ -429,8 +429,11 @@ remotes, branches, identities, messages, file names, or diff content. **Work pushed by type** stacks each day's pushed commits, or lines, by the kind of work: features, fixes, refactoring, performance, tests, docs, and build & ops. -With **Settings → Work insights** on, the local model reads each of your pushed -commits' subject line once, in memory, and Token Meter keeps only the label. +With **Settings → Work insights** on, a commit whose subject starts with a +conventional type (`feat:`, `fix(ui):`, `docs:`, `chore:`, and so on) is labeled +with that type as written. For other commits the local model reads the subject, +the first line of the body, and how many files of each kind (code, UI, tests, +docs, config, and so on) changed, once, in memory; Token Meter keeps only the label. Commits it was unsure about show as Unclear; commits not labeled yet (Work insights off, or still queued) stay visible as their own segment, never as zero. Deleting Work labels also deletes these labels. Missing diff --git a/specs/WORK_INSIGHTS.md b/specs/WORK_INSIGHTS.md index 968747e..2feee4c 100644 --- a/specs/WORK_INSIGHTS.md +++ b/specs/WORK_INSIGHTS.md @@ -226,27 +226,47 @@ History to backfill: 30, 90 (default), 365 days, or all. ### Pushed commits (Git page) The same worker labels the kind of work in each of the user's own pushed, -non-merge commits for the Git page's **Work pushed by type** chart. The Git scan -reads only the subject line (`%s`, whitespace-collapsed, 200 characters) of a -commit it records, or of an already-recorded commit that has no label for the -current question version, within the history-to-backfill window; at most 200 -subjects per five-minute scan. Subjects wait in a separate in-memory queue (256 -commits) that alternates with session requests, so neither starves, and they go -through the same pause, pace, memory, CPU, battery, and local-model checks. - -One question, `commit_type` (version `c1`), asks for feature, fix, refactor, -perf, test, docs, or ops, in both option orders averaged, after six invented -worked examples. Option descriptions carry verb cues ("often starts with Add", +non-merge commits for the Git page's **Work pushed by type** chart. + +A subject that starts with a conventional type (`feat:`, `fix(scope)!:`, `docs:`, +`chore:`, `perf:`, `test:`, `refactor:`, `style:`, `ci:`, `build:`, and a few +synonyms) is labeled with that type at once: confidence 1, model `prefix`, no model +call and no queue slot. Unlisted types (`revert:`, `polish:`, `wip:`) go to the model. + +For the rest, the Git scan reads one `git show --numstat --format=%s%x00%b%x00` per +commit it records, or per already-recorded commit with no label for the current +question version within the history-to-backfill window; at most 200 per +five-minute scan. It keeps the subject and the first body line that is not a +`Co-authored-by` or `Signed-off-by` trailer (whitespace-collapsed, 200 characters +each), and reduces the numstat paths, inside the scan, to a content-free line such +as `Changed files: 3 code, 1 tests (+120 / -30 lines).` using the Work file kinds. +Paths and file names never leave the scan. These wait in a separate in-memory +queue (256 commits) that alternates with session requests, so neither starves, +and go through the same pause, pace, memory, CPU, battery, and local-model checks. + +One question, `commit_type` (version `c2`), asks for feature, fix, refactor, +perf, test, docs, or ops from the commit message first and the changed files as +supporting evidence, in both option orders averaged, after six invented worked +examples. Option descriptions carry verb cues ("often starts with Add", "Fix, Stop, Prevent"). Below 0.5 confidence the label is `unclear`. Only the -type, confidence, version, and model digest are stored, in the Git ledger keyed -by the salted commit key; a failed commit is offered again by later scans up to -three attempts. A version bump makes older labels read as not labeled yet. - -Calibration (one local machine, 609 own commits from three repositories, 90 -days; reference is the author's own conventional-commit prefix): 95.9% agreement -with the prefix kept in the subject, and 58% with the prefix stripped (a proxy -for free-form subjects), up from 89.5% and 34% before the verb cues and -examples. Each commit takes two model calls. +type, confidence, version, and model digest (or `prefix`) are stored, in the Git +ledger keyed by the salted commit key; a failed commit is offered again by later +scans up to three attempts. A version bump makes older labels read as not labeled +yet, so `c1` labels are redone once. + +Calibration (one local machine, 664 own commits from three repositories, 180 +days; the reference is each author's own conventional-commit type with the prefix +stripped from the subject, a proxy for free-form messages; dev and held-out +halves split by commit hash). Held-out, both orders, 200 commits: + +| Model input | Agreement | With a body (167) | No body (33) | +|---|---|---|---| +| Subject only (`c1`) | 57.5% | 61% | 39% | +| Subject, first body line, file kinds (`c2`) | 72.0% | 73% | 67% | + +On the dev half, adding file basenames gave no gain (65% either way) and a +measured-prior correction gained under one point, so neither shipped. Prefixed +commits are now exact by construction. Each model-labeled commit takes two calls. ## 4. From labels to outcomes diff --git a/tests/test_git_delivery.py b/tests/test_git_delivery.py index 20ccd51..c65e856 100644 --- a/tests/test_git_delivery.py +++ b/tests/test_git_delivery.py @@ -2870,9 +2870,10 @@ def test_shape_rows_expose_each_measure_with_its_own_day_count(self): class FakeCommitClassifier: - def __init__(self, version="c1", since_day="0000-00-00", room=1000): + def __init__(self, version=git_delivery.COMMIT_LABEL_VERSION, since_day="0000-00-00", room=1000): self.version, self.since_day, self.room = version, since_day, room self.subjects = [] + self.details = [] def commit_label_request(self): return {"version": self.version, "since_day": self.since_day} @@ -2882,7 +2883,8 @@ def commit_status(self, repo_key, object_key): return "queued" return "take" if len(self.subjects) < self.room else "later" - def observe_commit(self, repo_key, object_key, subject, observed_at): + def observe_commit(self, repo_key, object_key, subject, observed_at, detail="", files=""): + self.details.append((detail, files)) if len(self.subjects) >= self.room: return False self.subjects.append((repo_key, object_key, subject)) @@ -2895,10 +2897,11 @@ def _subject_runner(subjects, **kwargs): def runner(argv, **options): args = tuple(argv[argv.index("-C") + 2:]) - if args[0:3] == ("show", "-s", "--format=%s"): + if args[0:3] == ("show", "--numstat", "--format=%s%x00%b%x00"): if reads is not None: reads.append("subject") - return {"returncode": 0, "stdout": subjects.get(args[-1], "Add a chart") + "\n"} + return {"returncode": 0, "stdout": subjects.get(args[-1], "Add a chart") + + "\x00Explain the change.\n\x00\n2\t1\tsrc/app.py\n"} return base(argv, **options) return runner @@ -2912,10 +2915,10 @@ def test_labels_persist_only_type_confidence_version_and_model(self): with tempfile.TemporaryDirectory() as tmp: ledger = self.ledger(tmp) ledger.record("repo", "obj", local_timestamp("2026-09-03"), 10, 2) - self.assertTrue(ledger.record_commit_label("repo", "obj", "fix", 0.91, "c1", "digest", 5)) - self.assertFalse(ledger.record_commit_label("repo", "missing", "fix", 0.9, "c1", "digest", 5)) + self.assertTrue(ledger.record_commit_label("repo", "obj", "fix", 0.91, git_delivery.COMMIT_LABEL_VERSION, "digest", 5)) + self.assertFalse(ledger.record_commit_label("repo", "missing", "fix", 0.9, git_delivery.COMMIT_LABEL_VERSION, "digest", 5)) with self.assertRaises(ValueError): - ledger.record_commit_label("repo", "obj", "chores", 0.9, "c1", "digest", 5) + ledger.record_commit_label("repo", "obj", "chores", 0.9, git_delivery.COMMIT_LABEL_VERSION, "digest", 5) with sqlite3.connect(Path(tmp) / "delivery.sqlite3") as connection: columns = {row[1] for row in connection.execute("PRAGMA table_info(delivery_commit_labels)")} self.assertEqual(columns, {"repo_key", "object_key", "work_type", "confidence", "version", @@ -2927,16 +2930,16 @@ def test_daily_type_rows_split_by_label_and_treat_other_versions_as_unlabeled(se day = local_timestamp("2026-09-03") for name, lines in (("a", 10), ("b", 6), ("c", 4), ("d", 1)): ledger.record("repo", name, day, lines, 0) - ledger.record_commit_label("repo", "a", "feature", 0.9, "c1", "m", 1) - ledger.record_commit_label("repo", "b", "feature", 0.9, "c1", "m", 1) + ledger.record_commit_label("repo", "a", "feature", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) + ledger.record_commit_label("repo", "b", "feature", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) ledger.record_commit_label("repo", "c", "fix", 0.9, "c0", "m", 1) # stale question version - rows = ledger.daily_type_rows(["repo"], "2026-09-03", "2026-09-03", "c1") + rows = ledger.daily_type_rows(["repo"], "2026-09-03", "2026-09-03", git_delivery.COMMIT_LABEL_VERSION) self.assertEqual(sorted((row["work_type"], row["commits"], row["lines"]) for row in rows), [("", 2, 5), ("feature", 2, 16)]) - self.assertEqual(ledger.unlabeled_keys("repo", "c1", "2026-09-01", 10), {"c", "d"}) + self.assertEqual(ledger.unlabeled_keys("repo", git_delivery.COMMIT_LABEL_VERSION, "2026-09-01", 10), {"c", "d"}) ledger.clear(10) - self.assertEqual(ledger.daily_type_rows(["repo"], "2026-09-03", "2026-09-03", "c1"), []) + self.assertEqual(ledger.daily_type_rows(["repo"], "2026-09-03", "2026-09-03", git_delivery.COMMIT_LABEL_VERSION), []) def test_scan_hands_new_commit_subjects_to_the_classifier_and_never_stores_them(self): tip = "a" * 40 @@ -2955,7 +2958,7 @@ def test_scan_hands_new_commit_subjects_to_the_classifier_and_never_stores_them( self.assertEqual([subject for _repo, _obj, subject in classifier.subjects], ["Fix the secret-sauce overflow"]) repo_key, object_key, _subject = classifier.subjects[0] - self.assertTrue(service.ledger.record_commit_label(repo_key, object_key, "fix", 0.8, "c1", "m", 1)) + self.assertTrue(service.ledger.record_commit_label(repo_key, object_key, "fix", 0.8, git_delivery.COMMIT_LABEL_VERSION, "m", 1)) stored = b"".join(path.read_bytes() for path in Path(tmp).iterdir()) self.assertNotIn(b"secret-sauce", stored) @@ -2988,7 +2991,7 @@ def test_scan_backfills_subjects_of_already_counted_unlabeled_commits(self): service.scan([{"root": "/repo", "project": "repo · a1"}]) self.assertEqual([subject for *_keys, subject in classifier.subjects], ["Add the weekly chart"]) repo_key, object_key, _subject = classifier.subjects[0] - service.ledger.record_commit_label(repo_key, object_key, "feature", 0.9, "c1", "m", 1) + service.ledger.record_commit_label(repo_key, object_key, "feature", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) classifier.subjects.clear() service.scan([{"root": "/repo", "project": "repo · a1"}]) self.assertEqual(classifier.subjects, []) # labeled commits are not offered again @@ -3009,7 +3012,7 @@ def test_commits_no_push_introduces_are_not_walked_for_again(self): service.commit_classifier = classifier service.scan([{"root": "/repo", "project": "repo · a1"}]) # offers the tip; the orphan is missed for repo, obj, _subject in classifier.subjects: - service.ledger.record_commit_label(repo, obj, "feature", 0.9, "c1", "m", 1) + service.ledger.record_commit_label(repo, obj, "feature", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) calls.clear() service.scan([{"root": "/repo", "project": "repo · a1"}]) service.scan([{"root": "/repo", "project": "repo · a1"}]) @@ -3079,6 +3082,30 @@ def test_a_walk_truncated_by_a_size_cap_still_lets_unfound_commits_be_skipped(se service.scan([{"root": "/repo", "project": "repo · a1"}]) self.assertIn("orphan", service._subject_skips.get(repo_key, set())) + def test_a_failed_backfill_walk_does_not_skip_and_is_retried(self): + tip = "a" * 40 + fail_walks = [False] + base = _subject_runner({}, tracked=("refs/remotes/origin/main",), + pushes={"refs/remotes/origin/main": tip}) + + def runner(argv, **options): + if tuple(argv[argv.index("-C") + 2:])[0] == "rev-list" and fail_walks[0]: + return {"returncode": 128, "stdout": ""} + return base(argv, **options) + + with tempfile.TemporaryDirectory() as tmp: + service = meter.GitDeliveryService(str(Path(tmp) / "delivery.sqlite3"), runner=runner, + now=lambda: 200, salt="test-salt") + service.scan([{"root": "/repo", "project": "repo · a1"}]) # counted before labeling existed + service.commit_classifier = classifier = FakeCommitClassifier() + fail_walks[0] = True + service.scan([{"root": "/repo", "project": "repo · a1"}]) + self.assertEqual(classifier.subjects, []) + self.assertEqual(service._subject_skips, {}) + fail_walks[0] = False + service.scan([{"root": "/repo", "project": "repo · a1"}]) + self.assertEqual(len(classifier.subjects), 1) + def test_a_failed_subject_read_is_retried_by_a_later_scan(self): tip = "a" * 40 failures = [True] @@ -3086,7 +3113,7 @@ def test_a_failed_subject_read_is_retried_by_a_later_scan(self): pushes={"refs/remotes/origin/main": tip}) def runner(argv, **options): - if tuple(argv[argv.index("-C") + 2:])[0:3] == ("show", "-s", "--format=%s") and failures: + if tuple(argv[argv.index("-C") + 2:])[0:3] == ("show", "--numstat", "--format=%s%x00%b%x00") and failures: failures.pop() return {"returncode": 128, "stdout": ""} return base(argv, **options) @@ -3130,7 +3157,7 @@ def test_query_reports_daily_work_types_with_unlabeled_commits_explicit(self): # The scanned push is dated by the fake reflog long ago; these two land in the window. service.ledger.record(repo_key, "first", local_timestamp("2026-09-04"), 2, 1) service.ledger.record(repo_key, "second", local_timestamp("2026-09-04"), 7, 0) - service.ledger.record_commit_label(repo_key, "second", "docs", 0.9, "c1", "m", 1) + service.ledger.record_commit_label(repo_key, "second", "docs", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) payload = service.query("", "7", [], ["repo · a1"], [{"root": "/repo", "project": "repo · a1"}]) work = payload["work_types"] @@ -3226,14 +3253,14 @@ def test_commit_labels_are_deleted_even_when_the_work_ledger_delete_fails(self): service = meter.GitDeliveryService(str(Path(tmp) / "delivery.sqlite3"), runner=_ref_runner(), now=lambda: 200, salt="test-salt") service.ledger.record("repo", "obj", local_timestamp("2026-09-03"), 3, 1) - service.ledger.record_commit_label("repo", "obj", "docs", 0.9, "c1", "m", 1) + service.ledger.record_commit_label("repo", "obj", "docs", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) work = mock.Mock() work.clear.side_effect = meter._work.LabelDeleteError("busy") with mock.patch.object(meter, "git_delivery_service", return_value=service), \ mock.patch.object(meter, "work_insights_service_if_started", return_value=work): with self.assertRaises(meter._work.LabelDeleteError): meter.clear_work_insights() - rows = service.ledger.daily_type_rows(["repo"], "2026-09-03", "2026-09-03", "c1") + rows = service.ledger.daily_type_rows(["repo"], "2026-09-03", "2026-09-03", git_delivery.COMMIT_LABEL_VERSION) self.assertEqual([(row["work_type"], row["commits"]) for row in rows], [("", 1)]) def test_a_git_ledger_failure_is_reported_not_swallowed(self): @@ -3249,10 +3276,45 @@ def test_deleting_work_labels_also_deletes_commit_work_type_labels(self): service = meter.GitDeliveryService(str(Path(tmp) / "delivery.sqlite3"), runner=_ref_runner(), now=lambda: 200, salt="test-salt") service.ledger.record("repo", "obj", local_timestamp("2026-09-03"), 3, 1) - service.ledger.record_commit_label("repo", "obj", "docs", 0.9, "c1", "m", 1) + service.ledger.record_commit_label("repo", "obj", "docs", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) with mock.patch.object(meter, "git_delivery_service", return_value=service), \ mock.patch.object(meter, "work_insights_service_if_started", return_value=None), \ mock.patch.object(meter, "TOKEN_METER_WORK_INSIGHTS_DB", str(Path(tmp) / "work.sqlite3")): meter.clear_work_insights() - rows = service.ledger.daily_type_rows(["repo"], "2026-09-03", "2026-09-03", "c1") + rows = service.ledger.daily_type_rows(["repo"], "2026-09-03", "2026-09-03", git_delivery.COMMIT_LABEL_VERSION) self.assertEqual([(row["work_type"], row["commits"]) for row in rows], [("", 1)]) + + +class CommitDetailsTests(unittest.TestCase): + def test_details_keep_the_first_body_line_and_reduce_paths_to_kind_counts(self): + output = ("Stop the overflow\x00\nClamp the secret-widget chart width.\nMore text.\n" + "Co-Authored-By: Someone \n\x00\n" + "10\t2\tdashboard/js/secret_widget.js\n3\t0\ttests/test_secret_widget.py\n-\t-\tlogo.png\n") + subject, detail, files = git_delivery.commit_details(output) + self.assertEqual(subject, "Stop the overflow") + self.assertEqual(detail, "Clamp the secret-widget chart width.") + self.assertEqual(files, "Changed files: 1 UI, 1 tests, 1 images (+13 / -2 lines).") + self.assertNotIn("secret_widget", files) + self.assertNotIn(".js", files) + + def test_trailer_only_body_and_no_files_read_as_no_detail(self): + subject, detail, files = git_delivery.commit_details("Bump version\x00Signed-off-by: A \n\x00\n") + self.assertEqual((subject, detail, files), ("Bump version", "", "Changed files: none.")) + long_subject, long_detail, _files = git_delivery.commit_details("x" * 500 + "\x00" + "y" * 500 + "\x00") + self.assertEqual((len(long_subject), len(long_detail)), (200, 200)) + + def test_scan_hands_detail_and_file_kinds_but_never_paths_to_the_classifier(self): + tip = "a" * 40 + with tempfile.TemporaryDirectory() as tmp: + service = meter.GitDeliveryService( + str(Path(tmp) / "delivery.sqlite3"), + runner=_subject_runner({tip: "Stop the overflow"}, tracked=("refs/remotes/origin/main",), + pushes={"refs/remotes/origin/main": tip}), + now=lambda: 200, salt="test-salt", + ) + service.commit_classifier = classifier = FakeCommitClassifier() + service.scan([{"root": "/repo", "project": "repo · a1"}]) + stored = b"".join(path.read_bytes() for path in Path(tmp).iterdir()) + self.assertEqual(classifier.details, [("Explain the change.", "Changed files: 1 code (+2 / -1 lines).")]) + self.assertNotIn("app.py", json.dumps(classifier.details)) + self.assertNotIn(b"Explain the change", stored) diff --git a/tests/test_mcp_delivery.py b/tests/test_mcp_delivery.py index 3dd9bee..e2f1569 100644 --- a/tests/test_mcp_delivery.py +++ b/tests/test_mcp_delivery.py @@ -72,6 +72,26 @@ def test_summary_matches_the_git_page_totals_without_labels(self): self.assertNotIn("project", {key for key in result}) self.assertNotIn("projects", text) + def test_work_types_are_estimated_counts_with_unlabeled_commits_explicit(self): + self.service.ledger.record_commit_label( + self.repo_key, "{:064x}".format(1), "fix", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) + payload = self.query() + summary = delivery.git_stats_projection(payload, "7", "summary", now=1) + self.assertEqual(summary["work_types"]["by_type"], { + "fix": {"commits": 1, "changed_lines": 150}, + "unlabeled": {"commits": 1, "changed_lines": 50}, + }) + self.assertTrue(summary["work_types"]["estimate"]) + self.assertEqual((summary["work_types"]["commits"], summary["work_types"]["labeled_commits"]), (2, 1)) + days = delivery.git_stats_projection(payload, "7", "days", limit=7) + today = next(row for row in days["days"] if row["day"] == self.today.isoformat()) + self.assertEqual(today["work_types"], {"fix": {"commits": 1, "changed_lines": 150}}) + payload["work_types"]["totals"]["secret-type"] = {"commits": 9, "lines": 9} + text = encoded(delivery.git_stats_projection(payload, "7", "summary", now=1)) + self.assertNotIn("secret-type", text) + for forbidden in FORBIDDEN: + self.assertNotIn(forbidden, text) + def test_unmeasured_evidence_is_null_and_explained_not_zero(self): empty = git_delivery.GitDeliveryService( os.path.join(self.tmp.name, "empty.sqlite3"), now=lambda: 1_790_000_000, diff --git a/tests/test_work_insights.py b/tests/test_work_insights.py index f239261..62f4844 100644 --- a/tests/test_work_insights.py +++ b/tests/test_work_insights.py @@ -3596,7 +3596,7 @@ class SubagentModelTrendTests(unittest.TestCase): def test_model_days_are_aggregated_projected_and_rendered(self): from token_meter.domain import agents from token_meter import projections - record = {"id": "c1", "kind": "spawned", "parent_id": "root", "runtime": "codex", "model": "gpt-5.6-terra", + record = {"id": W.COMMIT_LABEL_VERSION, "kind": "spawned", "parent_id": "root", "runtime": "codex", "model": "gpt-5.6-terra", "role": "tester", "activity_state": "complete", "cost": 2.0, "cost_available": True, "tokens": 10, "tokens_available": True, "last_activity_at": 1_790_000_000, "retries": 0, "failed_attempts": 0, "executions": 1, "depth": 1} @@ -3677,7 +3677,7 @@ def test_subject_is_labeled_by_the_local_model_and_only_the_label_leaves_memory( self.assertEqual(len(self.stored), 1) repo, obj, value, confidence, version, model, _labeled_at = self.stored[0] - self.assertEqual((repo, obj, value, version, model), ("repo", "obj", "fix", "c1", "digest-1")) + self.assertEqual((repo, obj, value, version, model), ("repo", "obj", "fix", W.COMMIT_LABEL_VERSION, "digest-1")) self.assertGreater(confidence, 0.5) self.assertEqual(len(FakeClient.prompts), 2) # both option orders self.assertIn("Stop the menu from flickering", FakeClient.prompts[0]) @@ -3713,7 +3713,7 @@ def test_no_subjects_are_taken_while_paused_or_for_exhausted_commits(self): def test_request_names_the_label_version_and_history_horizon(self): self.assertEqual(self.service.commit_label_request(), - {"version": "c1", "since_day": "0000-00-00"}) + {"version": W.COMMIT_LABEL_VERSION, "since_day": "0000-00-00"}) self.settings["backfill_days"] = 30 since = self.service.commit_label_request()["since_day"] self.assertEqual(since, time.strftime("%Y-%m-%d", time.localtime(self.clock.now - 30 * 86400))) @@ -3742,3 +3742,41 @@ def test_commit_subjects_alternate_with_session_requests(self): self.service.step() self.service.step() self.assertEqual(len(self.stored), 1) # one session item, then one commit + + +class CommitPrefixAndEvidenceTests(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.addCleanup(self._tmp.cleanup) + self.service, self.settings, self.clock = make_service(self._tmp.name) + self.stored = [] + self.service.commit_label_sink = lambda *args: self.stored.append(args) + + def test_conventional_prefix_is_the_label_with_no_model_call_or_queue_slot(self): + for subject, expected in (("feat(git): add a chart", "feature"), ("fix!: stop a crash", "fix"), + ("chore(deps): bump x", "ops"), ("Docs: explain setup", "docs")): + self.assertTrue(self.service.observe_commit("repo", subject, subject)) + self.assertEqual(self.stored[-1][2:6], (expected, 1.0, W.COMMIT_LABEL_VERSION, W.PREFIX_MODEL)) + self.assertFalse(self.service.commit_queue) + self.assertEqual(FakeClient.prompts, []) + + def test_unlisted_or_missing_prefixes_still_go_to_the_model(self): + for subject in ("revert: undo the chart", "Stop the chart from overflowing", "polish: tidy copy"): + self.assertIsNone(W.conventional_type(subject)) + self.service.observe_commit("repo", subject, subject) + self.assertEqual(len(self.service.commit_queue), 3) + self.assertEqual(self.stored, []) + + def test_prompt_carries_subject_detail_and_file_kinds(self): + FakeClient.responder = answer_for("fix") + self.service.observe_commit("repo", "obj", "Stop the overflow", detail="Clamp the chart width.", + files="Changed files: 1 UI (+3 / -1 lines).") + for _ in range(5): + self.clock.now += max(self.service.step(), 0.0) + 0.01 + prompt = FakeClient.prompts[-1] + state = prompt.rsplit("State:\n", 1)[1].split("\n\nQuestion:", 1)[0] + self.assertEqual(json.loads(state), "Commit message: Stop the overflow\nMessage details: Clamp the chart " + "width.\nChanged files: 1 UI (+3 / -1 lines).") + self.assertEqual(self.stored[0][2], "fix") + with open(os.path.join(self._tmp.name, "work.sqlite3"), "rb") as handle: + self.assertNotIn(b"Clamp the chart", handle.read()) diff --git a/token_meter/mcp/delivery.py b/token_meter/mcp/delivery.py index 42a9a2b..c2de7b4 100644 --- a/token_meter/mcp/delivery.py +++ b/token_meter/mcp/delivery.py @@ -196,12 +196,45 @@ def _git_model(value): } +COMMIT_WORK_TYPES = ("feature", "fix", "refactor", "perf", "test", "docs", "ops", "unclear", "unlabeled") + + +def _work_type_cells(value): + """Commits and changed lines per commit work type; only known enum keys, counts only.""" + cells = {} + for key in COMMIT_WORK_TYPES: + cell = value.get(key) if isinstance(value, dict) else None + if isinstance(cell, dict): + cells[key] = {"commits": _count(cell.get("commits")) or 0, "changed_lines": _count(cell.get("lines")) or 0} + return cells + + +def _git_work_types(value): + if not isinstance(value, dict): + return {"available": False, "estimate": True} + return { + "available": bool(value.get("available")), + "estimate": True, + "method": "local_model_commit_subject", + "classifier_enabled": bool(value.get("classifier_enabled")), + "commits": _count(value.get("commits")), + "labeled_commits": _count(value.get("labeled_commits")), + "by_type": _work_type_cells(value.get("totals")), + } + + def git_stats_projection(payload, range_key, view, cursor=None, limit=31, now=None): if not isinstance(payload, dict) or payload.get("ok") is not True: raise MCPQueryError("evidence_unavailable", "Git delivery evidence is unavailable") coverage = payload.get("coverage") or {} measured = _count(coverage.get("measured_repositories")) or 0 days = [row for row in (_git_day(item) for item in payload.get("days") or ()) if row] + type_days = { + str(item.get("day")): _work_type_cells(item.get("types")) + for item in ((payload.get("work_types") or {}).get("days") or ()) if isinstance(item, dict) + } + for row in days: + row["work_types"] = type_days.get(row["day"], {}) result = { "ok": True, "schema_version": SCHEMA_VERSION, @@ -246,6 +279,7 @@ def git_stats_projection(payload, range_key, view, cursor=None, limit=31, now=No "available": bool(comparison.get("available")), }, "models": [row for row in (_git_model(item) for item in payload.get("model_rows") or ()) if row], + "work_types": _git_work_types(payload.get("work_types")), "model_attribution": { "available": bool(model_coverage.get("available")), "estimate": True, diff --git a/token_meter/mcp/schema.py b/token_meter/mcp/schema.py index c3acfe7..3c3e7e7 100644 --- a/token_meter/mcp/schema.py +++ b/token_meter/mcp/schema.py @@ -178,6 +178,14 @@ "current.reasoning_share_of_output": {"unit": "ratio", "description": "Reasoning tokens over output tokens."}, "change": {"unit": "percent", "description": "Neutral change from the equal-length previous period."}, "models": {"description": "Model, runtime, effort, covered spend, and spend-share attributed lines."}, + "work_types": { + "description": ( + "Estimate: pushed commits and changed lines by kind of work (feature, fix, refactor, perf, test, " + "docs, ops, unclear, unlabeled), labeled from a conventional prefix, or else by the local Work model from each commit's message and changed-file kinds. " + "unlabeled means not labeled yet (Work insights off, queued, or older than its history), not zero. " + "Days rows carry the same per-day split." + ), + }, "coverage": {"description": "Selected, measured, and comparable repository counts and spend coverage."}, } diff --git a/token_meter/services/git_delivery.py b/token_meter/services/git_delivery.py index e1edf88..69e7eb3 100644 --- a/token_meter/services/git_delivery.py +++ b/token_meter/services/git_delivery.py @@ -1,5 +1,6 @@ """Privacy-bounded aggregation of locally observed successful Git pushes.""" +import collections import contextlib import datetime import hashlib @@ -11,6 +12,7 @@ import subprocess import threading +from token_meter.domain.work_evidence import file_kind from token_meter.platforms import ProcessPurpose, platform_services @@ -31,8 +33,37 @@ MAX_SUBJECTS_PER_SCAN = 200 MAX_SUBJECT_CHARS = 200 MAX_SUBJECT_SKIPS = 5_000 +MAX_DETAIL_CHARS = 200 +# Plain words for each file kind in the classifier's changed-files line; paths never leave the scan. +_FILE_KIND_WORDS = {"frontend": "UI", "backend": "code", "data": "data", "infra": "infrastructure", + "tooling": "scripts", "docs": "docs", "tests": "tests", "config": "config", "media": "images"} +_TRAILER_RE = re.compile(r"^(co-authored-by|signed-off-by)\s*:", re.I) + + +def commit_details(output): + """Split ``show --numstat --format=%s%x00%b%x00`` output into (subject, first body line, files line). + + The files line is content-free: counts of changed file kinds and lines, never paths or names. + """ + subject, _sep, rest = str(output or "").partition("\0") + body, _sep, numstat = rest.partition("\0") + detail = next((line.strip() for line in body.splitlines() + if line.strip() and not _TRAILER_RE.match(line.strip())), "") + kinds = collections.Counter() + added = deleted = 0 + for line in numstat.splitlines(): + parts = line.split("\t", 2) + if len(parts) != 3: + continue + kinds[_FILE_KIND_WORDS.get(file_kind(parts[2]), "other")] += 1 + if parts[0].isdecimal() and parts[1].isdecimal(): + added += int(parts[0]) + deleted += int(parts[1]) + files = ("Changed files: " + ", ".join(f"{count} {kind}" for kind, count in kinds.most_common()) + + f" (+{added} / -{deleted} lines).") if kinds else "Changed files: none." + return (" ".join(subject.split())[:MAX_SUBJECT_CHARS], " ".join(detail.split())[:MAX_DETAIL_CHARS], files) # Bumped when the commit-type question changes; labels from another version read as not labeled yet. -COMMIT_LABEL_VERSION = "c1" +COMMIT_LABEL_VERSION = "c2" COMMIT_TYPE_KEYS = ("feature", "fix", "refactor", "perf", "test", "docs", "ops", "unclear") _GIT_OID_LENGTHS = frozenset((40, 64)) _MUTATING_OR_NETWORK_GIT_VERBS = frozenset({ @@ -856,7 +887,10 @@ def _commit_request(self): return request def _send_subject(self, root, repo_key, object_key, oid, observed_at, wanted): - """Read one commit subject and hand it to the classifier queue; nothing is written to disk.""" + """Read one commit's subject, first body line, and changed-file kinds for the classifier queue. + + Nothing is written to disk; paths are reduced to kind counts here and never leave the scan. + """ if self._subjects_left <= 0: return wanted.discard(object_key) @@ -873,16 +907,18 @@ def _send_subject(self, root, repo_key, object_key, oid, observed_at, wanted): self._subjects_left = 0 # off, paused, or full: later scans offer the rest return self._subjects_left -= 1 - code, output = self._run_git(root, ("show", "-s", "--format=%s", oid)) + code, output = self._run_git(root, ("show", "--numstat", "--format=%s%x00%b%x00", oid)) if code != 0: self._walk_incomplete = True # possibly transient; offered again by a later scan return - subject = " ".join(str(output or "").split())[:MAX_SUBJECT_CHARS] + subject, detail, files = commit_details(output) if not subject: self._skip_subject(repo_key, object_key) return try: - accepted = self.commit_classifier.observe_commit(repo_key, object_key, subject, observed_at) + accepted = self.commit_classifier.observe_commit( + repo_key, object_key, subject, observed_at, detail=detail, files=files, + ) except Exception: accepted = False if not accepted: diff --git a/token_meter/services/work_insights.py b/token_meter/services/work_insights.py index 44b1eeb..5eea9e9 100644 --- a/token_meter/services/work_insights.py +++ b/token_meter/services/work_insights.py @@ -1,7 +1,7 @@ """Local work classification: bounded queue, label ledger, pacing, and a loopback Ollama client. -Typed user-turn text, the preceding assistant-reply tail, and the subjects of the user's own pushed -commits live only in bounded in-memory queues and are sent only to a loopback Ollama URL. The ledger stores salted keys, enum labels, confidences, +Typed user-turn text, the preceding assistant-reply tail, and the messages (subject and first body line, +with changed-file kind counts) of the user's own pushed commits live only in bounded in-memory queues and are sent only to a loopback Ollama URL. The ledger stores salted keys, enum labels, confidences, and reason codes; it never stores text. """ @@ -131,6 +131,39 @@ ("Cache parsed sessions so reloads are faster", "perf"), ("Explain install steps in the README", "docs"), ) +# A conventional-commit type names the author's own kind of work, so it is taken as the label with no model +# call. Types not listed here ("revert", "polish", "wip", ...) still go to the model. +CONVENTIONAL_TYPES = { + **dict.fromkeys(("feat", "feature", "features"), "feature"), + **dict.fromkeys(("fix", "fixes", "bugfix", "hotfix", "bug"), "fix"), + **dict.fromkeys(("refactor", "style", "cleanup", "lint"), "refactor"), + **dict.fromkeys(("perf", "performance"), "perf"), + **dict.fromkeys(("test", "tests", "testing"), "test"), + **dict.fromkeys(("docs", "doc", "documentation", "readme"), "docs"), + **dict.fromkeys(("chore", "ci", "build", "release", "deps", "dep", "ops", "infra", "config"), "ops"), +} +_CONVENTIONAL_RE = re.compile(r"^\s*([A-Za-z]+)(?:\([^)]{0,60}\))?!?:\s+\S") +PREFIX_MODEL = "prefix" + + +def commit_state(subject, detail="", files=""): + """The classifier's view of one commit: subject, first body line, and changed-file kind counts.""" + lines = [f"Commit message: {subject}"] + detail = " ".join(str(detail or "").split())[:MAX_COMMIT_CHARS] + if detail: + lines.append(f"Message details: {detail}") + files = " ".join(str(files or "").split())[:MAX_COMMIT_CHARS] + if files: + lines.append(files) + return "\n".join(lines) + + +def conventional_type(subject): + """The commit type a conventional-commit subject declares ("fix(ui): ..." -> "fix"), or None.""" + match = _CONVENTIONAL_RE.match(str(subject or "")) + return CONVENTIONAL_TYPES.get(match.group(1).lower()) if match else None + + COMMIT_QUEUE_LIMIT = 256 MAX_COMMIT_CHARS = 200 TURN_TYPES = { @@ -614,8 +647,8 @@ def question_for(name, settings): "agent? Judge only what this message asks for, not the whole session.", "options": list(enumerate(COMPLEXITY_LEVELS))} if name == "commit_type": - question = _choice("What kind of change does this commit message describe? Pick the main purpose.", - COMMIT_TYPES) + question = _choice("What kind of change is this commit? Use the commit message first and the changed " + "files as supporting evidence. Pick the main purpose.", COMMIT_TYPES) question["both_orders"] = True question["shots"] = list(COMMIT_SHOTS) return question @@ -1347,18 +1380,33 @@ def commit_status(self, repo_key, object_key): full = len(self.commit_queue) >= COMMIT_QUEUE_LIMIT return "later" if full or self.commit_label_request() is None else "take" - def observe_commit(self, repo_key, object_key, subject, observed_at=0.0): - """Queue one commit subject in memory; False when the queue is full or labeling is off.""" + def observe_commit(self, repo_key, object_key, subject, observed_at=0.0, detail="", files=""): + """Queue one commit in memory; False when the queue is full or labeling is off. + + ``detail`` is the first body line and ``files`` a content-free line of changed-file kind counts. + """ if self.commit_label_request() is None: return False text = " ".join(str(subject or "").split())[:MAX_COMMIT_CHARS] key = (str(repo_key), str(object_key)) + declared = conventional_type(text) + if declared: + # The author's own type: stored at once, without a model call or a queue slot. + generation = self.generation + with self.write_lock: + if generation == self.generation: + try: + self.commit_label_sink(key[0], key[1], declared, 1.0, COMMIT_LABEL_VERSION, + PREFIX_MODEL, self.clock()) + except (sqlite3.Error, ValueError, OSError): + pass # left unlabeled; a later scan offers it again + return True with self.lock: if not text or key in self.commit_queue or self._commit_attempts.get(key, 0) >= MAX_ITEM_ATTEMPTS: return True if len(self.commit_queue) >= COMMIT_QUEUE_LIMIT: return False - self.commit_queue[key] = (text, self.generation) + self.commit_queue[key] = (commit_state(text, detail, files), self.generation) self.wake.set() return True @@ -1371,7 +1419,7 @@ def _drop_commit(self, key): self.commit_queue.pop(key, None) def _step_commit(self, entry, settings, client, generation): - """Label one commit subject: both option orders, averaged; only the label leaves memory.""" + """Label one commit: both option orders, averaged; only the label leaves memory.""" key, (text, queued_generation) = entry if queued_generation != generation: self._drop_commit(key) diff --git a/token_meter_mcp.py b/token_meter_mcp.py index 09b4b2f..fe72b47 100644 --- a/token_meter_mcp.py +++ b/token_meter_mcp.py @@ -325,8 +325,9 @@ "title": "Aggregate Git delivery", "description": ( "Aggregate the Git page: pushed added, deleted, and changed lines, commits, covered spend, " - "spend per 1k changed lines, output efficiency, neutral period-over-period change, and spend-share " - "model attribution across all measured repositories. view=days returns paginated daily rows. " + "spend per 1k changed lines, output efficiency, neutral period-over-period change, spend-share " + "model attribution, and an estimated split of commits by kind of work (feature, fix, docs, and so " + "on) across all measured repositories. view=days returns paginated daily rows. " "Never returns repository or project names, branches, commit messages, or file paths. Unmeasured " "lines or cost are null with availability flags, never zero. Returns available=false with reason " "history_scan_incomplete while local session history is still being read; call again shortly." From 21dd2f9a9e3622efa5e2e4d2fd26d35c35404ec3 Mon Sep 17 00:00:00 2001 From: Pratik Bhavsar Date: Fri, 6 Nov 2026 12:50:53 +0530 Subject: [PATCH 2/6] fix(git): tighten commit-type evidence after review - Only lowercase Conventional Commits types are taken as the label; capitalised and package-style prefixes (Fix:, testing:, config:) go to the model. - Common Git and Gerrit trailers (Reviewed-by, Change-Id, Cc, ...) are not taken as the detail line. - File kinds use each file's new name, with quoted non-ASCII paths unquoted and renames resolved. - A commit too large to list its files is labeled from its message alone. - MCP method now says conventional_prefix_or_local_model; the chart note no longer claims every label came from the model. - Docs: calibration method, scope (prefix-free commits only), sample limits, line totals in the files line, and the one-time relabel. - Tests for the prefix path's off, pause, clear-fence, and storage-error handling; revert a stray test edit. Co-Authored-By: Claude Code --- AGENTS.md | 2 +- README.md | 2 +- dashboard/js/git.js | 2 +- specs/AGENTS.md | 2 +- specs/SECURITY.md | 2 +- specs/USER_GUIDE.md | 3 +- specs/WORK_INSIGHTS.md | 64 ++++++++++++++++++--------- tests/test_git_delivery.py | 37 +++++++++++++++- tests/test_work_insights.py | 28 ++++++++++-- token_meter/mcp/delivery.py | 2 +- token_meter/services/git_delivery.py | 34 +++++++++++++- token_meter/services/work_insights.py | 20 ++++----- 12 files changed, 149 insertions(+), 49 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f1913c0..0eb488a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -81,7 +81,7 @@ Token Meter reads local agent traces, calculates clearly labeled usage estimates - Keep `meter.py` and `token_meter_mcp.py` on the Python standard library. - Keep the dashboard local-only; do not add hosted assets, analytics, or telemetry. - Never output, commit, persist, or transmit prompts, responses, reasoning, tool contents, credentials, account data, or raw traces. -- Work insights are the only sanctioned consumer of typed user-turn text, the preceding assistant-reply tail (600 characters), per request, the basenames of files the agent changed, and, for Git work types, the subject and first body line (200 characters each) of the user's own pushed commits with a content-free count of the kinds of files each changed: they stay in the bounded in-memory queue, are sent only to the validated loopback Ollama URL, and only salted keys, labels, and content-free counts (costs, tool and command kinds, file categories) persist. Do not add other consumers. +- Work insights are the only sanctioned consumer of typed user-turn text, the preceding assistant-reply tail (600 characters), per request, the basenames of files the agent changed, and, for Git work types, the subject and first body line (200 characters each) of the user's own pushed commits with content-free counts of the kinds of files and lines each changed: they stay in the bounded in-memory queue, are sent only to the validated loopback Ollama URL, and only salted keys, labels, and content-free counts (costs, tool and command kinds, file categories) persist. Do not add other consumers. - Provider-account requests must remain narrow, bounded, sanitized, and read-only. - Cursor databases, transcripts, and request logs are read-only inputs. - Label estimates as estimates. Unavailable evidence must not become a measured zero. diff --git a/README.md b/README.md index f87b723..b63d389 100644 --- a/README.md +++ b/README.md @@ -258,7 +258,7 @@ skills, so the package targets Claude Code. - Costs are estimates. Codex uses public API rates, which can differ from subscription billing. - Work insights, when on, send typed prompts, the end of the reply before each one, changed file names, and your pushed commits' messages (subject and first body line) with - counts of the kinds of files they changed only to the local Ollama; + counts of the kinds of files and lines they changed only to the local Ollama; Token Meter stores labels and counts, never text. Full details: [User guide](specs/USER_GUIDE.md#data-and-evidence) · diff --git a/dashboard/js/git.js b/dashboard/js/git.js index 0cf1946..eefcd78 100644 --- a/dashboard/js/git.js +++ b/dashboard/js/git.js @@ -342,7 +342,7 @@ function deliveryTypeNote(work){ if(work.available!==true)return 'No pushed commits in this period.'; const labeled=Number(work.labeled_commits)||0,total=Number(work.commits)||0; if(!work.classifier_enabled&&labeled src/a.py\n' + '3\t1\tsrc/{old => tests}/test_b.py\n') + self.assertEqual(git_delivery.commit_details(output)[2], "Changed files: 2 tests, 1 code (+6 / -1 lines).") + + def test_common_trailers_are_not_taken_as_the_detail_line(self): + body = "Change-Id: I123\nReviewed-by: A \nCc: B \nNote: keep the old order.\n" + self.assertEqual(git_delivery.commit_details("S\x00" + body + "\x00")[1], "Note: keep the old order.") + def test_trailer_only_body_and_no_files_read_as_no_detail(self): subject, detail, files = git_delivery.commit_details("Bump version\x00Signed-off-by: A \n\x00\n") self.assertEqual((subject, detail, files), ("Bump version", "", "Changed files: none.")) long_subject, long_detail, _files = git_delivery.commit_details("x" * 500 + "\x00" + "y" * 500 + "\x00") self.assertEqual((len(long_subject), len(long_detail)), (200, 200)) + def test_a_commit_too_large_to_list_is_labeled_from_its_message_alone(self): + tip = "a" * 40 + calls = [] + base = _subject_runner({tip: "Vendor the parser"}, tracked=("refs/remotes/origin/main",), + pushes={"refs/remotes/origin/main": tip}, git_calls=calls) + + def runner(argv, **options): + if tuple(argv[argv.index("-C") + 2:])[0:2] == ("show", "--numstat") and "--format=%s%x00%b%x00" in argv: + return {"returncode": None, "stdout": ""} # timed out + return base(argv, **options) + + with tempfile.TemporaryDirectory() as tmp: + service = meter.GitDeliveryService(str(Path(tmp) / "delivery.sqlite3"), runner=runner, + now=lambda: 200, salt="test-salt") + service.commit_classifier = classifier = FakeCommitClassifier() + service.scan([{"root": "/repo", "project": "repo · a1"}]) + self.assertEqual([subject for *_keys, subject in classifier.subjects], ["Vendor the parser"]) + self.assertEqual(classifier.details, [("Explain the change.", "Changed files: none.")]) + self.assertIn("subject-only", calls) + def test_scan_hands_detail_and_file_kinds_but_never_paths_to_the_classifier(self): tip = "a" * 40 with tempfile.TemporaryDirectory() as tmp: diff --git a/tests/test_work_insights.py b/tests/test_work_insights.py index 62f4844..98dc3f7 100644 --- a/tests/test_work_insights.py +++ b/tests/test_work_insights.py @@ -3596,7 +3596,7 @@ class SubagentModelTrendTests(unittest.TestCase): def test_model_days_are_aggregated_projected_and_rendered(self): from token_meter.domain import agents from token_meter import projections - record = {"id": W.COMMIT_LABEL_VERSION, "kind": "spawned", "parent_id": "root", "runtime": "codex", "model": "gpt-5.6-terra", + record = {"id": "c1", "kind": "spawned", "parent_id": "root", "runtime": "codex", "model": "gpt-5.6-terra", "role": "tester", "activity_state": "complete", "cost": 2.0, "cost_available": True, "tokens": 10, "tokens_available": True, "last_activity_at": 1_790_000_000, "retries": 0, "failed_attempts": 0, "executions": 1, "depth": 1} @@ -3754,19 +3754,39 @@ def setUp(self): def test_conventional_prefix_is_the_label_with_no_model_call_or_queue_slot(self): for subject, expected in (("feat(git): add a chart", "feature"), ("fix!: stop a crash", "fix"), - ("chore(deps): bump x", "ops"), ("Docs: explain setup", "docs")): + ("chore(deps): bump x", "ops"), ("docs: explain setup", "docs")): self.assertTrue(self.service.observe_commit("repo", subject, subject)) self.assertEqual(self.stored[-1][2:6], (expected, 1.0, W.COMMIT_LABEL_VERSION, W.PREFIX_MODEL)) self.assertFalse(self.service.commit_queue) self.assertEqual(FakeClient.prompts, []) def test_unlisted_or_missing_prefixes_still_go_to_the_model(self): - for subject in ("revert: undo the chart", "Stop the chart from overflowing", "polish: tidy copy"): + for subject in ("revert: undo the chart", "Stop the chart from overflowing", "polish: tidy copy", + "Fix: the chart", "testing: add T.Context", "config: support X", "README: list"): self.assertIsNone(W.conventional_type(subject)) self.service.observe_commit("repo", subject, subject) - self.assertEqual(len(self.service.commit_queue), 3) + self.assertEqual(len(self.service.commit_queue), 7) self.assertEqual(self.stored, []) + def test_prefix_labels_respect_off_pause_clear_and_storage_errors(self): + self.settings["enabled"] = False + self.assertFalse(self.service.observe_commit("repo", "off", "fix: a")) + self.settings["enabled"] = True + with mock.patch.object(W, "is_paused", return_value=True): + self.assertFalse(self.service.observe_commit("repo", "paused", "fix: a")) + self.assertEqual(self.stored, []) + + with mock.patch.object(self.service, "write_lock") as lock: + # A delete lands while this write waits for the lock: the rotated generation fences it out. + lock.__enter__.side_effect = lambda: setattr(self.service, "generation", self.service.generation + 1) + self.assertTrue(self.service.observe_commit("repo", "fenced", "fix: a")) + self.assertEqual(self.stored, []) + + def broken(*args): + raise W.sqlite3.OperationalError("locked") + self.service.commit_label_sink = broken + self.assertTrue(self.service.observe_commit("repo", "broken", "fix: a")) # left for a later scan + def test_prompt_carries_subject_detail_and_file_kinds(self): FakeClient.responder = answer_for("fix") self.service.observe_commit("repo", "obj", "Stop the overflow", detail="Clamp the chart width.", diff --git a/token_meter/mcp/delivery.py b/token_meter/mcp/delivery.py index c2de7b4..d3fed61 100644 --- a/token_meter/mcp/delivery.py +++ b/token_meter/mcp/delivery.py @@ -215,7 +215,7 @@ def _git_work_types(value): return { "available": bool(value.get("available")), "estimate": True, - "method": "local_model_commit_subject", + "method": "conventional_prefix_or_local_model", "classifier_enabled": bool(value.get("classifier_enabled")), "commits": _count(value.get("commits")), "labeled_commits": _count(value.get("labeled_commits")), diff --git a/token_meter/services/git_delivery.py b/token_meter/services/git_delivery.py index 69e7eb3..5c8cf9c 100644 --- a/token_meter/services/git_delivery.py +++ b/token_meter/services/git_delivery.py @@ -37,7 +37,34 @@ # Plain words for each file kind in the classifier's changed-files line; paths never leave the scan. _FILE_KIND_WORDS = {"frontend": "UI", "backend": "code", "data": "data", "infra": "infrastructure", "tooling": "scripts", "docs": "docs", "tests": "tests", "config": "config", "media": "images"} -_TRAILER_RE = re.compile(r"^(co-authored-by|signed-off-by)\s*:", re.I) +# Git and Gerrit trailers (Co-authored-by, Reviewed-by, Change-Id, Cc, ...) name people or tracking ids, not +# the change; a hyphenated token before the colon, or one of the short ones, marks one. +_TRAILER_RE = re.compile(r"^([a-z]+(?:-[a-z]+)+|cc|fixes|closes|refs|bug|link):\s", re.I) +_QUOTED_PATH_RE = re.compile(r"\\([0-7]{3}|.)") + + +def _numstat_path(value): + """The destination path of one numstat entry: unquoted, with renames resolved to the new name.""" + path = value.strip() + if len(path) >= 2 and path[0] == path[-1] == '"': + escapes = {"n": 10, "t": 9, '"': 34, "\\": 92} + raw = bytearray() + index = 1 + while index < len(path) - 1: + match = _QUOTED_PATH_RE.match(path, index) + if match: + code = match.group(1) + raw.append(int(code, 8) if len(code) == 3 else escapes.get(code, ord(code[0]) % 256)) + index = match.end() + else: + raw.extend(path[index].encode("utf-8")) + index += 1 + path = raw.decode("utf-8", "replace") + if "{" in path and " => " in path: + path = re.sub(r"\{[^{}]* => ([^{}]*)\}", r"\1", path).replace("//", "/") + elif " => " in path: + path = path.split(" => ", 1)[1] + return path def commit_details(output): @@ -55,7 +82,7 @@ def commit_details(output): parts = line.split("\t", 2) if len(parts) != 3: continue - kinds[_FILE_KIND_WORDS.get(file_kind(parts[2]), "other")] += 1 + kinds[_FILE_KIND_WORDS.get(file_kind(_numstat_path(parts[2])), "other")] += 1 if parts[0].isdecimal() and parts[1].isdecimal(): added += int(parts[0]) deleted += int(parts[1]) @@ -908,6 +935,9 @@ def _send_subject(self, root, repo_key, object_key, oid, observed_at, wanted): return self._subjects_left -= 1 code, output = self._run_git(root, ("show", "--numstat", "--format=%s%x00%b%x00", oid)) + if code != 0: + # A very large commit can outlast the timeout listing its files: label it from its message alone. + code, output = self._run_git(root, ("show", "-s", "--format=%s%x00%b%x00", oid)) if code != 0: self._walk_incomplete = True # possibly transient; offered again by a later scan return diff --git a/token_meter/services/work_insights.py b/token_meter/services/work_insights.py index 5eea9e9..8940584 100644 --- a/token_meter/services/work_insights.py +++ b/token_meter/services/work_insights.py @@ -131,18 +131,14 @@ ("Cache parsed sessions so reloads are faster", "perf"), ("Explain install steps in the README", "docs"), ) -# A conventional-commit type names the author's own kind of work, so it is taken as the label with no model -# call. Types not listed here ("revert", "polish", "wip", ...) still go to the model. +# A Conventional Commits type names the author's own kind of work, so it is taken as the label with no model +# call. Only the spec's lowercase types count: "Fix: ..." or package-style prefixes such as "testing:" or +# "config:" (Go, Linux) go to the model, as do "revert:" and anything else not listed. CONVENTIONAL_TYPES = { - **dict.fromkeys(("feat", "feature", "features"), "feature"), - **dict.fromkeys(("fix", "fixes", "bugfix", "hotfix", "bug"), "fix"), - **dict.fromkeys(("refactor", "style", "cleanup", "lint"), "refactor"), - **dict.fromkeys(("perf", "performance"), "perf"), - **dict.fromkeys(("test", "tests", "testing"), "test"), - **dict.fromkeys(("docs", "doc", "documentation", "readme"), "docs"), - **dict.fromkeys(("chore", "ci", "build", "release", "deps", "dep", "ops", "infra", "config"), "ops"), + "feat": "feature", "fix": "fix", "docs": "docs", "style": "refactor", "refactor": "refactor", + "perf": "perf", "test": "test", "build": "ops", "ci": "ops", "chore": "ops", } -_CONVENTIONAL_RE = re.compile(r"^\s*([A-Za-z]+)(?:\([^)]{0,60}\))?!?:\s+\S") +_CONVENTIONAL_RE = re.compile(r"^([a-z]+)(?:\([^)]{0,60}\))?!?: \S") PREFIX_MODEL = "prefix" @@ -160,8 +156,8 @@ def commit_state(subject, detail="", files=""): def conventional_type(subject): """The commit type a conventional-commit subject declares ("fix(ui): ..." -> "fix"), or None.""" - match = _CONVENTIONAL_RE.match(str(subject or "")) - return CONVENTIONAL_TYPES.get(match.group(1).lower()) if match else None + match = _CONVENTIONAL_RE.match(str(subject or "").strip()) + return CONVENTIONAL_TYPES.get(match.group(1)) if match else None COMMIT_QUEUE_LIMIT = 256 From 5e44f7e55299877ba04a409cd1d054d16f3cf9de Mon Sep 17 00:00:00 2001 From: Pratik Bhavsar Date: Fri, 6 Nov 2026 12:54:39 +0530 Subject: [PATCH 3/6] fix(git): say files were not listed instead of none, and parse quoted renames When only a commit's message could be read, the model was told no files changed; it now hears the files were not listed. Quoted renames are split before unquoting so each side's kind is read correctly. Trailers come from an explicit allowlist, so detail lines such as "Follow-up:" or "Fixes:" are kept. Comments and docs state the lowercase-only prefix choice and the calibration reference precisely. Co-Authored-By: Claude Code --- specs/WORK_INSIGHTS.md | 24 +++++++++++++-------- tests/test_git_delivery.py | 9 +++++--- token_meter/services/git_delivery.py | 30 +++++++++++++++++---------- token_meter/services/work_insights.py | 6 ++++-- 4 files changed, 44 insertions(+), 25 deletions(-) diff --git a/specs/WORK_INSIGHTS.md b/specs/WORK_INSIGHTS.md index f46a8eb..113f627 100644 --- a/specs/WORK_INSIGHTS.md +++ b/specs/WORK_INSIGHTS.md @@ -237,16 +237,19 @@ fails or times out, it reads the message alone (`show -s`). A subject that starts with a lowercase Conventional Commits type (`feat:`, `fix(scope)!:`, `docs:`, `style:`, `refactor:`, `perf:`, `test:`, `build:`, `ci:`, `chore:`) is then labeled with that type at once: confidence 1, model `prefix`, no -model call and no queue slot. Other prefixes go to the model: capitalised ones -(`Fix:`), package-style ones (`testing:`, `config:` in Go or Linux history), and +model call and no queue slot. The spec itself is case-insensitive; capitalised +prefixes (`Fix:`) are left to the model because they are often plain sentences, +as are package-style ones (`testing:`, `config:` in Go or Linux history) and `revert:` or `wip:`. For the rest it keeps the subject and the first body line that is not a -trailer (`Co-authored-by`, `Reviewed-by`, `Change-Id`, `Cc`, and other hyphenated -or tracking trailers), whitespace-collapsed, 200 characters each. It reduces the +trailer (`Co-authored-by`, `Signed-off-by`, `Reviewed-by`, `Acked-by`, +`Tested-by`, `Change-Id`, `Cc`, and similar), whitespace-collapsed, 200 +characters each. It reduces the numstat paths, inside the scan, to a content-free line of file-kind counts and the commit's total added and deleted lines, such as `Changed files: 3 code, 1 tests (+120 / -30 lines).`, using the Work file kinds on each file's new name (quoted non-ASCII paths unquoted, renames resolved). Paths and -file names never leave the scan. These wait in a separate in-memory +file names never leave the scan. When only the message could be read, the line +says the files were not listed, never that none changed. These wait in a separate in-memory queue (256 commits) that alternates with session requests, so neither starves, and go through the same pause, pace, memory, CPU, battery, and local-model checks. @@ -261,16 +264,19 @@ scans up to three attempts. A version bump makes older labels read as not labele yet, so `c1` labels are redone once after the update; the chart shows those commits as not labeled yet until then. -Calibration. These figures cover only commits without a recognised prefix; -prefixed commits take their declared type and are not scored. The evaluation ran +Calibration. The figures estimate how the model does on commits that reach it, +the ones without a recognised prefix; prefixed commits take their declared type. +No naturally prefix-free commits had a reference label, so the test set is +prefixed commits with the prefix removed. The evaluation ran once on one machine with `token-meter-gemma` (Gemma 4 E4B), using `commit_state`'s exact three-line state and this question. Its harness reads private Git history, so it stayed in a scratch folder and was deleted with the data; only these aggregates were kept. - Data: 664 of one author's own non-merge commits, 180 days, three repositories. -- Reference: each commit's own Conventional Commits type, mapped as the shipped - table does but case-insensitively and with `readme:` as docs, with the prefix stripped from the subject before labeling. A stripped +- Reference: the type each commit's own prefix declares. The mapping was the ten + spec types above, matched case-insensitively, plus `readme:` as docs. The prefix + was removed from the subject before labeling. A stripped subject is a proxy for a free-form message. It is written by someone who uses prefixes, so it may read differently from naturally free-form messages. - Split: by commit hash into dev and held-out halves; prompt variants were chosen diff --git a/tests/test_git_delivery.py b/tests/test_git_delivery.py index 5806042..25dba64 100644 --- a/tests/test_git_delivery.py +++ b/tests/test_git_delivery.py @@ -3303,12 +3303,15 @@ def test_details_keep_the_first_body_line_and_reduce_paths_to_kind_counts(self): def test_quoted_and_renamed_paths_are_classified_by_their_new_name(self): output = ('S\x00\x00\n1\t0\t"caf\\303\\251/tests/test_x.py"\n2\t0\ttests/a.py => src/a.py\n' - '3\t1\tsrc/{old => tests}/test_b.py\n') - self.assertEqual(git_delivery.commit_details(output)[2], "Changed files: 2 tests, 1 code (+6 / -1 lines).") + '3\t1\tsrc/{old => tests}/test_b.py\n' + '4\t0\t"caf\\303\\251/a.py" => "tests/caf\\303\\251.py"\n') + self.assertEqual(git_delivery.commit_details(output)[2], "Changed files: 3 tests, 1 code (+10 / -1 lines).") def test_common_trailers_are_not_taken_as_the_detail_line(self): body = "Change-Id: I123\nReviewed-by: A \nCc: B \nNote: keep the old order.\n" self.assertEqual(git_delivery.commit_details("S\x00" + body + "\x00")[1], "Note: keep the old order.") + for line in ("Follow-up: handle empty input", "Fixes: the crash on load"): + self.assertEqual(git_delivery.commit_details(f"S\x00{line}\n\x00")[1], line) def test_trailer_only_body_and_no_files_read_as_no_detail(self): subject, detail, files = git_delivery.commit_details("Bump version\x00Signed-off-by: A \n\x00\n") @@ -3333,7 +3336,7 @@ def runner(argv, **options): service.commit_classifier = classifier = FakeCommitClassifier() service.scan([{"root": "/repo", "project": "repo · a1"}]) self.assertEqual([subject for *_keys, subject in classifier.subjects], ["Vendor the parser"]) - self.assertEqual(classifier.details, [("Explain the change.", "Changed files: none.")]) + self.assertEqual(classifier.details, [("Explain the change.", "Changed files: not listed (too large to read).")]) self.assertIn("subject-only", calls) def test_scan_hands_detail_and_file_kinds_but_never_paths_to_the_classifier(self): diff --git a/token_meter/services/git_delivery.py b/token_meter/services/git_delivery.py index 5c8cf9c..c886185 100644 --- a/token_meter/services/git_delivery.py +++ b/token_meter/services/git_delivery.py @@ -37,15 +37,18 @@ # Plain words for each file kind in the classifier's changed-files line; paths never leave the scan. _FILE_KIND_WORDS = {"frontend": "UI", "backend": "code", "data": "data", "infra": "infrastructure", "tooling": "scripts", "docs": "docs", "tests": "tests", "config": "config", "media": "images"} -# Git and Gerrit trailers (Co-authored-by, Reviewed-by, Change-Id, Cc, ...) name people or tracking ids, not -# the change; a hyphenated token before the colon, or one of the short ones, marks one. -_TRAILER_RE = re.compile(r"^([a-z]+(?:-[a-z]+)+|cc|fixes|closes|refs|bug|link):\s", re.I) +# Git and Gerrit trailers name people or tracking ids, not the change, so they are never the detail line. +_TRAILER_RE = re.compile( + r"^(co-authored-by|signed-off-by|reviewed-by|reviewed-on|acked-by|tested-by|reported-by|suggested-by" + r"|helped-by|requested-by|change-id|cc|bug|link):\s", re.I) _QUOTED_PATH_RE = re.compile(r"\\([0-7]{3}|.)") def _numstat_path(value): """The destination path of one numstat entry: unquoted, with renames resolved to the new name.""" path = value.strip() + if " => " in path and "{" not in path: + path = path.split(" => ", 1)[1].strip() # git quotes each side of a rename on its own if len(path) >= 2 and path[0] == path[-1] == '"': escapes = {"n": 10, "t": 9, '"': 34, "\\": 92} raw = bytearray() @@ -62,15 +65,14 @@ def _numstat_path(value): path = raw.decode("utf-8", "replace") if "{" in path and " => " in path: path = re.sub(r"\{[^{}]* => ([^{}]*)\}", r"\1", path).replace("//", "/") - elif " => " in path: - path = path.split(" => ", 1)[1] return path -def commit_details(output): +def commit_details(output, files_listed=True): """Split ``show --numstat --format=%s%x00%b%x00`` output into (subject, first body line, files line). - The files line is content-free: counts of changed file kinds and lines, never paths or names. + The files line is content-free: counts of changed file kinds and lines, never paths or names. When + the files could not be listed (``files_listed`` False), it says so rather than claiming none changed. """ subject, _sep, rest = str(output or "").partition("\0") body, _sep, numstat = rest.partition("\0") @@ -86,8 +88,13 @@ def commit_details(output): if parts[0].isdecimal() and parts[1].isdecimal(): added += int(parts[0]) deleted += int(parts[1]) - files = ("Changed files: " + ", ".join(f"{count} {kind}" for kind, count in kinds.most_common()) - + f" (+{added} / -{deleted} lines).") if kinds else "Changed files: none." + if not files_listed: + files = "Changed files: not listed (too large to read)." + elif kinds: + files = ("Changed files: " + ", ".join(f"{count} {kind}" for kind, count in kinds.most_common()) + + f" (+{added} / -{deleted} lines).") + else: + files = "Changed files: none." return (" ".join(subject.split())[:MAX_SUBJECT_CHARS], " ".join(detail.split())[:MAX_DETAIL_CHARS], files) # Bumped when the commit-type question changes; labels from another version read as not labeled yet. COMMIT_LABEL_VERSION = "c2" @@ -935,13 +942,14 @@ def _send_subject(self, root, repo_key, object_key, oid, observed_at, wanted): return self._subjects_left -= 1 code, output = self._run_git(root, ("show", "--numstat", "--format=%s%x00%b%x00", oid)) - if code != 0: + listed = code == 0 + if not listed: # A very large commit can outlast the timeout listing its files: label it from its message alone. code, output = self._run_git(root, ("show", "-s", "--format=%s%x00%b%x00", oid)) if code != 0: self._walk_incomplete = True # possibly transient; offered again by a later scan return - subject, detail, files = commit_details(output) + subject, detail, files = commit_details(output, files_listed=listed) if not subject: self._skip_subject(repo_key, object_key) return diff --git a/token_meter/services/work_insights.py b/token_meter/services/work_insights.py index 8940584..e406714 100644 --- a/token_meter/services/work_insights.py +++ b/token_meter/services/work_insights.py @@ -132,8 +132,10 @@ ("Explain install steps in the README", "docs"), ) # A Conventional Commits type names the author's own kind of work, so it is taken as the label with no model -# call. Only the spec's lowercase types count: "Fix: ..." or package-style prefixes such as "testing:" or -# "config:" (Go, Linux) go to the model, as do "revert:" and anything else not listed. +# call. The spec treats types case-insensitively, but only lowercase spec types are taken here: capitalised +# prefixes ("Fix: the chart") are often plain sentences, and package-style prefixes ("testing:", "config:" +# in Go or Linux history) name an area, not a kind of work. Those, "revert:", and anything unlisted go to +# the model. CONVENTIONAL_TYPES = { "feat": "feature", "fix": "fix", "docs": "docs", "style": "refactor", "refactor": "refactor", "perf": "perf", "test": "test", "build": "ops", "ci": "ops", "chore": "ops", From f876ec4f5eba13946437763e75f97dcd378143f6 Mon Sep 17 00:00:00 2001 From: Pratik Bhavsar Date: Fri, 6 Nov 2026 12:59:53 +0530 Subject: [PATCH 4/6] fix(git): resolve brace renames first, widen trailers, plain fallback note - Renames resolve braces first, then split each quoted side, so a name containing "{" still classifies by its new path; the test now uses old and new names of different kinds so it fails without the split. - Any "...-by:" key and Fixes/Closes/Resolves/Refs followed by an id are trailers; "Fixes: the crash" stays a detail line. - The message-only fallback says "Changed files: not listed." without guessing a cause. Co-Authored-By: Claude Code --- specs/WORK_INSIGHTS.md | 6 +++--- tests/test_git_delivery.py | 10 +++++++--- token_meter/services/git_delivery.py | 15 +++++++++------ 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/specs/WORK_INSIGHTS.md b/specs/WORK_INSIGHTS.md index 113f627..4fc5254 100644 --- a/specs/WORK_INSIGHTS.md +++ b/specs/WORK_INSIGHTS.md @@ -241,9 +241,9 @@ model call and no queue slot. The spec itself is case-insensitive; capitalised prefixes (`Fix:`) are left to the model because they are often plain sentences, as are package-style ones (`testing:`, `config:` in Go or Linux history) and `revert:` or `wip:`. For the rest it keeps the subject and the first body line that is not a -trailer (`Co-authored-by`, `Signed-off-by`, `Reviewed-by`, `Acked-by`, -`Tested-by`, `Change-Id`, `Cc`, and similar), whitespace-collapsed, 200 -characters each. It reduces the +trailer (any `…-by:` key such as `Co-authored-by` or `Reviewed-by`, `Change-Id`, +`Cc`, and `Fixes:`/`Closes:`/`Refs:` followed by an issue number or commit id), +whitespace-collapsed, 200 characters each. It reduces the numstat paths, inside the scan, to a content-free line of file-kind counts and the commit's total added and deleted lines, such as `Changed files: 3 code, 1 tests (+120 / -30 lines).`, using the Work file kinds on diff --git a/tests/test_git_delivery.py b/tests/test_git_delivery.py index 25dba64..59491db 100644 --- a/tests/test_git_delivery.py +++ b/tests/test_git_delivery.py @@ -3304,14 +3304,18 @@ def test_details_keep_the_first_body_line_and_reduce_paths_to_kind_counts(self): def test_quoted_and_renamed_paths_are_classified_by_their_new_name(self): output = ('S\x00\x00\n1\t0\t"caf\\303\\251/tests/test_x.py"\n2\t0\ttests/a.py => src/a.py\n' '3\t1\tsrc/{old => tests}/test_b.py\n' - '4\t0\t"caf\\303\\251/a.py" => "tests/caf\\303\\251.py"\n') - self.assertEqual(git_delivery.commit_details(output)[2], "Changed files: 3 tests, 1 code (+10 / -1 lines).") + '4\t0\t"tests/caf\\303\\251.py" => "src/caf\\303\\251.py"\n' + '5\t0\tsrc/a{1}.py => tests/test_c.py\n') + # Old names look like tests where the new names are code, and the reverse: only new names count. + self.assertEqual(git_delivery.commit_details(output)[2], "Changed files: 3 tests, 2 code (+15 / -1 lines).") def test_common_trailers_are_not_taken_as_the_detail_line(self): body = "Change-Id: I123\nReviewed-by: A \nCc: B \nNote: keep the old order.\n" self.assertEqual(git_delivery.commit_details("S\x00" + body + "\x00")[1], "Note: keep the old order.") for line in ("Follow-up: handle empty input", "Fixes: the crash on load"): self.assertEqual(git_delivery.commit_details(f"S\x00{line}\n\x00")[1], line) + trailers = "Co-developed-by: A \nFixes: #123\nRefs: 1a2b3c4d\nAssisted-by: tool\nKeep it short.\n" + self.assertEqual(git_delivery.commit_details("S\x00" + trailers + "\x00")[1], "Keep it short.") def test_trailer_only_body_and_no_files_read_as_no_detail(self): subject, detail, files = git_delivery.commit_details("Bump version\x00Signed-off-by: A \n\x00\n") @@ -3336,7 +3340,7 @@ def runner(argv, **options): service.commit_classifier = classifier = FakeCommitClassifier() service.scan([{"root": "/repo", "project": "repo · a1"}]) self.assertEqual([subject for *_keys, subject in classifier.subjects], ["Vendor the parser"]) - self.assertEqual(classifier.details, [("Explain the change.", "Changed files: not listed (too large to read).")]) + self.assertEqual(classifier.details, [("Explain the change.", "Changed files: not listed.")]) self.assertIn("subject-only", calls) def test_scan_hands_detail_and_file_kinds_but_never_paths_to_the_classifier(self): diff --git a/token_meter/services/git_delivery.py b/token_meter/services/git_delivery.py index c886185..42cf30e 100644 --- a/token_meter/services/git_delivery.py +++ b/token_meter/services/git_delivery.py @@ -38,16 +38,21 @@ _FILE_KIND_WORDS = {"frontend": "UI", "backend": "code", "data": "data", "infra": "infrastructure", "tooling": "scripts", "docs": "docs", "tests": "tests", "config": "config", "media": "images"} # Git and Gerrit trailers name people or tracking ids, not the change, so they are never the detail line. +# Any "...-by:" key (Co-authored-by, Co-developed-by, Reported-and-tested-by), plus Gerrit and tracker keys. +# Tracker keys count only when an id follows ("Fixes: #12", "Fixes: 1a2b3c4 (...)"), so "Fixes: the crash" +# stays a detail line. _TRAILER_RE = re.compile( - r"^(co-authored-by|signed-off-by|reviewed-by|reviewed-on|acked-by|tested-by|reported-by|suggested-by" - r"|helped-by|requested-by|change-id|cc|bug|link):\s", re.I) + r"^([a-z]+(?:-[a-z]+)*-by|reviewed-on|change-id|cc|bug|link):\s" + r"|^(fixes|closes|resolves|refs):\s+(#?\d+\b|[0-9a-f]{7,40}\b)", re.I) _QUOTED_PATH_RE = re.compile(r"\\([0-7]{3}|.)") def _numstat_path(value): """The destination path of one numstat entry: unquoted, with renames resolved to the new name.""" path = value.strip() - if " => " in path and "{" not in path: + if "{" in path and " => " in path: + path = re.sub(r"\{[^{}]* => ([^{}]*)\}", r"\1", path).replace("//", "/") + if " => " in path: path = path.split(" => ", 1)[1].strip() # git quotes each side of a rename on its own if len(path) >= 2 and path[0] == path[-1] == '"': escapes = {"n": 10, "t": 9, '"': 34, "\\": 92} @@ -63,8 +68,6 @@ def _numstat_path(value): raw.extend(path[index].encode("utf-8")) index += 1 path = raw.decode("utf-8", "replace") - if "{" in path and " => " in path: - path = re.sub(r"\{[^{}]* => ([^{}]*)\}", r"\1", path).replace("//", "/") return path @@ -89,7 +92,7 @@ def commit_details(output, files_listed=True): added += int(parts[0]) deleted += int(parts[1]) if not files_listed: - files = "Changed files: not listed (too large to read)." + files = "Changed files: not listed." elif kinds: files = ("Changed files: " + ", ".join(f"{count} {kind}" for kind, count in kinds.most_common()) + f" (+{added} / -{deleted} lines).") From e6f288d915b96aa5df483f341bb1bad6779964cf Mon Sep 17 00:00:00 2001 From: Pratik Bhavsar Date: Fri, 6 Nov 2026 13:06:38 +0530 Subject: [PATCH 5/6] test(git): move commit work-type tests to their own module tests/test_git_delivery.py had grown past its size budget, so the commit work-type tests move to tests/test_git_commit_types.py unchanged (same 122 tests across the two modules). The rename fixture now has an old name that looks like tests where the new name is code, for both the plain and the brace form, so dropping either rename step fails the test. Co-Authored-By: Claude Code --- tests/test_git_commit_types.py | 513 +++++++++++++++++++++++++++++++++ tests/test_git_delivery.py | 490 ------------------------------- 2 files changed, 513 insertions(+), 490 deletions(-) create mode 100644 tests/test_git_commit_types.py diff --git a/tests/test_git_commit_types.py b/tests/test_git_commit_types.py new file mode 100644 index 0000000..bf0b640 --- /dev/null +++ b/tests/test_git_commit_types.py @@ -0,0 +1,513 @@ +"""Git commit work types: subject and detail reads, prefix-free labeling evidence, and the type chart.""" + +import datetime +import json +import re +import shutil +import sqlite3 +import subprocess +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +import meter +from token_meter.services import git_delivery +from tests.dashboard_page import DASHBOARD_PAGE +from tests.test_git_delivery import _ref_runner, delivery_js, local_timestamp + + +class FakeCommitClassifier: + def __init__(self, version=git_delivery.COMMIT_LABEL_VERSION, since_day="0000-00-00", room=1000): + self.version, self.since_day, self.room = version, since_day, room + self.subjects = [] + self.details = [] + + def commit_label_request(self): + return {"version": self.version, "since_day": self.since_day} + + def commit_status(self, repo_key, object_key): + if any((repo, obj) == (repo_key, object_key) for repo, obj, _subject in self.subjects): + return "queued" + return "take" if len(self.subjects) < self.room else "later" + + def observe_commit(self, repo_key, object_key, subject, observed_at, detail="", files=""): + self.details.append((detail, files)) + if len(self.subjects) >= self.room: + return False + self.subjects.append((repo_key, object_key, subject)) + return True + + +def _subject_runner(subjects, **kwargs): + base = _ref_runner(**kwargs) + reads = kwargs.get("git_calls") + + def runner(argv, **options): + args = tuple(argv[argv.index("-C") + 2:]) + if args[0:3] == ("show", "-s", "--format=%s%x00%b%x00"): + if reads is not None: + reads.append("subject-only") + return {"returncode": 0, "stdout": subjects.get(args[-1], "Add a chart") + "\x00Explain the change.\n\x00\n"} + if args[0:3] == ("show", "--numstat", "--format=%s%x00%b%x00"): + if reads is not None: + reads.append("subject") + return {"returncode": 0, "stdout": subjects.get(args[-1], "Add a chart") + + "\x00Explain the change.\n\x00\n2\t1\tsrc/app.py\n"} + return base(argv, **options) + + return runner + + +class GitCommitWorkTypeTests(unittest.TestCase): + def ledger(self, tmp): + return meter.GitDeliveryLedger(str(Path(tmp) / "delivery.sqlite3"), "test-salt") + + def test_labels_persist_only_type_confidence_version_and_model(self): + with tempfile.TemporaryDirectory() as tmp: + ledger = self.ledger(tmp) + ledger.record("repo", "obj", local_timestamp("2026-09-03"), 10, 2) + self.assertTrue(ledger.record_commit_label("repo", "obj", "fix", 0.91, git_delivery.COMMIT_LABEL_VERSION, "digest", 5)) + self.assertFalse(ledger.record_commit_label("repo", "missing", "fix", 0.9, git_delivery.COMMIT_LABEL_VERSION, "digest", 5)) + with self.assertRaises(ValueError): + ledger.record_commit_label("repo", "obj", "chores", 0.9, git_delivery.COMMIT_LABEL_VERSION, "digest", 5) + with sqlite3.connect(Path(tmp) / "delivery.sqlite3") as connection: + columns = {row[1] for row in connection.execute("PRAGMA table_info(delivery_commit_labels)")} + self.assertEqual(columns, {"repo_key", "object_key", "work_type", "confidence", "version", + "model", "labeled_at"}) + + def test_daily_type_rows_split_by_label_and_treat_other_versions_as_unlabeled(self): + with tempfile.TemporaryDirectory() as tmp: + ledger = self.ledger(tmp) + day = local_timestamp("2026-09-03") + for name, lines in (("a", 10), ("b", 6), ("c", 4), ("d", 1)): + ledger.record("repo", name, day, lines, 0) + ledger.record_commit_label("repo", "a", "feature", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) + ledger.record_commit_label("repo", "b", "feature", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) + ledger.record_commit_label("repo", "c", "fix", 0.9, "c0", "m", 1) # stale question version + + rows = ledger.daily_type_rows(["repo"], "2026-09-03", "2026-09-03", git_delivery.COMMIT_LABEL_VERSION) + self.assertEqual(sorted((row["work_type"], row["commits"], row["lines"]) for row in rows), + [("", 2, 5), ("feature", 2, 16)]) + self.assertEqual(ledger.unlabeled_keys("repo", git_delivery.COMMIT_LABEL_VERSION, "2026-09-01", 10), {"c", "d"}) + ledger.clear(10) + self.assertEqual(ledger.daily_type_rows(["repo"], "2026-09-03", "2026-09-03", git_delivery.COMMIT_LABEL_VERSION), []) + + def test_scan_hands_new_commit_subjects_to_the_classifier_and_never_stores_them(self): + tip = "a" * 40 + with tempfile.TemporaryDirectory() as tmp: + service = meter.GitDeliveryService( + str(Path(tmp) / "delivery.sqlite3"), + runner=_subject_runner({tip: "Fix the secret-sauce overflow"}, + tracked=("refs/remotes/origin/main",), + pushes={"refs/remotes/origin/main": tip}), + now=lambda: 200, salt="test-salt", + ) + classifier = FakeCommitClassifier() + service.commit_classifier = classifier + service.scan([{"root": "/repo", "project": "repo · a1"}]) + + self.assertEqual([subject for _repo, _obj, subject in classifier.subjects], + ["Fix the secret-sauce overflow"]) + repo_key, object_key, _subject = classifier.subjects[0] + self.assertTrue(service.ledger.record_commit_label(repo_key, object_key, "fix", 0.8, git_delivery.COMMIT_LABEL_VERSION, "m", 1)) + stored = b"".join(path.read_bytes() for path in Path(tmp).iterdir()) + self.assertNotIn(b"secret-sauce", stored) + + def test_scan_reads_no_subjects_without_an_enabled_classifier(self): + calls = [] + with tempfile.TemporaryDirectory() as tmp: + service = meter.GitDeliveryService( + str(Path(tmp) / "delivery.sqlite3"), + runner=_subject_runner({}, tracked=("refs/remotes/origin/main",), + pushes={"refs/remotes/origin/main": "a" * 40}, git_calls=calls), + now=lambda: 200, salt="test-salt", + ) + service.scan([{"root": "/repo", "project": "repo · a1"}]) + classifier = FakeCommitClassifier() + classifier.commit_label_request = lambda: None + service.commit_classifier = classifier + service.scan([{"root": "/repo", "project": "repo · a1"}]) + self.assertNotIn("subject", calls) + + def test_scan_backfills_subjects_of_already_counted_unlabeled_commits(self): + tip = "a" * 40 + with tempfile.TemporaryDirectory() as tmp: + runner = _subject_runner({tip: "Add the weekly chart"}, tracked=("refs/remotes/origin/main",), + pushes={"refs/remotes/origin/main": tip}) + service = meter.GitDeliveryService(str(Path(tmp) / "delivery.sqlite3"), runner=runner, + now=lambda: 200, salt="test-salt") + service.scan([{"root": "/repo", "project": "repo · a1"}]) # counted before labeling existed + classifier = FakeCommitClassifier() + service.commit_classifier = classifier + service.scan([{"root": "/repo", "project": "repo · a1"}]) + self.assertEqual([subject for *_keys, subject in classifier.subjects], ["Add the weekly chart"]) + repo_key, object_key, _subject = classifier.subjects[0] + service.ledger.record_commit_label(repo_key, object_key, "feature", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) + classifier.subjects.clear() + service.scan([{"root": "/repo", "project": "repo · a1"}]) + self.assertEqual(classifier.subjects, []) # labeled commits are not offered again + + def test_commits_no_push_introduces_are_not_walked_for_again(self): + tip = "a" * 40 + calls = [] + with tempfile.TemporaryDirectory() as tmp: + runner = _subject_runner({}, tracked=("refs/remotes/origin/main",), + pushes={"refs/remotes/origin/main": tip}, git_calls=calls) + service = meter.GitDeliveryService(str(Path(tmp) / "delivery.sqlite3"), runner=runner, + now=lambda: 200, salt="test-salt") + service.scan([{"root": "/repo", "project": "repo · a1"}]) + repo_key = service.ledger.rows()[0]["repo_key"] + # Counted from a branch whose reflog is gone (deleted after merge): no push introduces it now. + service.ledger.record(repo_key, "orphan", 150, 3, 0) + classifier = FakeCommitClassifier() + service.commit_classifier = classifier + service.scan([{"root": "/repo", "project": "repo · a1"}]) # offers the tip; the orphan is missed + for repo, obj, _subject in classifier.subjects: + service.ledger.record_commit_label(repo, obj, "feature", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) + calls.clear() + service.scan([{"root": "/repo", "project": "repo · a1"}]) + service.scan([{"root": "/repo", "project": "repo · a1"}]) + self.assertNotIn("subject", calls) + self.assertNotIn("rev-list", calls) + service.forget_commit_subjects() + service.scan([{"root": "/repo", "project": "repo · a1"}]) + self.assertIn("rev-list", calls) # a label delete offers it again + + def service_with(self, tmp, calls, commits=None, failing=()): + tip = "a" * 40 + runner = _subject_runner({}, tracked=("refs/remotes/origin/main",), + pushes={"refs/remotes/origin/main": tip}, git_calls=calls, + commits=commits, failing=failing) + return meter.GitDeliveryService(str(Path(tmp) / "delivery.sqlite3"), runner=runner, + now=lambda: 200, salt="test-salt") + + def test_subject_is_not_read_for_a_commit_whose_attempts_are_used_up(self): + calls = [] + with tempfile.TemporaryDirectory() as tmp: + service = self.service_with(tmp, calls) + classifier = FakeCommitClassifier() + classifier.commit_status = lambda repo_key, object_key: "never" + service.commit_classifier = classifier + service.scan([{"root": "/repo", "project": "repo · a1"}]) + service.scan([{"root": "/repo", "project": "repo · a1"}]) + self.assertNotIn("subject", calls) + self.assertEqual(classifier.subjects, []) + + def test_commits_that_did_not_fit_a_full_queue_are_offered_once_it_drains(self): + calls = [] + tip = "a" * 40 + chain = ("c" * 40, "d" * 40, tip) + with tempfile.TemporaryDirectory() as tmp: + service = self.service_with(tmp, calls, commits={tip: chain}) + classifier = FakeCommitClassifier(room=1) + service.commit_classifier = classifier + service.scan([{"root": "/repo", "project": "repo · a1"}]) + self.assertEqual(len(classifier.subjects), 1) + classifier.room = 1000 # the worker drained the queue + service.scan([{"root": "/repo", "project": "repo · a1"}]) + self.assertEqual(len({obj for _repo, obj, _subject in classifier.subjects}), 3) + + def test_a_still_queued_commit_is_not_skipped_and_is_offered_again_after_it_fails(self): + calls = [] + with tempfile.TemporaryDirectory() as tmp: + service = self.service_with(tmp, calls) + classifier = FakeCommitClassifier() + service.commit_classifier = classifier + service.scan([{"root": "/repo", "project": "repo · a1"}]) + service.scan([{"root": "/repo", "project": "repo · a1"}]) # still queued: left alone + self.assertEqual(len(classifier.subjects), 1) + classifier.subjects.clear() # the model call failed and the item was dropped + service.scan([{"root": "/repo", "project": "repo · a1"}]) + self.assertEqual(len(classifier.subjects), 1) + + def test_a_walk_truncated_by_a_size_cap_still_lets_unfound_commits_be_skipped(self): + calls = [] + tip = "a" * 40 + with tempfile.TemporaryDirectory() as tmp, \ + mock.patch.object(git_delivery, "MAX_COMMITS_PER_PUSH", 2): + service = self.service_with(tmp, calls, commits={tip: ("c" * 40, "d" * 40, tip)}) + service.scan([{"root": "/repo", "project": "repo · a1"}]) + repo_key = service.ledger.rows()[0]["repo_key"] + service.ledger.record(repo_key, "orphan", 150, 3, 0) # its branch was deleted after merge + service.commit_classifier = classifier = FakeCommitClassifier() + service.scan([{"root": "/repo", "project": "repo · a1"}]) + self.assertIn("orphan", service._subject_skips.get(repo_key, set())) + + def test_a_failed_backfill_walk_does_not_skip_and_is_retried(self): + tip = "a" * 40 + fail_walks = [False] + base = _subject_runner({}, tracked=("refs/remotes/origin/main",), + pushes={"refs/remotes/origin/main": tip}) + + def runner(argv, **options): + if tuple(argv[argv.index("-C") + 2:])[0] == "rev-list" and fail_walks[0]: + return {"returncode": 128, "stdout": ""} + return base(argv, **options) + + with tempfile.TemporaryDirectory() as tmp: + service = meter.GitDeliveryService(str(Path(tmp) / "delivery.sqlite3"), runner=runner, + now=lambda: 200, salt="test-salt") + service.scan([{"root": "/repo", "project": "repo · a1"}]) # counted before labeling existed + service.commit_classifier = classifier = FakeCommitClassifier() + fail_walks[0] = True + service.scan([{"root": "/repo", "project": "repo · a1"}]) + self.assertEqual(classifier.subjects, []) + self.assertEqual(service._subject_skips, {}) + fail_walks[0] = False + service.scan([{"root": "/repo", "project": "repo · a1"}]) + self.assertEqual(len(classifier.subjects), 1) + + def test_a_failed_subject_read_is_retried_by_a_later_scan(self): + tip = "a" * 40 + failures = [True, True] # the full read and its message-only fallback + base = _subject_runner({}, tracked=("refs/remotes/origin/main",), + pushes={"refs/remotes/origin/main": tip}) + + def runner(argv, **options): + if tuple(argv[argv.index("-C") + 2:])[0] == "show" and "--format=%s%x00%b%x00" in argv and failures: + failures.pop() + return {"returncode": 128, "stdout": ""} + return base(argv, **options) + + with tempfile.TemporaryDirectory() as tmp: + service = meter.GitDeliveryService(str(Path(tmp) / "delivery.sqlite3"), runner=runner, + now=lambda: 200, salt="test-salt") + service.commit_classifier = classifier = FakeCommitClassifier() + service.scan([{"root": "/repo", "project": "repo · a1"}]) + self.assertEqual(classifier.subjects, []) + self.assertEqual(service._subject_skips, {}) + service.scan([{"root": "/repo", "project": "repo · a1"}]) + self.assertEqual(len(classifier.subjects), 1) + + def test_a_transient_reflog_failure_does_not_skip_unfound_commits(self): + calls = [] + with tempfile.TemporaryDirectory() as tmp: + service = self.service_with(tmp, calls) + service.scan([{"root": "/repo", "project": "repo · a1"}]) # counted before labeling existed + failing = self.service_with(tmp, calls, failing=("refs/remotes/origin/main",)) + failing.commit_classifier = classifier = FakeCommitClassifier() + failing.scan([{"root": "/repo", "project": "repo · a1"}]) + self.assertEqual(classifier.subjects, []) + failing._runner = service._runner # the reflog reads again + failing.scan([{"root": "/repo", "project": "repo · a1"}]) + self.assertEqual(len(classifier.subjects), 1) + + def test_query_reports_daily_work_types_with_unlabeled_commits_explicit(self): + tip = "a" * 40 + today = datetime.date.fromtimestamp(local_timestamp("2026-09-04")).isoformat() + with tempfile.TemporaryDirectory() as tmp: + service = meter.GitDeliveryService( + str(Path(tmp) / "delivery.sqlite3"), + runner=_subject_runner({}, tracked=("refs/remotes/origin/main",), + pushes={"refs/remotes/origin/main": tip}), + now=lambda: local_timestamp("2026-09-04"), salt="test-salt", + ) + service.scan([{"root": "/repo", "project": "repo · a1"}]) + repo_key = service.ledger.rows()[0]["repo_key"] + service.ledger.record("repo-other", "x", local_timestamp("2026-09-04"), 1, 1) + # The scanned push is dated by the fake reflog long ago; these two land in the window. + service.ledger.record(repo_key, "first", local_timestamp("2026-09-04"), 2, 1) + service.ledger.record(repo_key, "second", local_timestamp("2026-09-04"), 7, 0) + service.ledger.record_commit_label(repo_key, "second", "docs", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) + payload = service.query("", "7", [], ["repo · a1"], [{"root": "/repo", "project": "repo · a1"}]) + + work = payload["work_types"] + self.assertEqual(work["commits"], 2) + self.assertEqual(work["labeled_commits"], 1) + self.assertFalse(work["classifier_enabled"]) + self.assertTrue(work["estimate"]) + self.assertEqual(work["totals"], {"unlabeled": {"commits": 1, "lines": 3}, + "docs": {"commits": 1, "lines": 7}}) + self.assertEqual(len(work["days"]), 7) + self.assertEqual(work["days"][-1]["day"], today) + self.assertEqual(work["days"][-1]["types"]["docs"], {"commits": 1, "lines": 7}) + + +class GitWorkTypeChartContractTests(unittest.TestCase): + page = Path(DASHBOARD_PAGE).read_text(encoding="utf-8") + + def test_work_type_card_sits_after_the_daily_chart_and_resizes_with_it(self): + git_page = self.page.split("id=view-git", 1)[1].split("id=view-learn", 1)[0] + self.assertLess(git_page.index("id=d-daily-chart"), git_page.index("id=d-type-chart")) + self.assertLess(git_page.index("id=d-type-chart"), git_page.index("id=d-insight-title")) + self.assertIn(">Work pushed by type<", git_page) + self.assertIn("data-delivery-type-metric=commits", git_page) + self.assertIn("data-delivery-type-metric=lines", git_page) + self.assertIn("git:['d-daily-chart','d-type-chart']", self.page) + self.assertIn("drawDeliveryTypeChart(payload?.work_types)", self.page) + colors = re.search(r"^const DELIVERY_TYPE_COLORS=\{[^\n]*\};$", self.page, re.MULTILINE).group(0) + self.assertNotRegex(colors, r"#[0-9a-fA-F]{3,8}|rgba?\(") + + @unittest.skipUnless(shutil.which("node"), "Node.js is required for dashboard JavaScript") + def test_chart_stacks_each_day_and_keeps_unlabeled_commits_explicit(self): + preamble = """ +const nodes={}; +const node=()=>({innerHTML:'',textContent:'',hidden:false,clientWidth:1040,clientHeight:270,style:{},attrs:{}, + setAttribute(name,value){this.attrs[name]=value;},getBoundingClientRect(){return{left:0,width:1040};}}); +['d-type-chart','d-type-tip','d-type-legend','d-type-note','d-type-hover'].forEach(id=>nodes[id]=node()); +const $=id=>nodes[id]; +const document={querySelectorAll:()=>[]}; +const esc=value=>String(value); +const deliveryPercent=value=>`${Math.round(value)}%`; +const deliveryDateLabel=day=>day; +let deliveryTypeMetric='commits'; +""" + driver = """ +const work={available:true,classifier_enabled:false,commits:4,labeled_commits:3, + totals:{fix:{commits:2,lines:30},feature:{commits:1,lines:5},unlabeled:{commits:1,lines:9}}, + days:[{day:'Sep 3',types:{fix:{commits:2,lines:30},unlabeled:{commits:1,lines:9}}},{day:'Sep 4',types:{feature:{commits:1,lines:5}}}]}; +drawDeliveryTypeChart(work); +const svg=nodes['d-type-chart'].innerHTML; +const first={segments:(svg.match(/class="typeSeg/g)||[]).length,unlabeled:(svg.match(/typeSeg unlabeled/g)||[]).length, + legend:nodes['d-type-legend'].innerHTML,note:nodes['d-type-note'].textContent}; +deliveryTypeMetric='lines';drawDeliveryTypeChart(work); +const lines=nodes['d-type-chart'].attrs['aria-label']; +const svgNode=nodes['d-type-chart'],keys=[]; +svgNode.onfocus();keys.push(nodes['d-type-tip'].innerHTML); +svgNode.onkeydown({key:'ArrowLeft',preventDefault(){}});keys.push(nodes['d-type-tip'].innerHTML); +svgNode.onkeydown({key:'End',preventDefault(){}});keys.push(nodes['d-type-tip'].innerHTML); +const hatched=nodes['d-type-chart'].innerHTML.includes('fill:url(#d-type-hatch)')&&nodes['d-type-chart'].innerHTML.includes('50%", result["first"]["legend"]) + self.assertIn("Not labeled yet 25%", result["first"]["legend"]) + self.assertEqual(result["first"]["note"], + "3 of 4 commits labeled · turn on Work insights in Settings to label the rest locally") + self.assertEqual(result["lines"], "Pushed lines per day by type: Features 5, Fixes 30, Not labeled yet 9") + self.assertIn("Sep 4", result["keys"][0]) # focus opens on the latest day + self.assertIn("Sep 3", result["keys"][1]) # arrows move between days + self.assertIn("Sep 4", result["keys"][2]) + self.assertTrue(result["hatched"]) + self.assertEqual(result["axes"], [2, 4, 6, 10, 56, 1600]) # even tops: every gridline is whole + self.assertEqual(result["loading"], "Reading labels\u2026") + self.assertIn("No pushed commits in this period", result["empty"]) + self.assertEqual(result["emptyNote"], "No pushed commits in this period.") + + +class CommitWorkLabelClearTests(unittest.TestCase): + def test_commit_labels_are_deleted_even_when_the_work_ledger_delete_fails(self): + with tempfile.TemporaryDirectory() as tmp: + service = meter.GitDeliveryService(str(Path(tmp) / "delivery.sqlite3"), runner=_ref_runner(), + now=lambda: 200, salt="test-salt") + service.ledger.record("repo", "obj", local_timestamp("2026-09-03"), 3, 1) + service.ledger.record_commit_label("repo", "obj", "docs", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) + work = mock.Mock() + work.clear.side_effect = meter._work.LabelDeleteError("busy") + with mock.patch.object(meter, "git_delivery_service", return_value=service), \ + mock.patch.object(meter, "work_insights_service_if_started", return_value=work): + with self.assertRaises(meter._work.LabelDeleteError): + meter.clear_work_insights() + rows = service.ledger.daily_type_rows(["repo"], "2026-09-03", "2026-09-03", git_delivery.COMMIT_LABEL_VERSION) + self.assertEqual([(row["work_type"], row["commits"]) for row in rows], [("", 1)]) + + def test_a_git_ledger_failure_is_reported_not_swallowed(self): + service = mock.Mock() + service.ledger.clear_commit_labels.side_effect = sqlite3.OperationalError("locked") + with mock.patch.object(meter, "git_delivery_service", return_value=service), \ + mock.patch.object(meter, "work_insights_service_if_started", return_value=mock.Mock()): + with self.assertRaises(sqlite3.Error): + meter.clear_work_insights() + + def test_deleting_work_labels_also_deletes_commit_work_type_labels(self): + with tempfile.TemporaryDirectory() as tmp: + service = meter.GitDeliveryService(str(Path(tmp) / "delivery.sqlite3"), runner=_ref_runner(), + now=lambda: 200, salt="test-salt") + service.ledger.record("repo", "obj", local_timestamp("2026-09-03"), 3, 1) + service.ledger.record_commit_label("repo", "obj", "docs", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) + with mock.patch.object(meter, "git_delivery_service", return_value=service), \ + mock.patch.object(meter, "work_insights_service_if_started", return_value=None), \ + mock.patch.object(meter, "TOKEN_METER_WORK_INSIGHTS_DB", str(Path(tmp) / "work.sqlite3")): + meter.clear_work_insights() + rows = service.ledger.daily_type_rows(["repo"], "2026-09-03", "2026-09-03", git_delivery.COMMIT_LABEL_VERSION) + self.assertEqual([(row["work_type"], row["commits"]) for row in rows], [("", 1)]) + + +class CommitDetailsTests(unittest.TestCase): + def test_details_keep_the_first_body_line_and_reduce_paths_to_kind_counts(self): + output = ("Stop the overflow\x00\nClamp the secret-widget chart width.\nMore text.\n" + "Co-Authored-By: Someone \n\x00\n" + "10\t2\tdashboard/js/secret_widget.js\n3\t0\ttests/test_secret_widget.py\n-\t-\tlogo.png\n") + subject, detail, files = git_delivery.commit_details(output) + self.assertEqual(subject, "Stop the overflow") + self.assertEqual(detail, "Clamp the secret-widget chart width.") + self.assertEqual(files, "Changed files: 1 UI, 1 tests, 1 images (+13 / -2 lines).") + self.assertNotIn("secret_widget", files) + self.assertNotIn(".js", files) + + def test_quoted_and_renamed_paths_are_classified_by_their_new_name(self): + output = ('S\x00\x00\n1\t0\t"caf\\303\\251/tests/test_x.py"\n2\t0\ttests/a.py => src/a.py\n' + '3\t1\tsrc/{old => tests}/test_b.py\n' + '4\t0\t"tests/caf\\303\\251.py" => "src/caf\\303\\251.py"\n' + '5\t0\ttests/a{1}.py => src/c.py\n' + '6\t0\ttests/{a => b}/x.py\n') + # Old names look like tests where the new names are code, and the reverse: only new names count. + self.assertEqual(git_delivery.commit_details(output)[2], "Changed files: 3 tests, 3 code (+21 / -1 lines).") + + def test_common_trailers_are_not_taken_as_the_detail_line(self): + body = "Change-Id: I123\nReviewed-by: A \nCc: B \nNote: keep the old order.\n" + self.assertEqual(git_delivery.commit_details("S\x00" + body + "\x00")[1], "Note: keep the old order.") + for line in ("Follow-up: handle empty input", "Fixes: the crash on load"): + self.assertEqual(git_delivery.commit_details(f"S\x00{line}\n\x00")[1], line) + trailers = "Co-developed-by: A \nFixes: #123\nRefs: 1a2b3c4d\nAssisted-by: tool\nKeep it short.\n" + self.assertEqual(git_delivery.commit_details("S\x00" + trailers + "\x00")[1], "Keep it short.") + + def test_trailer_only_body_and_no_files_read_as_no_detail(self): + subject, detail, files = git_delivery.commit_details("Bump version\x00Signed-off-by: A \n\x00\n") + self.assertEqual((subject, detail, files), ("Bump version", "", "Changed files: none.")) + long_subject, long_detail, _files = git_delivery.commit_details("x" * 500 + "\x00" + "y" * 500 + "\x00") + self.assertEqual((len(long_subject), len(long_detail)), (200, 200)) + + def test_a_commit_too_large_to_list_is_labeled_from_its_message_alone(self): + tip = "a" * 40 + calls = [] + base = _subject_runner({tip: "Vendor the parser"}, tracked=("refs/remotes/origin/main",), + pushes={"refs/remotes/origin/main": tip}, git_calls=calls) + + def runner(argv, **options): + if tuple(argv[argv.index("-C") + 2:])[0:2] == ("show", "--numstat") and "--format=%s%x00%b%x00" in argv: + return {"returncode": None, "stdout": ""} # timed out + return base(argv, **options) + + with tempfile.TemporaryDirectory() as tmp: + service = meter.GitDeliveryService(str(Path(tmp) / "delivery.sqlite3"), runner=runner, + now=lambda: 200, salt="test-salt") + service.commit_classifier = classifier = FakeCommitClassifier() + service.scan([{"root": "/repo", "project": "repo · a1"}]) + self.assertEqual([subject for *_keys, subject in classifier.subjects], ["Vendor the parser"]) + self.assertEqual(classifier.details, [("Explain the change.", "Changed files: not listed.")]) + self.assertIn("subject-only", calls) + + def test_scan_hands_detail_and_file_kinds_but_never_paths_to_the_classifier(self): + tip = "a" * 40 + with tempfile.TemporaryDirectory() as tmp: + service = meter.GitDeliveryService( + str(Path(tmp) / "delivery.sqlite3"), + runner=_subject_runner({tip: "Stop the overflow"}, tracked=("refs/remotes/origin/main",), + pushes={"refs/remotes/origin/main": tip}), + now=lambda: 200, salt="test-salt", + ) + service.commit_classifier = classifier = FakeCommitClassifier() + service.scan([{"root": "/repo", "project": "repo · a1"}]) + stored = b"".join(path.read_bytes() for path in Path(tmp).iterdir()) + self.assertEqual(classifier.details, [("Explain the change.", "Changed files: 1 code (+2 / -1 lines).")]) + self.assertNotIn("app.py", json.dumps(classifier.details)) + self.assertNotIn(b"Explain the change", stored) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_git_delivery.py b/tests/test_git_delivery.py index 59491db..b6bfe90 100644 --- a/tests/test_git_delivery.py +++ b/tests/test_git_delivery.py @@ -2868,493 +2868,3 @@ def test_shape_rows_expose_each_measure_with_its_own_day_count(self): if __name__ == "__main__": unittest.main() - -class FakeCommitClassifier: - def __init__(self, version=git_delivery.COMMIT_LABEL_VERSION, since_day="0000-00-00", room=1000): - self.version, self.since_day, self.room = version, since_day, room - self.subjects = [] - self.details = [] - - def commit_label_request(self): - return {"version": self.version, "since_day": self.since_day} - - def commit_status(self, repo_key, object_key): - if any((repo, obj) == (repo_key, object_key) for repo, obj, _subject in self.subjects): - return "queued" - return "take" if len(self.subjects) < self.room else "later" - - def observe_commit(self, repo_key, object_key, subject, observed_at, detail="", files=""): - self.details.append((detail, files)) - if len(self.subjects) >= self.room: - return False - self.subjects.append((repo_key, object_key, subject)) - return True - - -def _subject_runner(subjects, **kwargs): - base = _ref_runner(**kwargs) - reads = kwargs.get("git_calls") - - def runner(argv, **options): - args = tuple(argv[argv.index("-C") + 2:]) - if args[0:3] == ("show", "-s", "--format=%s%x00%b%x00"): - if reads is not None: - reads.append("subject-only") - return {"returncode": 0, "stdout": subjects.get(args[-1], "Add a chart") + "\x00Explain the change.\n\x00\n"} - if args[0:3] == ("show", "--numstat", "--format=%s%x00%b%x00"): - if reads is not None: - reads.append("subject") - return {"returncode": 0, "stdout": subjects.get(args[-1], "Add a chart") - + "\x00Explain the change.\n\x00\n2\t1\tsrc/app.py\n"} - return base(argv, **options) - - return runner - - -class GitCommitWorkTypeTests(unittest.TestCase): - def ledger(self, tmp): - return meter.GitDeliveryLedger(str(Path(tmp) / "delivery.sqlite3"), "test-salt") - - def test_labels_persist_only_type_confidence_version_and_model(self): - with tempfile.TemporaryDirectory() as tmp: - ledger = self.ledger(tmp) - ledger.record("repo", "obj", local_timestamp("2026-09-03"), 10, 2) - self.assertTrue(ledger.record_commit_label("repo", "obj", "fix", 0.91, git_delivery.COMMIT_LABEL_VERSION, "digest", 5)) - self.assertFalse(ledger.record_commit_label("repo", "missing", "fix", 0.9, git_delivery.COMMIT_LABEL_VERSION, "digest", 5)) - with self.assertRaises(ValueError): - ledger.record_commit_label("repo", "obj", "chores", 0.9, git_delivery.COMMIT_LABEL_VERSION, "digest", 5) - with sqlite3.connect(Path(tmp) / "delivery.sqlite3") as connection: - columns = {row[1] for row in connection.execute("PRAGMA table_info(delivery_commit_labels)")} - self.assertEqual(columns, {"repo_key", "object_key", "work_type", "confidence", "version", - "model", "labeled_at"}) - - def test_daily_type_rows_split_by_label_and_treat_other_versions_as_unlabeled(self): - with tempfile.TemporaryDirectory() as tmp: - ledger = self.ledger(tmp) - day = local_timestamp("2026-09-03") - for name, lines in (("a", 10), ("b", 6), ("c", 4), ("d", 1)): - ledger.record("repo", name, day, lines, 0) - ledger.record_commit_label("repo", "a", "feature", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) - ledger.record_commit_label("repo", "b", "feature", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) - ledger.record_commit_label("repo", "c", "fix", 0.9, "c0", "m", 1) # stale question version - - rows = ledger.daily_type_rows(["repo"], "2026-09-03", "2026-09-03", git_delivery.COMMIT_LABEL_VERSION) - self.assertEqual(sorted((row["work_type"], row["commits"], row["lines"]) for row in rows), - [("", 2, 5), ("feature", 2, 16)]) - self.assertEqual(ledger.unlabeled_keys("repo", git_delivery.COMMIT_LABEL_VERSION, "2026-09-01", 10), {"c", "d"}) - ledger.clear(10) - self.assertEqual(ledger.daily_type_rows(["repo"], "2026-09-03", "2026-09-03", git_delivery.COMMIT_LABEL_VERSION), []) - - def test_scan_hands_new_commit_subjects_to_the_classifier_and_never_stores_them(self): - tip = "a" * 40 - with tempfile.TemporaryDirectory() as tmp: - service = meter.GitDeliveryService( - str(Path(tmp) / "delivery.sqlite3"), - runner=_subject_runner({tip: "Fix the secret-sauce overflow"}, - tracked=("refs/remotes/origin/main",), - pushes={"refs/remotes/origin/main": tip}), - now=lambda: 200, salt="test-salt", - ) - classifier = FakeCommitClassifier() - service.commit_classifier = classifier - service.scan([{"root": "/repo", "project": "repo · a1"}]) - - self.assertEqual([subject for _repo, _obj, subject in classifier.subjects], - ["Fix the secret-sauce overflow"]) - repo_key, object_key, _subject = classifier.subjects[0] - self.assertTrue(service.ledger.record_commit_label(repo_key, object_key, "fix", 0.8, git_delivery.COMMIT_LABEL_VERSION, "m", 1)) - stored = b"".join(path.read_bytes() for path in Path(tmp).iterdir()) - self.assertNotIn(b"secret-sauce", stored) - - def test_scan_reads_no_subjects_without_an_enabled_classifier(self): - calls = [] - with tempfile.TemporaryDirectory() as tmp: - service = meter.GitDeliveryService( - str(Path(tmp) / "delivery.sqlite3"), - runner=_subject_runner({}, tracked=("refs/remotes/origin/main",), - pushes={"refs/remotes/origin/main": "a" * 40}, git_calls=calls), - now=lambda: 200, salt="test-salt", - ) - service.scan([{"root": "/repo", "project": "repo · a1"}]) - classifier = FakeCommitClassifier() - classifier.commit_label_request = lambda: None - service.commit_classifier = classifier - service.scan([{"root": "/repo", "project": "repo · a1"}]) - self.assertNotIn("subject", calls) - - def test_scan_backfills_subjects_of_already_counted_unlabeled_commits(self): - tip = "a" * 40 - with tempfile.TemporaryDirectory() as tmp: - runner = _subject_runner({tip: "Add the weekly chart"}, tracked=("refs/remotes/origin/main",), - pushes={"refs/remotes/origin/main": tip}) - service = meter.GitDeliveryService(str(Path(tmp) / "delivery.sqlite3"), runner=runner, - now=lambda: 200, salt="test-salt") - service.scan([{"root": "/repo", "project": "repo · a1"}]) # counted before labeling existed - classifier = FakeCommitClassifier() - service.commit_classifier = classifier - service.scan([{"root": "/repo", "project": "repo · a1"}]) - self.assertEqual([subject for *_keys, subject in classifier.subjects], ["Add the weekly chart"]) - repo_key, object_key, _subject = classifier.subjects[0] - service.ledger.record_commit_label(repo_key, object_key, "feature", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) - classifier.subjects.clear() - service.scan([{"root": "/repo", "project": "repo · a1"}]) - self.assertEqual(classifier.subjects, []) # labeled commits are not offered again - - def test_commits_no_push_introduces_are_not_walked_for_again(self): - tip = "a" * 40 - calls = [] - with tempfile.TemporaryDirectory() as tmp: - runner = _subject_runner({}, tracked=("refs/remotes/origin/main",), - pushes={"refs/remotes/origin/main": tip}, git_calls=calls) - service = meter.GitDeliveryService(str(Path(tmp) / "delivery.sqlite3"), runner=runner, - now=lambda: 200, salt="test-salt") - service.scan([{"root": "/repo", "project": "repo · a1"}]) - repo_key = service.ledger.rows()[0]["repo_key"] - # Counted from a branch whose reflog is gone (deleted after merge): no push introduces it now. - service.ledger.record(repo_key, "orphan", 150, 3, 0) - classifier = FakeCommitClassifier() - service.commit_classifier = classifier - service.scan([{"root": "/repo", "project": "repo · a1"}]) # offers the tip; the orphan is missed - for repo, obj, _subject in classifier.subjects: - service.ledger.record_commit_label(repo, obj, "feature", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) - calls.clear() - service.scan([{"root": "/repo", "project": "repo · a1"}]) - service.scan([{"root": "/repo", "project": "repo · a1"}]) - self.assertNotIn("subject", calls) - self.assertNotIn("rev-list", calls) - service.forget_commit_subjects() - service.scan([{"root": "/repo", "project": "repo · a1"}]) - self.assertIn("rev-list", calls) # a label delete offers it again - - def service_with(self, tmp, calls, commits=None, failing=()): - tip = "a" * 40 - runner = _subject_runner({}, tracked=("refs/remotes/origin/main",), - pushes={"refs/remotes/origin/main": tip}, git_calls=calls, - commits=commits, failing=failing) - return meter.GitDeliveryService(str(Path(tmp) / "delivery.sqlite3"), runner=runner, - now=lambda: 200, salt="test-salt") - - def test_subject_is_not_read_for_a_commit_whose_attempts_are_used_up(self): - calls = [] - with tempfile.TemporaryDirectory() as tmp: - service = self.service_with(tmp, calls) - classifier = FakeCommitClassifier() - classifier.commit_status = lambda repo_key, object_key: "never" - service.commit_classifier = classifier - service.scan([{"root": "/repo", "project": "repo · a1"}]) - service.scan([{"root": "/repo", "project": "repo · a1"}]) - self.assertNotIn("subject", calls) - self.assertEqual(classifier.subjects, []) - - def test_commits_that_did_not_fit_a_full_queue_are_offered_once_it_drains(self): - calls = [] - tip = "a" * 40 - chain = ("c" * 40, "d" * 40, tip) - with tempfile.TemporaryDirectory() as tmp: - service = self.service_with(tmp, calls, commits={tip: chain}) - classifier = FakeCommitClassifier(room=1) - service.commit_classifier = classifier - service.scan([{"root": "/repo", "project": "repo · a1"}]) - self.assertEqual(len(classifier.subjects), 1) - classifier.room = 1000 # the worker drained the queue - service.scan([{"root": "/repo", "project": "repo · a1"}]) - self.assertEqual(len({obj for _repo, obj, _subject in classifier.subjects}), 3) - - def test_a_still_queued_commit_is_not_skipped_and_is_offered_again_after_it_fails(self): - calls = [] - with tempfile.TemporaryDirectory() as tmp: - service = self.service_with(tmp, calls) - classifier = FakeCommitClassifier() - service.commit_classifier = classifier - service.scan([{"root": "/repo", "project": "repo · a1"}]) - service.scan([{"root": "/repo", "project": "repo · a1"}]) # still queued: left alone - self.assertEqual(len(classifier.subjects), 1) - classifier.subjects.clear() # the model call failed and the item was dropped - service.scan([{"root": "/repo", "project": "repo · a1"}]) - self.assertEqual(len(classifier.subjects), 1) - - def test_a_walk_truncated_by_a_size_cap_still_lets_unfound_commits_be_skipped(self): - calls = [] - tip = "a" * 40 - with tempfile.TemporaryDirectory() as tmp, \ - mock.patch.object(git_delivery, "MAX_COMMITS_PER_PUSH", 2): - service = self.service_with(tmp, calls, commits={tip: ("c" * 40, "d" * 40, tip)}) - service.scan([{"root": "/repo", "project": "repo · a1"}]) - repo_key = service.ledger.rows()[0]["repo_key"] - service.ledger.record(repo_key, "orphan", 150, 3, 0) # its branch was deleted after merge - service.commit_classifier = classifier = FakeCommitClassifier() - service.scan([{"root": "/repo", "project": "repo · a1"}]) - self.assertIn("orphan", service._subject_skips.get(repo_key, set())) - - def test_a_failed_backfill_walk_does_not_skip_and_is_retried(self): - tip = "a" * 40 - fail_walks = [False] - base = _subject_runner({}, tracked=("refs/remotes/origin/main",), - pushes={"refs/remotes/origin/main": tip}) - - def runner(argv, **options): - if tuple(argv[argv.index("-C") + 2:])[0] == "rev-list" and fail_walks[0]: - return {"returncode": 128, "stdout": ""} - return base(argv, **options) - - with tempfile.TemporaryDirectory() as tmp: - service = meter.GitDeliveryService(str(Path(tmp) / "delivery.sqlite3"), runner=runner, - now=lambda: 200, salt="test-salt") - service.scan([{"root": "/repo", "project": "repo · a1"}]) # counted before labeling existed - service.commit_classifier = classifier = FakeCommitClassifier() - fail_walks[0] = True - service.scan([{"root": "/repo", "project": "repo · a1"}]) - self.assertEqual(classifier.subjects, []) - self.assertEqual(service._subject_skips, {}) - fail_walks[0] = False - service.scan([{"root": "/repo", "project": "repo · a1"}]) - self.assertEqual(len(classifier.subjects), 1) - - def test_a_failed_subject_read_is_retried_by_a_later_scan(self): - tip = "a" * 40 - failures = [True, True] # the full read and its message-only fallback - base = _subject_runner({}, tracked=("refs/remotes/origin/main",), - pushes={"refs/remotes/origin/main": tip}) - - def runner(argv, **options): - if tuple(argv[argv.index("-C") + 2:])[0] == "show" and "--format=%s%x00%b%x00" in argv and failures: - failures.pop() - return {"returncode": 128, "stdout": ""} - return base(argv, **options) - - with tempfile.TemporaryDirectory() as tmp: - service = meter.GitDeliveryService(str(Path(tmp) / "delivery.sqlite3"), runner=runner, - now=lambda: 200, salt="test-salt") - service.commit_classifier = classifier = FakeCommitClassifier() - service.scan([{"root": "/repo", "project": "repo · a1"}]) - self.assertEqual(classifier.subjects, []) - self.assertEqual(service._subject_skips, {}) - service.scan([{"root": "/repo", "project": "repo · a1"}]) - self.assertEqual(len(classifier.subjects), 1) - - def test_a_transient_reflog_failure_does_not_skip_unfound_commits(self): - calls = [] - with tempfile.TemporaryDirectory() as tmp: - service = self.service_with(tmp, calls) - service.scan([{"root": "/repo", "project": "repo · a1"}]) # counted before labeling existed - failing = self.service_with(tmp, calls, failing=("refs/remotes/origin/main",)) - failing.commit_classifier = classifier = FakeCommitClassifier() - failing.scan([{"root": "/repo", "project": "repo · a1"}]) - self.assertEqual(classifier.subjects, []) - failing._runner = service._runner # the reflog reads again - failing.scan([{"root": "/repo", "project": "repo · a1"}]) - self.assertEqual(len(classifier.subjects), 1) - - def test_query_reports_daily_work_types_with_unlabeled_commits_explicit(self): - tip = "a" * 40 - today = datetime.date.fromtimestamp(local_timestamp("2026-09-04")).isoformat() - with tempfile.TemporaryDirectory() as tmp: - service = meter.GitDeliveryService( - str(Path(tmp) / "delivery.sqlite3"), - runner=_subject_runner({}, tracked=("refs/remotes/origin/main",), - pushes={"refs/remotes/origin/main": tip}), - now=lambda: local_timestamp("2026-09-04"), salt="test-salt", - ) - service.scan([{"root": "/repo", "project": "repo · a1"}]) - repo_key = service.ledger.rows()[0]["repo_key"] - service.ledger.record("repo-other", "x", local_timestamp("2026-09-04"), 1, 1) - # The scanned push is dated by the fake reflog long ago; these two land in the window. - service.ledger.record(repo_key, "first", local_timestamp("2026-09-04"), 2, 1) - service.ledger.record(repo_key, "second", local_timestamp("2026-09-04"), 7, 0) - service.ledger.record_commit_label(repo_key, "second", "docs", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) - payload = service.query("", "7", [], ["repo · a1"], [{"root": "/repo", "project": "repo · a1"}]) - - work = payload["work_types"] - self.assertEqual(work["commits"], 2) - self.assertEqual(work["labeled_commits"], 1) - self.assertFalse(work["classifier_enabled"]) - self.assertTrue(work["estimate"]) - self.assertEqual(work["totals"], {"unlabeled": {"commits": 1, "lines": 3}, - "docs": {"commits": 1, "lines": 7}}) - self.assertEqual(len(work["days"]), 7) - self.assertEqual(work["days"][-1]["day"], today) - self.assertEqual(work["days"][-1]["types"]["docs"], {"commits": 1, "lines": 7}) - - -class GitWorkTypeChartContractTests(unittest.TestCase): - page = Path(DASHBOARD_PAGE).read_text(encoding="utf-8") - - def test_work_type_card_sits_after_the_daily_chart_and_resizes_with_it(self): - git_page = self.page.split("id=view-git", 1)[1].split("id=view-learn", 1)[0] - self.assertLess(git_page.index("id=d-daily-chart"), git_page.index("id=d-type-chart")) - self.assertLess(git_page.index("id=d-type-chart"), git_page.index("id=d-insight-title")) - self.assertIn(">Work pushed by type<", git_page) - self.assertIn("data-delivery-type-metric=commits", git_page) - self.assertIn("data-delivery-type-metric=lines", git_page) - self.assertIn("git:['d-daily-chart','d-type-chart']", self.page) - self.assertIn("drawDeliveryTypeChart(payload?.work_types)", self.page) - colors = re.search(r"^const DELIVERY_TYPE_COLORS=\{[^\n]*\};$", self.page, re.MULTILINE).group(0) - self.assertNotRegex(colors, r"#[0-9a-fA-F]{3,8}|rgba?\(") - - @unittest.skipUnless(shutil.which("node"), "Node.js is required for dashboard JavaScript") - def test_chart_stacks_each_day_and_keeps_unlabeled_commits_explicit(self): - preamble = """ -const nodes={}; -const node=()=>({innerHTML:'',textContent:'',hidden:false,clientWidth:1040,clientHeight:270,style:{},attrs:{}, - setAttribute(name,value){this.attrs[name]=value;},getBoundingClientRect(){return{left:0,width:1040};}}); -['d-type-chart','d-type-tip','d-type-legend','d-type-note','d-type-hover'].forEach(id=>nodes[id]=node()); -const $=id=>nodes[id]; -const document={querySelectorAll:()=>[]}; -const esc=value=>String(value); -const deliveryPercent=value=>`${Math.round(value)}%`; -const deliveryDateLabel=day=>day; -let deliveryTypeMetric='commits'; -""" - driver = """ -const work={available:true,classifier_enabled:false,commits:4,labeled_commits:3, - totals:{fix:{commits:2,lines:30},feature:{commits:1,lines:5},unlabeled:{commits:1,lines:9}}, - days:[{day:'Sep 3',types:{fix:{commits:2,lines:30},unlabeled:{commits:1,lines:9}}},{day:'Sep 4',types:{feature:{commits:1,lines:5}}}]}; -drawDeliveryTypeChart(work); -const svg=nodes['d-type-chart'].innerHTML; -const first={segments:(svg.match(/class="typeSeg/g)||[]).length,unlabeled:(svg.match(/typeSeg unlabeled/g)||[]).length, - legend:nodes['d-type-legend'].innerHTML,note:nodes['d-type-note'].textContent}; -deliveryTypeMetric='lines';drawDeliveryTypeChart(work); -const lines=nodes['d-type-chart'].attrs['aria-label']; -const svgNode=nodes['d-type-chart'],keys=[]; -svgNode.onfocus();keys.push(nodes['d-type-tip'].innerHTML); -svgNode.onkeydown({key:'ArrowLeft',preventDefault(){}});keys.push(nodes['d-type-tip'].innerHTML); -svgNode.onkeydown({key:'End',preventDefault(){}});keys.push(nodes['d-type-tip'].innerHTML); -const hatched=nodes['d-type-chart'].innerHTML.includes('fill:url(#d-type-hatch)')&&nodes['d-type-chart'].innerHTML.includes('50%", result["first"]["legend"]) - self.assertIn("Not labeled yet 25%", result["first"]["legend"]) - self.assertEqual(result["first"]["note"], - "3 of 4 commits labeled · turn on Work insights in Settings to label the rest locally") - self.assertEqual(result["lines"], "Pushed lines per day by type: Features 5, Fixes 30, Not labeled yet 9") - self.assertIn("Sep 4", result["keys"][0]) # focus opens on the latest day - self.assertIn("Sep 3", result["keys"][1]) # arrows move between days - self.assertIn("Sep 4", result["keys"][2]) - self.assertTrue(result["hatched"]) - self.assertEqual(result["axes"], [2, 4, 6, 10, 56, 1600]) # even tops: every gridline is whole - self.assertEqual(result["loading"], "Reading labels\u2026") - self.assertIn("No pushed commits in this period", result["empty"]) - self.assertEqual(result["emptyNote"], "No pushed commits in this period.") - - -class CommitWorkLabelClearTests(unittest.TestCase): - def test_commit_labels_are_deleted_even_when_the_work_ledger_delete_fails(self): - with tempfile.TemporaryDirectory() as tmp: - service = meter.GitDeliveryService(str(Path(tmp) / "delivery.sqlite3"), runner=_ref_runner(), - now=lambda: 200, salt="test-salt") - service.ledger.record("repo", "obj", local_timestamp("2026-09-03"), 3, 1) - service.ledger.record_commit_label("repo", "obj", "docs", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) - work = mock.Mock() - work.clear.side_effect = meter._work.LabelDeleteError("busy") - with mock.patch.object(meter, "git_delivery_service", return_value=service), \ - mock.patch.object(meter, "work_insights_service_if_started", return_value=work): - with self.assertRaises(meter._work.LabelDeleteError): - meter.clear_work_insights() - rows = service.ledger.daily_type_rows(["repo"], "2026-09-03", "2026-09-03", git_delivery.COMMIT_LABEL_VERSION) - self.assertEqual([(row["work_type"], row["commits"]) for row in rows], [("", 1)]) - - def test_a_git_ledger_failure_is_reported_not_swallowed(self): - service = mock.Mock() - service.ledger.clear_commit_labels.side_effect = sqlite3.OperationalError("locked") - with mock.patch.object(meter, "git_delivery_service", return_value=service), \ - mock.patch.object(meter, "work_insights_service_if_started", return_value=mock.Mock()): - with self.assertRaises(sqlite3.Error): - meter.clear_work_insights() - - def test_deleting_work_labels_also_deletes_commit_work_type_labels(self): - with tempfile.TemporaryDirectory() as tmp: - service = meter.GitDeliveryService(str(Path(tmp) / "delivery.sqlite3"), runner=_ref_runner(), - now=lambda: 200, salt="test-salt") - service.ledger.record("repo", "obj", local_timestamp("2026-09-03"), 3, 1) - service.ledger.record_commit_label("repo", "obj", "docs", 0.9, git_delivery.COMMIT_LABEL_VERSION, "m", 1) - with mock.patch.object(meter, "git_delivery_service", return_value=service), \ - mock.patch.object(meter, "work_insights_service_if_started", return_value=None), \ - mock.patch.object(meter, "TOKEN_METER_WORK_INSIGHTS_DB", str(Path(tmp) / "work.sqlite3")): - meter.clear_work_insights() - rows = service.ledger.daily_type_rows(["repo"], "2026-09-03", "2026-09-03", git_delivery.COMMIT_LABEL_VERSION) - self.assertEqual([(row["work_type"], row["commits"]) for row in rows], [("", 1)]) - - -class CommitDetailsTests(unittest.TestCase): - def test_details_keep_the_first_body_line_and_reduce_paths_to_kind_counts(self): - output = ("Stop the overflow\x00\nClamp the secret-widget chart width.\nMore text.\n" - "Co-Authored-By: Someone \n\x00\n" - "10\t2\tdashboard/js/secret_widget.js\n3\t0\ttests/test_secret_widget.py\n-\t-\tlogo.png\n") - subject, detail, files = git_delivery.commit_details(output) - self.assertEqual(subject, "Stop the overflow") - self.assertEqual(detail, "Clamp the secret-widget chart width.") - self.assertEqual(files, "Changed files: 1 UI, 1 tests, 1 images (+13 / -2 lines).") - self.assertNotIn("secret_widget", files) - self.assertNotIn(".js", files) - - def test_quoted_and_renamed_paths_are_classified_by_their_new_name(self): - output = ('S\x00\x00\n1\t0\t"caf\\303\\251/tests/test_x.py"\n2\t0\ttests/a.py => src/a.py\n' - '3\t1\tsrc/{old => tests}/test_b.py\n' - '4\t0\t"tests/caf\\303\\251.py" => "src/caf\\303\\251.py"\n' - '5\t0\tsrc/a{1}.py => tests/test_c.py\n') - # Old names look like tests where the new names are code, and the reverse: only new names count. - self.assertEqual(git_delivery.commit_details(output)[2], "Changed files: 3 tests, 2 code (+15 / -1 lines).") - - def test_common_trailers_are_not_taken_as_the_detail_line(self): - body = "Change-Id: I123\nReviewed-by: A \nCc: B \nNote: keep the old order.\n" - self.assertEqual(git_delivery.commit_details("S\x00" + body + "\x00")[1], "Note: keep the old order.") - for line in ("Follow-up: handle empty input", "Fixes: the crash on load"): - self.assertEqual(git_delivery.commit_details(f"S\x00{line}\n\x00")[1], line) - trailers = "Co-developed-by: A \nFixes: #123\nRefs: 1a2b3c4d\nAssisted-by: tool\nKeep it short.\n" - self.assertEqual(git_delivery.commit_details("S\x00" + trailers + "\x00")[1], "Keep it short.") - - def test_trailer_only_body_and_no_files_read_as_no_detail(self): - subject, detail, files = git_delivery.commit_details("Bump version\x00Signed-off-by: A \n\x00\n") - self.assertEqual((subject, detail, files), ("Bump version", "", "Changed files: none.")) - long_subject, long_detail, _files = git_delivery.commit_details("x" * 500 + "\x00" + "y" * 500 + "\x00") - self.assertEqual((len(long_subject), len(long_detail)), (200, 200)) - - def test_a_commit_too_large_to_list_is_labeled_from_its_message_alone(self): - tip = "a" * 40 - calls = [] - base = _subject_runner({tip: "Vendor the parser"}, tracked=("refs/remotes/origin/main",), - pushes={"refs/remotes/origin/main": tip}, git_calls=calls) - - def runner(argv, **options): - if tuple(argv[argv.index("-C") + 2:])[0:2] == ("show", "--numstat") and "--format=%s%x00%b%x00" in argv: - return {"returncode": None, "stdout": ""} # timed out - return base(argv, **options) - - with tempfile.TemporaryDirectory() as tmp: - service = meter.GitDeliveryService(str(Path(tmp) / "delivery.sqlite3"), runner=runner, - now=lambda: 200, salt="test-salt") - service.commit_classifier = classifier = FakeCommitClassifier() - service.scan([{"root": "/repo", "project": "repo · a1"}]) - self.assertEqual([subject for *_keys, subject in classifier.subjects], ["Vendor the parser"]) - self.assertEqual(classifier.details, [("Explain the change.", "Changed files: not listed.")]) - self.assertIn("subject-only", calls) - - def test_scan_hands_detail_and_file_kinds_but_never_paths_to_the_classifier(self): - tip = "a" * 40 - with tempfile.TemporaryDirectory() as tmp: - service = meter.GitDeliveryService( - str(Path(tmp) / "delivery.sqlite3"), - runner=_subject_runner({tip: "Stop the overflow"}, tracked=("refs/remotes/origin/main",), - pushes={"refs/remotes/origin/main": tip}), - now=lambda: 200, salt="test-salt", - ) - service.commit_classifier = classifier = FakeCommitClassifier() - service.scan([{"root": "/repo", "project": "repo · a1"}]) - stored = b"".join(path.read_bytes() for path in Path(tmp).iterdir()) - self.assertEqual(classifier.details, [("Explain the change.", "Changed files: 1 code (+2 / -1 lines).")]) - self.assertNotIn("app.py", json.dumps(classifier.details)) - self.assertNotIn(b"Explain the change", stored) From 622683d4b7458217070a4873cdfada62d32ac608 Mon Sep 17 00:00:00 2001 From: Pratik Bhavsar Date: Fri, 6 Nov 2026 13:10:04 +0530 Subject: [PATCH 6/6] test(git): drop trailing blank line left by the test move Co-Authored-By: Claude Code --- tests/test_git_delivery.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_git_delivery.py b/tests/test_git_delivery.py index b6bfe90..f663b8a 100644 --- a/tests/test_git_delivery.py +++ b/tests/test_git_delivery.py @@ -2867,4 +2867,3 @@ def test_shape_rows_expose_each_measure_with_its_own_day_count(self): if __name__ == "__main__": unittest.main() -