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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- **`time_range.since`/`until` accept day/week suffixes (`7d`, `2w`) and reject unparseable values with a 400 instead of an opaque ClickHouse error** (`internal/query/builder.go`, `internal/query/builder_test.go`, `docs/src/content/docs/api.md`, `docs/src/content/docs/sdk.md`): closes #285. Go's `time.ParseDuration` stops at hours, so a documented `"7d"` (sdk.md) failed to parse and fell through to ClickHouse as a raw `DateTime` literal → opaque `Cannot read DateTime` error (surfaced as a retryable 500, per #271). `resolveTimeValue` now pre-expands `Nd`→`N*24h` / `Nw`→`N*168h` (Prometheus/ClickHouse-style) before parsing, and any value that is neither a relative duration nor an RFC3339 timestamp now fails closed — the builder maps it to a 400 with a clear message rather than handing the string to ClickHouse.
- **development.md & CONTRIBUTING.md no longer describe the pre-consolidation CI / review automation** (`docs/src/content/docs/development.md` §CI & review automation, `CONTRIBUTING.md`, `Makefile` comments): the contributor docs still referenced `pr-title.yml`/`Validate`, `Check`/`Build` status checks, `project-orchestrator.yml`, and `label.yml` — all replaced or deleted by the May 2026 workflow consolidation (`housekeeping.yml`) and #115 — and described card movement and reviewer re-requests the orchestrator no longer performs. Rewritten against the live config: required checks are `CI` (one job running `make ci`), `PR housekeeping` (Conventional Commits title + the previously undocumented 72-char cap, file-path labels via the labeler step, reviewer assignment on open/ready), and `Admin approval`; board placement is native Projects v2 workflows; the stale "Lint/Test not required (#57)" note is gone (those suites run inside the required `CI` check). Also corrects a misattribution this branch had propagated: `starlight-links-validator` does not use Chromium — only `rehype-mermaid` does (fixed in the prose and two `Makefile` comments), and `make clean`'s help text/targets table now mention `docs/.dev-dist/`. Same sweep, code-comment side: `admin-approval.yml`'s header claimed the ruleset's `required_approving_review_count` was `0` — the live ruleset says `1` (any collaborator), with the workflow's status check being what makes one of those approvals specifically an admin's — and AGENTS.md's `make clean` one-liner now lists the docs build outputs.

- **Theme-adaptive favicon that actually displays** (`docs/astro.config.mjs`, `docs/src/components/Head.astro`, `docs/scripts/branding/generate.sh`, `docs/public/branding/favicon-{light,dark}.svg` (new)): the CSS-swap `favicon.svg` has carried a `prefers-color-scheme` flip since the branding pipeline landed, but no browser ever showed it — the docs final pass (#193) dropped the explicit `sizes="any"` SVG icon link, so Chromium selected the static `.ico`, and Chromium rasterizes an SVG favicon only once regardless (crbug.com/1208277). The head now declares the numeric-sized `.ico` + `sizes="any"` SVG pair, the brand kit gains statically-colored `favicon-{light,dark}.svg` variants (two new copy-out assets), and a `Head.astro` inline script live-swaps them on `prefers-color-scheme` changes, GitHub-style: replacing the link node each repaint (in-place `href` mutation is honored only transiently) and removing the `.ico` from the live candidate list, since Chromium's favicon scorer otherwise commits an exact-size `.ico` over a `sizes="any"` SVG (verified against the browser's profile `Favicons` DB rather than blog lore). No-JS consumers keep the full SSR markup (CSS-swap `favicon.svg` + `.ico`); Safari ≤18, which can't use SVG favicons, falls back to the root `/favicon.ico` auto-probe that the brand pipeline keeps in place. Supersedes the favicon wiring described in the branding-pipeline entry below (`favicon:` option + `.ico`/apple-touch head tags only).
Expand Down
4 changes: 2 additions & 2 deletions docs/src/content/docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ Executes a type-safe structured query against a table. The query AST is validate
| `group_by` | string[] | No | GROUP BY columns. |
| `order_by` | object[] | No | ORDER BY clauses (`column`, `dir`). |
| `limit` | int | No | Max rows. |
| `time_range` | object | No | Time window (`column`, `since`, `until`). `since` can be relative ("1h", "30m") or RFC3339. |
| `time_range` | object | No | Time window (`column`, `since`, `until`). `since` can be relative ("1h", "30m", "7d", "2w") or RFC3339. |

**Response:**

Expand All @@ -426,7 +426,7 @@ JSON array of result rows. The response carries an `X-Cache: HIT` or `X-Cache: M

| Status | Body | Cause |
| ------ | ---- | ----- |
| 400 | `{"error":"..."}` | Schema validation error (unknown column, bad aggregation) |
| 400 | `{"error":"..."}` | Schema validation error (unknown column, bad aggregation, or an unparseable `time_range` `since`/`until` — neither a relative duration nor an RFC3339 timestamp) |
| 403 | `{"error":"forbidden"}` | Role lacks select permission on table |
| 403 | `{"error":"column \"x\" not allowed"}` | Column denied by policy |
| 403 | `{"error":"aggregation \"x\" not allowed"}` | Aggregation fn denied by policy |
Expand Down
2 changes: 1 addition & 1 deletion docs/src/content/docs/sdk.md
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ If no limit is specified, `QueryBuilder.DEFAULT_LIMIT` (1000) is applied automat

#### `.timeRange(column, since, until?)`

Filter by a time window. `since` accepts RFC3339 timestamps or relative durations (`'1h'`, `'30m'`, `'7d'`).
Filter by a time window. `since` accepts RFC3339 timestamps or relative durations (`'1h'`, `'30m'`, `'7d'`, `'2w'`).

```ts
clicks.select('page').timeRange('received_timestamp', '1h')
Expand Down
2 changes: 1 addition & 1 deletion internal/query/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ type OrderClause struct {
// TimeRange constrains a query to a time window.
type TimeRange struct {
Column string `json:"column"` // timestamp column name
Since string `json:"since"` // RFC3339 or relative (e.g. "1h", "30m")
Since string `json:"since"` // RFC3339 or relative (e.g. "1h", "30m", "7d", "2w")
Until string `json:"until"` // RFC3339 or empty (=now)
}
62 changes: 50 additions & 12 deletions internal/query/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,18 @@ func buildWhere(filters []Filter, timeRange *TimeRange, bucketSeconds int) ([]st
}

if timeRange != nil && timeRange.Column != "" && timeRange.Since != "" {
sinceTime := resolveTimeValue(timeRange.Since, bucketSeconds)
sinceTime, err := resolveTimeValue(timeRange.Since, bucketSeconds)
if err != nil {
return nil, nil, fmt.Errorf("time_range since: %w", err)
}
parts = append(parts, fmt.Sprintf("%s >= ?", timeRange.Column))
params = append(params, sinceTime)

if timeRange.Until != "" {
untilTime := resolveTimeValue(timeRange.Until, bucketSeconds)
untilTime, err := resolveTimeValue(timeRange.Until, bucketSeconds)
if err != nil {
return nil, nil, fmt.Errorf("time_range until: %w", err)
}
parts = append(parts, fmt.Sprintf("%s <= ?", timeRange.Column))
params = append(params, untilTime)
}
Expand Down Expand Up @@ -258,25 +264,57 @@ func formatClickHouseTime(t time.Time) string {
return t.UTC().Format(clickHouseDateTimeLayout)
}

// dayWeekRe matches a duration component with a day ("d") or week ("w") unit —
// the two units time.ParseDuration rejects (it stops at hours). The magnitude
// may be fractional, e.g. "0.5d".
var dayWeekRe = regexp.MustCompile(`(\d+(?:\.\d+)?)([dw])`)

// expandDayWeek rewrites the day/week components of a duration string into hours
// so time.ParseDuration accepts them: "7d" → "168h", "2w" → "336h", "1d12h" →
// "24h12h" (ParseDuration sums repeated units). Components in units it already
// understands are left untouched, and a string with no day/week component is
// returned verbatim. This keeps the documented "7d"-style ranges working
// (sdk.md) without reimplementing duration parsing. RFC3339 timestamps contain
// no lowercase d/w, so they pass through unchanged.
func expandDayWeek(s string) string {
return dayWeekRe.ReplaceAllStringFunc(s, func(m string) string {
sub := dayWeekRe.FindStringSubmatch(m)
n, err := strconv.ParseFloat(sub[1], 64)
if err != nil {
return m // regex guarantees a numeric sub[1]; never corrupt on surprise input
}
hoursPerUnit := 24.0
if sub[2] == "w" {
hoursPerUnit = 168.0
}
return strconv.FormatFloat(n*hoursPerUnit, 'f', -1, 64) + "h"
})
}

// resolveTimeValue parses an RFC3339 timestamp or a relative duration like "1h",
// "30m" and renders it as a ClickHouse DateTime literal (see formatClickHouseTime).
// When bucketSeconds > 0, timestamps are bucketed (truncated) to the nearest
// boundary. An unrecognised value is returned unchanged, which will likely
// cause a ClickHouse error.
// "30m", "7d" or "2w" and renders it as a ClickHouse DateTime literal (see
// formatClickHouseTime). When bucketSeconds > 0, timestamps are bucketed
// (truncated) to the nearest boundary.
//
// A value that is neither a duration nor a timestamp is rejected with an error
// (which the builder surfaces as a 400) rather than returned unchanged: passing
// a raw string to ClickHouse surfaces as an opaque DateTime parse error (#285).
//
// The output deliberately matches coerceFilterValue's format rather than RFC3339:
// a bare "…T…Z" string is rejected by DateTime64 columns.
func resolveTimeValue(val string, bucketSeconds int) string {
// Try relative duration first (e.g., "1h", "30m", "5m").
if d, err := time.ParseDuration(val); err == nil {
return formatClickHouseTime(bucketTime(time.Now().UTC().Add(-d), bucketSeconds))
func resolveTimeValue(val string, bucketSeconds int) (string, error) {
// Try a relative duration first (e.g., "1h", "30m", "7d", "2w"). Go's
// time.ParseDuration only understands units up to hours, so day/week
// suffixes are pre-expanded to hours.
if d, err := time.ParseDuration(expandDayWeek(val)); err == nil {
return formatClickHouseTime(bucketTime(time.Now().UTC().Add(-d), bucketSeconds)), nil
}
// Try an absolute timestamp (RFC3339Nano accepts fractional and whole-second
// input); normalise to UTC before bucketing.
if t, err := time.Parse(time.RFC3339Nano, val); err == nil {
return formatClickHouseTime(bucketTime(t.UTC(), bucketSeconds))
return formatClickHouseTime(bucketTime(t.UTC(), bucketSeconds)), nil
}
return val
return "", fmt.Errorf("invalid time value %q: want an RFC3339 timestamp or a relative duration such as \"1h\", \"30m\", \"7d\", \"2w\"", val)
}

// bucketTime truncates a time to the nearest bucket boundary.
Expand Down
70 changes: 67 additions & 3 deletions internal/query/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,8 @@ func TestBuild_InvalidGroupByColumn(t *testing.T) {

func TestResolveTimeValue_RelativeDuration(t *testing.T) {
t.Parallel()
result := resolveTimeValue("1h", 0)
result, err := resolveTimeValue("1h", 0)
require.NoError(t, err)
assert.NotEmpty(t, result)
assert.NotEqual(t, "1h", result, "relative duration should resolve to a timestamp")

Expand All @@ -249,17 +250,70 @@ func TestResolveTimeValue_RelativeDuration(t *testing.T) {
func TestResolveTimeValue_RFC3339(t *testing.T) {
t.Parallel()

result := resolveTimeValue("2024-01-01T00:00:00Z", 0)
result, err := resolveTimeValue("2024-01-01T00:00:00Z", 0)
require.NoError(t, err)
assert.Equal(t, "2024-01-01 00:00:00", result)
}

func TestResolveTimeValue_WithBucketing(t *testing.T) {
t.Parallel()
// With 60s buckets, a time at :30 should truncate to :00.
result := resolveTimeValue("2024-01-01T12:34:30Z", 60)
result, err := resolveTimeValue("2024-01-01T12:34:30Z", 60)
require.NoError(t, err)
assert.Equal(t, "2024-01-01 12:34:00", result)
}

func TestExpandDayWeek(t *testing.T) {
t.Parallel()
cases := map[string]string{
"7d": "168h", // documented day suffix (#285)
"1d": "24h",
"2w": "336h",
"1w": "168h",
"0.5d": "12h", // fractional magnitude
"1d12h": "24h12h", // ParseDuration sums repeated units
"1h": "1h", // already a unit ParseDuration handles — untouched
"30m": "30m",
"": "", // no component — verbatim
}
for in, want := range cases {
assert.Equal(t, want, expandDayWeek(in), "expandDayWeek(%q)", in)
}
}

func TestResolveTimeValue_DayWeekSuffix(t *testing.T) {
t.Parallel()
// "7d"/"2w" are documented (sdk.md) but not Go durations; they must resolve
// to a real ClickHouse DateTime, not fall through as the raw string (#285).
for _, in := range []string{"7d", "2w", "1d12h"} {
result, err := resolveTimeValue(in, 0)
require.NoError(t, err, "resolveTimeValue(%q)", in)
assert.NotEqual(t, in, result, "%q must resolve, not pass through raw", in)
assert.NotContains(t, result, "T")
assert.NotContains(t, result, "Z")
}

// "7d" must resolve to the same instant as its hour-equivalent "168h".
// Bucket to the day so the sub-millisecond gap between the two time.Now()
// reads can't make the comparison flaky.
day := 86400
d7, err := resolveTimeValue("7d", day)
require.NoError(t, err)
h168, err := resolveTimeValue("168h", day)
require.NoError(t, err)
assert.Equal(t, h168, d7, `"7d" and "168h" should resolve to the same bucketed time`)
}

func TestResolveTimeValue_Invalid(t *testing.T) {
t.Parallel()
// Neither a duration nor a timestamp: must fail closed (→ 400) instead of
// reaching ClickHouse as a raw literal (#285).
for _, in := range []string{"7dd", "banana", "7 days", "168", "7D"} {
_, err := resolveTimeValue(in, 0)
require.Error(t, err, "resolveTimeValue(%q) should error", in)
}
}

func TestBucketTime_ZeroBucket(t *testing.T) {
t.Parallel()
ts, _ := time.Parse(time.RFC3339, "2024-01-01T12:34:56Z")
Expand Down Expand Up @@ -378,6 +432,16 @@ func TestBuild_InvalidColumns(t *testing.T) {
},
wantErr: "unknown column",
},
{
// Valid column, unparseable duration: must fail closed at Build
// (→ 400) rather than reach ClickHouse as a raw literal (#285).
name: "invalid time range since duration",
sq: &StructuredQuery{
Columns: []string{"page"},
TimeRange: &TimeRange{Column: "ts", Since: "banana"},
},
wantErr: "invalid time value",
},
}
for _, tt := range tests {
tt := tt
Expand Down
Loading