Skip to content

Commit 0e52eb8

Browse files
danyelfclaude
andcommitted
docs: drop project-phase and ticket-id references from this PR's code comments
Comments shouldn't carry 'Stage A/B/C/D' or 'DRC-XXXX' — they're write-time context that means nothing to a reader later. Reworded the comments and docstrings this PR adds to describe the behavior in plain terms instead. Ticket/phase references stay in commit messages and Linear, not the code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Danyel Fisher <danyel@gmail.com>
1 parent 9f4a3dd commit 0e52eb8

10 files changed

Lines changed: 50 additions & 50 deletions

File tree

js/packages/storybook/stories/profile-distribution/InlineProfileDistributionCellDatetime.stories.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { continuousEventTime, continuousStable } from "./fixtures";
55
import { SchemaContainerMock, SchemaRowMock } from "./SchemaRowMock";
66

77
/**
8-
* Continuous tooltip formatting by column type (DRC-3390 review note 1).
8+
* Continuous tooltip formatting by column type.
99
*
1010
* The backend wraps datetime columns in an `epoch()` cast before binning, but
1111
* the numeric edges mean different things per type:

js/packages/storybook/stories/profile-distribution/fixtures.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,11 @@
2828
// The top-K (counts) and result types below are deliberately Storybook-LOCAL
2929
// and DIVERGE from the canonical `run.ts` contract: the fixtures bundle
3030
// envelope-level `base_total`/`current_total` onto the counts payload for
31-
// Stage-A display convenience, whereas the wire contract puts those on the
31+
// display convenience here, whereas the wire contract puts those on the
3232
// `ProfileDistributionOkResult` envelope and types counts as `(number|null)[]`.
3333
// Do NOT copy these shapes into production — use the `@datarecce/ui/api` types.
3434
// (Fully canonicalizing the counts fixtures means moving totals to the envelope
35-
// across the Stage-A discrete stories; deferred — storybook-only, no prod impact.)
35+
// across the discrete stories; deferred — storybook-only, no prod impact.)
3636

3737
import type { ProfileDistributionHistogramPayload } from "@datarecce/ui/api";
3838

@@ -183,7 +183,7 @@ export const continuousAddedOnly = continuousFromProportions(
183183
* backend's `epoch()` cast emits for `TIME` (distinct from the Unix-epoch
184184
* seconds a TIMESTAMP/DATE emits). An activity bump around midday. Used to
185185
* exercise the `HH:MM:SS` clock-time tooltip path (vs the calendar-date path
186-
* a TIMESTAMP takes) — see DRC-3390 review note 1.
186+
* a TIMESTAMP takes).
187187
*/
188188
const HOUR = 3600;
189189
const timeOfDayEdges = [0, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22].map(

js/packages/ui/src/components/data/InlineProfileDistributionCell.tsx

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,10 @@ import {
1919

2020
/**
2121
* @file InlineProfileDistributionCell.tsx
22-
* @description DRC-3390 Stage C — the schema-grid cell container that turns a
23-
* single column's {@link ProfileDistributionColumnPayload} into the right
24-
* Stage A paired-histogram leaf, and renders the loading / error / empty /
25-
* per-column-failure states around it.
22+
* @description The schema-grid cell container that turns a single column's
23+
* {@link ProfileDistributionColumnPayload} into the right paired-histogram
24+
* leaf, and renders the loading / error / empty / per-column-failure states
25+
* around it.
2626
*
2727
* This is the one place the snake_case wire payload is mapped to the
2828
* camelCase cell props, and the only place that has to know the column's dbt
@@ -50,10 +50,10 @@ export interface InlineProfileDistributionCellProps {
5050
columnType?: string;
5151
/**
5252
* Envelope-level row totals — the denominator for counts-mode proportions.
53-
* Unused in ranks mode (the DuckDB Stage B path that ships today), so
54-
* optional. NOT speculative: Stage D's full adapters (Snowflake/BigQuery
55-
* top-K return value+count pairs) emit counts mode, so this is threaded now
56-
* to match the single `run.ts` payload contract rather than retrofitted then.
53+
* Unused in ranks mode (the DuckDB path that ships today), so optional. NOT
54+
* speculative: full warehouse adapters (Snowflake/BigQuery top-K return
55+
* value+count pairs) emit counts mode, so this is threaded now to match the
56+
* single `run.ts` payload contract rather than retrofitted later.
5757
*/
5858
baseTotal?: number;
5959
currentTotal?: number;
@@ -65,9 +65,9 @@ export interface InlineProfileDistributionCellProps {
6565
}
6666

6767
/**
68-
* Calendar-date types whose histogram edges are seconds since the Unix epoch
69-
* (see DRC-3504). Bare `TIME` is deliberately excluded — its edges are
70-
* seconds-since-midnight, not an epoch, and is handled by `isTimeOfDayType`.
68+
* Calendar-date types whose histogram edges are seconds since the Unix epoch.
69+
* Bare `TIME` is deliberately excluded — its edges are seconds-since-midnight,
70+
* not an epoch, and is handled by `isTimeOfDayType`.
7171
*/
7272
function isDatetimeType(type?: string): boolean {
7373
if (!type) return false;
@@ -81,8 +81,8 @@ function isDatetimeType(type?: string): boolean {
8181
* Time-of-day types (`TIME`, `TIME WITH/WITHOUT TIME ZONE`). The backend's
8282
* `epoch()` cast emits **seconds-since-midnight** (0–86399) for these, so the
8383
* edges must be read as a clock time, not a calendar date — otherwise every
84-
* tooltip collapses to "Jan 1, 1970" (DRC-3390 review note 1). Matches `time`
85-
* but not `timestamp`/`datetime`, which carry real epoch seconds.
84+
* tooltip collapses to "Jan 1, 1970". Matches `time` but not
85+
* `timestamp`/`datetime`, which carry real epoch seconds.
8686
*/
8787
function isTimeOfDayType(type?: string): boolean {
8888
if (!type) return false;
@@ -152,12 +152,12 @@ function toDiscreteData(
152152
trimmed: p.trimmed,
153153
};
154154
}
155-
// Counts mode. Stage B (DuckDB) only emits ranks, so this branch is inert
156-
// today — but it's a known future need, not speculation: Stage D's full
155+
// Counts mode. The DuckDB path only emits ranks, so this branch is inert
156+
// today — but it's a known future need, not speculation: full warehouse
157157
// adapters (Snowflake/BigQuery top-K return value+count pairs) emit counts,
158-
// and the cell conforms to the whole `run.ts` payload contract now so Stage D
159-
// is a backend-only change. A per-slot `null` means the value is absent from
160-
// that env's top-K — coerce to 0 so the bar simply doesn't render
158+
// and the cell conforms to the whole `run.ts` payload contract now so adding
159+
// them is a backend-only change. A per-slot `null` means the value is absent
160+
// from that env's top-K — coerce to 0 so the bar simply doesn't render
161161
// (gap-on-absent), which is exactly the cell's 0-height behavior.
162162
return {
163163
mode: "counts",

js/packages/ui/src/components/data/__tests__/InlineProfileDistributionCell.test.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
/**
22
* @file InlineProfileDistributionCell.test.tsx
3-
* @description Tests for the Stage C schema-grid cell container (DRC-3390).
4-
* Covers the state machine: loading / error / empty / per-column failure /
5-
* histogram / topk (ranks + counts).
3+
* @description Tests for the schema-grid cell container. Covers the state
4+
* machine: loading / error / empty / per-column failure / histogram /
5+
* topk (ranks + counts).
66
*/
77

88
import { render } from "@testing-library/react";
@@ -146,7 +146,7 @@ describe("InlineProfileDistributionCell", () => {
146146
it("formats TIME histogram edges as HH:MM:SS clock times, not dates", () => {
147147
// The backend's epoch() cast emits seconds-since-midnight for TIME, so the
148148
// tooltip must read as a clock time — never the bogus "Jan 1, 1970" a
149-
// calendar-date formatter would produce (DRC-3390 review note 1).
149+
// calendar-date formatter would produce.
150150
const timeHistogram: ProfileDistributionHistogramPayload = {
151151
...histogram,
152152
// 00:00:00, 01:00:00, 02:00:00, ... (whole hours past midnight)

js/packages/ui/src/components/schema/SchemaView.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,8 +240,8 @@ export function PrivateSchemaView(
240240
new Map(),
241241
);
242242

243-
// DRC-3390: per-model opt-in to profile *every* column rather than just the
244-
// changed ones. Keyed by the node id it was enabled for (not a bare boolean +
243+
// Per-model opt-in to profile *every* column rather than just the changed
244+
// ones. Keyed by the node id it was enabled for (not a bare boolean +
245245
// reset-in-effect) so switching nodes restores the changed-columns default on
246246
// the SAME render — the view isn't remounted per node, so a reset effect left
247247
// one stale render that fired a spurious all-columns run before it landed.

js/packages/ui/src/components/schema/selectInlineProfileScope.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
/**
22
* @file selectInlineProfileScope.ts
3-
* @description DRC-3390 Stage C — pure scoping logic for the inline paired
4-
* distribution in the schema view, extracted from `SchemaView` so the wiring
5-
* is unit-testable in isolation (no React, no lineage context).
3+
* @description Pure scoping logic for the inline paired distribution in the
4+
* schema view, extracted from `SchemaView` so the wiring is unit-testable in
5+
* isolation (no React, no lineage context).
66
*
77
* Given the change signals for a node and the user's "Profile all columns"
88
* opt-in, it decides which columns to profile and whether to profile at all.
@@ -90,7 +90,7 @@ export function selectInlineProfileScope({
9090
// Attribute impacted ids to this node by exact `<nodeId>_<column>` membership
9191
// over the node's own columns (shared with the grid's isImpacted painting) —
9292
// never by prefix-stripping the global set, which would mis-attribute a
93-
// sibling model's columns (DRC-3390 review #1; identity cleanup: DRC-3646).
93+
// sibling model's columns.
9494
if (impactedColumns && nodeId && nodeColumnNames) {
9595
for (const name of nodeColumnNames) {
9696
if (isColumnImpacted(nodeId, name, impactedColumns)) names.add(name);
@@ -105,11 +105,11 @@ export function selectInlineProfileScope({
105105
// specific columns, so any column's *values* may have shifted even when its
106106
// definition is untouched, and the changed-column subset would under-cover.
107107
//
108-
// COST NOTE (intended for Stage D — DRC-3631): combined with the run firing
109-
// on node-open, this means opening a whole-model-changed node profiles ALL
110-
// columns with no count cap. Cheap on the DuckDB-only Stage B path; against a
111-
// wide warehouse model this is an unbounded scan triggered by navigation. An
112-
// adapter-cost guard / column cap is deferred to the Stage D adapter work.
108+
// COST NOTE: combined with the run firing on node-open, opening a
109+
// whole-model-changed node profiles ALL columns with no count cap. Cheap on
110+
// the in-process DuckDB path; against a wide warehouse model this is an
111+
// unbounded scan triggered by navigation. A column cap / adapter-cost guard
112+
// is deferred to the warehouse-adapter work (tracked separately).
113113
const scopedColumns =
114114
profileAllColumns || wholeModelChange || !hasChangedScope
115115
? undefined

js/packages/ui/src/hooks/useInlineProfileDistribution.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { useApiConfig } from "./useApiConfig";
1313

1414
/**
1515
* @file useInlineProfileDistribution.ts
16-
* @description DRC-3390 Stage C — fire-and-parse hook for inline paired
16+
* @description Fire-and-parse hook for inline paired
1717
* distributions in the schema view.
1818
*
1919
* Gated end-to-end on the `inline_profile` server flag: when the flag is off
@@ -29,7 +29,7 @@ import { useApiConfig } from "./useApiConfig";
2929
* hand-rolled submit/poll loop. CAVEAT: the OSS fetch client sets no request
3030
* timeout (`useApiConfig.ts`), so a slow remote-warehouse adapter would hold
3131
* this request open with no client-side abort — there is NO timeout error or
32-
* Sentry capture for that case today (tracked in DRC-3629). Transport errors
32+
* Sentry capture for that case today (tracked separately). Transport errors
3333
* that do occur (network failure, non-2xx) surface as the hook's `error` state
3434
* and a Sentry capture via the queryFn's catch.
3535
*

js/packages/ui/src/utils/__tests__/cllColumnId.test.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
/**
22
* @file cllColumnId.test.ts
3-
* @description The CLL column-id grammar + safe membership (DRC-3390 review #1,
4-
* DRC-3646). Shared by the schema grid's isImpacted painting and the inline
5-
* profile-distribution scope.
3+
* @description The CLL column-id grammar + safe membership, shared by the
4+
* schema grid's isImpacted painting and the inline profile-distribution scope.
65
*/
76

87
import { describe, expect, it } from "vitest";

js/packages/ui/src/utils/__tests__/formatTime.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
/**
22
* @file formatTime.test.ts
33
* @description Covers `formatTimeOfDay` — the wall-clock formatter for
4-
* seconds-since-midnight (DRC-3390 review note 1). Its contract differs from
5-
* `formatDuration("compact")`: clock times always show two-digit hours, never
6-
* drop the hours component, and wrap at 24h.
4+
* seconds-since-midnight. Its contract differs from `formatDuration("compact")`:
5+
* clock times always show two-digit hours, never drop the hours component, and
6+
* wrap at 24h.
77
*/
88

99
import { describe, expect, it } from "vitest";

js/packages/ui/src/utils/cllColumnId.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,11 @@
55
* Column-level-lineage ids are `{node_id}_{column_name}` (mirrors the backend's
66
* `recce/util/lineage.py`). Because both halves can contain underscores and one
77
* node id can be a string-prefix of another, this id is NOT safely parseable by
8-
* splitting/prefix-stripping — that mis-attributes a sibling model's columns
9-
* (DRC-3390 review #1). Always go the other way: build the id from a KNOWN
10-
* (nodeId, column) pair and test membership. A fully-typed Node/Column identity
11-
* is tracked in DRC-3646; until then, route the join + membership through here.
8+
* splitting/prefix-stripping — doing so mis-attributes a sibling model's columns
9+
* (e.g. `model.shop.orders` absorbing a column of `model.shop.orders_summary`).
10+
* Always go the other way: build the id from a KNOWN (nodeId, column) pair and
11+
* test membership. A fully-typed Node/Column identity would remove the hazard
12+
* entirely (tracked separately); until then, route the join + membership here.
1213
*/
1314

1415
/** Construct a CLL column id from a node's `unique_id` and a column name. */
@@ -27,7 +28,7 @@ export function cllColumnId(nodeId: string, column: string): string {
2728
* for a column the node doesn't have can collide with a sibling's. Callers
2829
* iterate the node's own columns (the schema grid's merged rows, the profile
2930
* scope's `nodeColumnNames`), so this never arises in practice. The lossless
30-
* fix is typed identity (DRC-3646).
31+
* fix is typed identity.
3132
*/
3233
export function isColumnImpacted(
3334
nodeId: string,

0 commit comments

Comments
 (0)