Skip to content

Commit 62d486e

Browse files
author
ca.mathieu
committed
docs(stats): finish scrubbing development-process references from comments
The earlier comment cleanup swept server/ and webapp/src/ but missed webapp/e2e/, the plik client tests, and task numbers written as words. Comments now carry their rationale inline instead of referencing decision IDs, task numbers, or review-finding codes that exist only in uncommitted planning documents. Also drops a stale mention of a deleted retry symbol and the registry's removed add closures.
1 parent 50bfa21 commit 62d486e

8 files changed

Lines changed: 23 additions & 22 deletions

File tree

plik/client_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -240,9 +240,9 @@ func TestUploadFiles(t *testing.T) {
240240
// This uploads ~30 files onto one shared Upload concurrently; each file status
241241
// write takes server/metadata's canonical upload-row lock (lockUploadRow) before
242242
// touching the file row. It was historically flaky (~17% over 60 runs) because
243-
// the old fixed retry budget (maxStatsRetryAttempts) could exhaust under SQLite
244-
// write contention and surface a 500 ("database is locked"). Task 24 removed the
245-
// retry apparatus and instead opens SQLite transactions with the write lock held
243+
// a fixed transaction-retry budget could exhaust under SQLite write contention
244+
// and surface a 500 ("database is locked"). That retry apparatus is gone:
245+
// SQLite transactions are instead opened with the write lock held
246246
// from BEGIN (_txlock=immediate, see metadata.sqliteConnectionString) plus a
247247
// busy_timeout, so concurrent writers serialize as bounded lock waits rather than
248248
// racing to upgrade a deferred read into a writer and deadlocking. The retry

plik/z4_e2e_client_concurrent_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,8 @@ func TestMultipleFilesInParallel(t *testing.T) {
134134
// a process-killing panic, so it was historically sensitive to the same SQLite
135135
// write contention as TestUploadMultipleFiles (client_test.go): concurrent
136136
// uploads onto one shared Upload could exhaust the old fixed retry budget on the
137-
// upload-row lock. Task 24 removed the retry apparatus and opens SQLite
138-
// transactions IMMEDIATE (metadata.sqliteConnectionString), so writers serialize
137+
// upload-row lock. That retry apparatus is gone: SQLite transactions now open
138+
// IMMEDIATE (metadata.sqliteConnectionString), so writers serialize
139139
// as bounded busy_timeout lock waits instead of deadlocking on a deferred-read
140140
// upgrade. -race shows no data races.
141141
func TestMultipleFilesInParallelBusyLoop(t *testing.T) {

server/handlers/get_file.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ func GetFile(ctx *context.Context, resp http.ResponseWriter, req *http.Request)
313313
// file, so every GET is a complete download — applying the
314314
// byte-0 range policy here would let a Range header evade
315315
// counting a one-shot/stream file that was fully consumed. The
316-
// event is therefore always counted (Task 7 behavior); recording
316+
// event is therefore always counted; recording
317317
// happens after streaming so the bytes served are captured too.
318318
cw := newCountingResponseWriter(resp)
319319

server/handlers/get_file_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -508,8 +508,8 @@ func TestGetOneShotFileRangedRequestCounts(t *testing.T) {
508508
require.Equal(t, int64(1), f.DownloadCount, "invalid download count")
509509

510510
// The one-shot/stream branch ignores Range and streams the full file, so the
511-
// full file content length is recorded as bytes served (Task 7 behavior
512-
// preserved). file.Size is not set in this fixture, so compare to len(data).
511+
// full file content length is recorded as bytes served. file.Size is not set
512+
// in this fixture, so compare to len(data).
513513
fileDownloads, fileBytes, found := dailyDownloadStatsFor(t, ctx, common.DownloadStatsEntityFile, file.ID)
514514
require.True(t, found)
515515
require.Equal(t, int64(1), fileDownloads, "one-shot ranged GET still counts one event")

server/metadata/stats_counters_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func gormColumnName(tag string) string {
3434
// this test; production paths iterate the registry directly.
3535
//
3636
// This only checks that the column *names* match set-for-set; it says nothing
37-
// about whether each counterSpec's get/add/set closures are wired to the right
37+
// about whether each counterSpec's get/set closures are wired to the right
3838
// usageDelta/common.UsageStats field. That correctness is covered elsewhere, by
3939
// the agreement/exactness tests (e.g. TestBackend_ImportBackfillLifetimeCounterAgreement
4040
// and the various "...IsExact" tests in stats_test.go) that exercise real

webapp/ARCHITECTURE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -579,7 +579,7 @@ Extracted so the current/lifetime stats dashboard is defined once and reused per
579579
| `StatsUsagePanel` | Composite. Given the canonical `usage` object it renders the current/lifetime tile columns, one merged **Activity** card, and the six side-by-side distribution cards (feature / TTL / file-size). The Activity card has a 4-metric segmented selector — Downloads / Uploads / Downloaded data / Uploaded data — that drives BOTH the window tiles (Today / 7 days / 30 days / **Lifetime**) and the `ActivityChart` for the selected metric. Count-metric windows come from the `usage` object (`downloads`/`uploads` today/last7Days/last30Days + total); byte-metric windows are summed from `dailySeries` (Lifetime from `usage.*.bytes`), so a null series leaves byte windows at 0 while Lifetime still renders. Scope-specific bits are props/slots: `currentTiles`/`lifetimeTiles`, `showActivity` (scopes without windows/series), a `#storage` slot (admin-only), and `dailySeries` (the 30-day activity series; `null`/empty omits the chart but keeps the tiles). The "since {date}" is printed once, in the lifetime column title. |
580580
| `ActivityChart` | Presentational 30-day activity column chart. It is **controlled** by the panel: the measure is a `metric` prop (`downloads` \| `uploads` \| `downloadedBytes` \| `uploadedBytes`) — the segmented selector lives on the Activity card, not inside the chart. Props: `series` (`[{day, downloads, downloadedBytes, uploads, uploadedBytes}]`) and `metric`. Bars use a single accent token, axis/tooltip text use `surface-*` ink, gridlines a recessive surface token; y-axis starts at 0 with ~3 gridlines, x-axis labels every 7th UTC day, values humanized per metric — counts (`1.2k`) for download/upload metrics, byte sizes (`1.5 MB`) for the `*Bytes` metrics. Per-column hover shows a day+value tooltip (edge-flipped so it never clips); all-zero data draws the axis + a quiet empty-state line; the grow-in animation respects `prefers-reduced-motion`. Pure scale/tick/label helpers live in `chart-utils.js` (unit-tested), including `isByteMetric()` which classifies the four metric keys. |
581581

582-
> `StatsUsagePanel` owns the `statsPanel.*` i18n namespace (period titles, the Activity metric-selector labels, window-tile labels, distribution labels, and the `ActivityChart` empty/aria strings). Distribution bar colours are intentionally a single accent token pending the colour-finalisation task.
582+
> `StatsUsagePanel` owns the `statsPanel.*` i18n namespace (period titles, the Activity metric-selector labels, window-tile labels, distribution labels, and the `ActivityChart` empty/aria strings). Distribution bar colours are intentionally a single accent token; per-category tinting is a possible future refinement.
583583
>
584584
> `ActivityChart` is a hand-rolled SVG in a Vue SFC with **no charting dependency** — a bar chart this small needs only `<rect>`/`<path>` plus a handful of pure geometry helpers, so adding a library (and its bundle/theming/maintenance cost) would not pay for itself; theme reactivity comes for free from the `--color-*` tokens the SVG references.
585585

webapp/e2e/stats-ui.spec.js

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ test.describe('Stats UI', () => {
8181
// Genuinely binary content (NUL bytes) — the server sniffs actual bytes
8282
// to set isText, regardless of filename/declared content-type. A real
8383
// .bin file must not be auto-previewed by DownloadView, or the "Never
84-
// downloaded yet" baseline below wouldn't hold (D2: previews count).
84+
// downloaded yet" baseline below wouldn't hold (previews count as downloads).
8585
const binaryContent = '\x00'.repeat(32)
8686
const never = await createAuthenticatedUploadWithFile(page, 'stats-download-never.bin', binaryContent)
8787
await page.goto(`/#/?id=${never.upload.id}`)
@@ -155,9 +155,10 @@ test.describe('Stats UI', () => {
155155
})
156156

157157
test('sidebar download count reflects the auto-preview immediately after an in-app upload (no stale count)', async ({ authenticatedPage: page }) => {
158-
// D2: owner self-downloads and previews count as downloads. Uploading a
159-
// single text file through the UI auto-opens its preview, which fetches
160-
// the file content and — per D2 — is itself counted as a download.
158+
// Owner self-downloads and previews count as downloads (they are real
159+
// served traffic). Uploading a single text file through the UI auto-opens
160+
// its preview, which fetches the file content and is itself counted as a
161+
// download.
161162
// Regression check for the stale-count fix: without any reload, the
162163
// sidebar must reflect that download, not the pre-preview count.
163164
await uploadTestFile(page, 'stats-stale-count.txt', 'stale count regression content')
@@ -229,7 +230,7 @@ test.describe('Stats UI', () => {
229230

230231
// Trending window chips use the same Today/7 days/30 days/Lifetime
231232
// vocabulary as the Activity tiles (not the raw 1d/7d/30d/all API values,
232-
// and not the "All time" wording those tiles retired — B1/audit-tech F1).
233+
// and not the retired "All time" wording).
233234
const trendingCard = page.locator('.glass-card').filter({ hasText: 'Trending Uploads' })
234235
await expect(trendingCard.getByRole('button', { name: 'Today', exact: true })).toBeVisible()
235236
await expect(trendingCard.getByRole('button', { name: '7 days', exact: true })).toBeVisible()
@@ -276,7 +277,7 @@ test.describe('Stats UI', () => {
276277
await expect(bytesToggle).toBeVisible()
277278

278279
// Default sort (downloads): every row shows BOTH a downloads figure and a
279-
// humanized bytes figure (T27 — the toggle emphasizes, never hides, the
280+
// humanized bytes figure (the toggle emphasizes, never hides, the
280281
// other metric).
281282
await expect(trendingCard.getByText(/\d+ downloads/).first()).toBeVisible()
282283
await expect(trendingCard.getByText(/[\d.]+\s*(B|KB|MB|GB)/).first()).toBeVisible()
@@ -303,7 +304,7 @@ test.describe('Stats UI', () => {
303304
await expect(page.getByText('Current Usage')).toBeVisible()
304305
await expect(page.getByText('Lifetime Usage')).toBeVisible()
305306

306-
// D9: Home renders through the same shared StatsUsagePanel as Admin, scoped
307+
// Home renders through the same shared StatsUsagePanel as Admin, scoped
307308
// to this user — distribution cards and the downloads windows row included.
308309
await expect(page.getByText('Current Feature Usage')).toBeVisible()
309310
await expect(page.getByText('Lifetime Feature Usage')).toBeVisible()
@@ -316,10 +317,10 @@ test.describe('Stats UI', () => {
316317
await expect(activityPanel.getByText('Lifetime', { exact: true })).toBeVisible()
317318

318319
// Privacy: no admin/server-scope-only data leaks onto the user's Home page.
319-
// T27 adds a SELF-scoped Trending Uploads panel to Home (this user's own
320+
// Home carries a SELF-scoped Trending Uploads panel (this user's own
320321
// uploads only, dedicated test below) — so a blanket "no Trending at all"
321-
// check no longer holds; what must still never appear is the admin-only,
322-
// cross-user Trending Files card (D18: no self-scoped files endpoint) and
322+
// check would not hold; what must still never appear is the admin-only,
323+
// cross-user Trending Files card (deliberately no self-scoped files endpoint) and
323324
// the owner/anonymous chips that only make sense server-wide.
324325
await expect(page.getByText('Server Statistics')).not.toBeVisible()
325326
await expect(page.getByText('Trending Files')).not.toBeVisible()
@@ -406,7 +407,7 @@ test.describe('Stats UI', () => {
406407
await expect(tooltip).toContainText(/\d/)
407408

408409
// Switching to a byte metric swaps the tooltip to a byte value. Scoped to
409-
// the Activity card specifically (T27 added a second, unrelated
410+
// the Activity card specifically (there is a second, unrelated
410411
// "Downloaded data" toggle on this page — the Trending Uploads metric
411412
// toggle — so an unscoped lookup is now ambiguous).
412413
const activityPanel = page.locator('.glass-card').filter({ hasText: 'Activity' }).filter({ hasText: 'Lifetime' })

webapp/src/__tests__/locales.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ describe('locale suspicious-identical-to-English check', () => {
329329
})
330330

331331
it(`${file}'s allowlist has no stale entries`, () => {
332-
// Keeps the allowlist honest (task requirement: pass because things
332+
// Keeps the allowlist honest (the suite must pass because strings
333333
// are translated, not because the allowlist is bloated) — an
334334
// allowlisted key whose value no longer matches en.json (translated
335335
// since, or the English source changed) should be removed from the

0 commit comments

Comments
 (0)