Conversation
Counters only. No query text is stored and there is no per-request log
anywhere: what is kept is a set of integers keyed by (project, kind) and
(project, kind, file). Nothing in this commit is wired up yet; the
instrumentation and the HTTP surface land next.
WHY A SEPARATE SQLITE FILE
This is the load-bearing decision, and it is not general tidiness — it comes
straight out of two properties this server already promises:
1. Search must keep serving during a database compaction. That is encoded in
httpapi.readOnlyPostSuffixes, where `/search` is listed as a POST that may
proceed while writes are frozen. A counter written into projects.db would
either need an exemption from the freeze — committing into a snapshot that
is about to be discarded — or would block on the compactor's held write
transaction for the full busy_timeout, measured at 5.06 s in
dbmaint/freeze.go, while holding one of the eight connections that pool
allows. Eight of those and reads stall too. The same hazard already forced
Sessions.Touch to be skipped while frozen (httpapi/middleware.go); this
package sidesteps it by not being in that file.
2. The indexer owns the write lock in bursts. chunks_fts and chunks_meta are
written per file, in a transaction per file. A counter has no business
queueing behind a full reindex.
Two further reasons that only became visible while writing it: the bucket
tables are delete-heavy by design, and in projects.db that churn inflates the
free-list — which is exactly what dbmaint/stats.go computes the "time to
compact" verdict from, so analytics would start recommending maintenance
windows for the main database. And these are derived numbers about traffic
already served, so a file that can simply be deleted is worth having.
The cost of the split is that the counters cannot be JOINed against `projects`.
That turned out not to matter: the caller has to resolve which projects the
requester may see from the system database anyway (the same
access.AccessibleProjectHostPaths that workspace search uses), so every query
here is already parameterised by a set of project paths.
TWO TIERS, TWO RETENTION POLICIES
search_totals / search_file_totals are cumulative and NEVER pruned.
search_buckets / search_file_buckets carry a rolling 7-day window at 30-minute
resolution and ARE pruned.
They are separate tables rather than one bucketed table that gets summed,
because folding them together means choosing which one to break: deriving the
totals by summing the buckets makes the totals silently drop every time the
window slides, and keeping the buckets forever makes the file unbounded.
Neither table grows with the calendar — rows exist only where there was
activity — and the window tier is additionally capped at 336 buckets per key by
its retention.
SCHEMA NOTES THAT ARE EASY TO UNDO BY ACCIDENT
- project_path is interned into projects_seen. The strings are long (a local
project's key is `local:{machine_id}:{abs_path}`) and in a WITHOUT ROWID
table the primary key IS the row, so repeating it would put ~80 bytes into
every file row and every index key a scan walks. Interning also buys
ON DELETE CASCADE: discarding a removed project's counters is one DELETE.
- AUTOINCREMENT on projects_seen.id is load-bearing for the same reason it is
in the vector store. A plain INTEGER PRIMARY KEY is the rowid, and SQLite
hands the largest free rowid to the next insert — so deleting the
highest-numbered project would give its id to the next project recorded, and
a cached id would silently attribute one project's counters to another.
- `bucket` leads the primary key of both window tables. Pruning is
`DELETE ... WHERE bucket < ?`, which against this key order is a contiguous
range at the front of the b-tree rather than a full scan.
- The inner regroup by file_path in ProjectStatsPage is what makes
top_file_hits mean "the busiest FILE". On the windowed tier a file's hits are
split across buckets, so MAX(hits) taken straight off the table would report
the busiest half hour of the busiest file instead. There is a test for it.
RECORDING NEVER BLOCKS A SEARCH
Counters accumulate in memory and flush in one transaction every 10 seconds.
Record takes a mutex and writes to a map; it touches no database and returns no
error, because a counter that could fail is a counter that needs error handling
on the search path. On a flush failure the batch is DROPPED rather than
retried — retrying would hold a failed batch in memory while new counters
accumulate behind it, turning a transient disk error into unbounded growth.
The pending per-file map is capped at 100k distinct keys; past the cap file
detail is dropped and logged while query counts keep accruing. Analytics is
never allowed to be the reason the server runs out of memory.
The prune task is REGISTERED, not left as a helper. sessions.GC in this same
codebase carries a "safe to run periodically" comment and has no caller
anywhere, so expired sessions are never swept; that is the mistake this avoids.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Instruments all six search paths, adds three endpoints, and wires the store into main.go behind CIX_SEARCH_STATS_ENABLED (default true). WHAT IS COUNTED Every successful search records one query against the project plus one hit per file that appeared in the result. The six kinds — semantic, symbols, definitions, references, files, workspace — are stored separately rather than summed, because they cost wildly different things: a semantic query embeds text and scans a vector collection, a definition lookup is one indexed SELECT. A single "searches" number would average those together and mean nothing. Only successful searches count, and every call site sits after the response has been decided. A request refused for access, rejected for a malformed body, or failed at the embedding step did not search the project, and counting it would make the numbers a measure of client bugs. A search that legitimately returned NOTHING is counted, with no files — a project with many queries and few file hits is one whose index has stopped answering, and folding those away would hide exactly that. recordSearch DEDUPLICATES the file list, so a file counts once per search however many of its chunks matched. This is what keeps the two columns comparable: a file's hits can never exceed the project's query count, so a row reads as "this file came back in 42 of the project's 128 searches". Counting each matching chunk would let the number exceed the number of searches and the column would stop having an interpretation. Semantic search already groups by file so it is a no-op there; symbols/definitions/references return one row per match and genuinely repeat paths. WORKSPACE ATTRIBUTION — TWO POPULATIONS, DELIBERATELY The QUERY is recorded against every project the fan-out actually scanned. That work was paid for whether or not the project made the answer, and a repo carrying a busy workspace's traffic should not read as idle. FILE hits come only from `surviving` — the projects that cleared the relevance threshold — and specifically NOT from the display panel, which is truncated to the caller's top_projects parameter. Letting a request parameter decide what gets stored would make the numbers a measurement of how the caller configured their request. The same distinction is already load-bearing for projects_returned in this handler, where the comments record that it has been broken once and re-broken during a refactor. The call sits after the per-project chunk cap so the recorded files are the ones actually returned. ACCESS — GroupRead, and it matters more than usual GET /search-stats and GET /search-stats/series are list endpoints spanning projects, so there is no per-resource helper to lean on. They apply the gate the way ListProjects and WorkspaceSearch do: resolve the caller's accessible host_paths via access.AccessibleProjectHostPaths (admins skip the filter) and pass that set into the query, which has no other way to name a project. An empty set yields an empty table — the safe default if a future caller forgets to populate it. The counters carry FILE PATHS out of every project on the server, so an unscoped response would leak the directory structure of repositories the caller cannot open. `project_hash` on the series endpoint is resolved through the same accessible set rather than by direct lookup, so a hash the caller may not see is indistinguishable from one that does not exist — both 404. POST /admin/search-stats/reset is admin-only. The local docs/AUTH_REVIEW.md matrix has been updated alongside; gating tests cover 401 unauthenticated, 403 for a non-admin reset, and cross-user scoping of both reads. TWO THINGS THAT LOOK LIKE THEY COULD BE SIMPLER AND CANNOT - projects_without_activity is asked as its own query (ActiveProjectCount) rather than derived from the page. The page is both filtered and paginated, so counting the projects missing from it reports anything excluded by a min_queries filter as "never searched", which is a different and wrong statement. There is a test pinning this. - The row count and the footer sums wrap the SAME statement as the page, so the three can never disagree about what matched. Sorting by a computed aggregate and then paginating cannot be done in the caller without pulling every project's numbers across first, which is what "filters run on the server" exists to avoid. The sort key is looked up in a whitelist map — nothing from the query string ever reaches the statement text. An unrecognised `kinds` entry is DROPPED rather than rejected: the alternative is a 422 on a typo in a parameter that only ever narrows a read, and dropping keeps an older client talking to a newer server. GENERATOR SIDE EFFECT, CALLED OUT SO IT IS NOT A SURPRISE The new `window=all` enum collides with the existing `owner=all`, so oapi-codegen now prefixes both: openapi.All became openapi.ListApiKeysParamsOwnerAll. Nothing referenced the old bare name. Opening the stats database is NOT fatal at boot. These are derived numbers about traffic already served; refusing to start the server because a statistics file is unreadable would trade a working index for a chart. The recorder is stopped before the store is closed, because Stop drains what is still buffered and draining into a closed pool would discard the last interval on every clean shutdown. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
New module at /dashboard/search-stats: a filter bar, a totals strip, an activity chart, and the table itself — project, queries, distinct files, top files in results with their hit counts, last searched. Sorting and every filter round-trip to the server; nothing is filtered client-side, so `limit` bounds what is computed rather than merely what is displayed. Not admin-gated. The page is scoped to the projects the viewer can already search, so it tells them nothing they could not learn by searching. It sits at weight 26, directly after the two surfaces it reports on. DECISIONS THAT ARE NOT OBVIOUS FROM THE DIFF - Numeric filters are held as STRINGS in component state. A number would have to represent "empty" as something, and every candidate is a real filter value the user did not ask for: 0 filters out the projects with no searches, NaN needs special-casing at every use. Empty stays empty. - Free-text fields are debounced 300 ms; windows, sorts and kind chips apply at once, because a dropdown that lags behind the click reads as a broken control. Verified: typing "150" issues one request, not three. The debounce keys off a single joined string, not an object — an object rebuilt every render would restart the timer on every render and the request would never fire while anything else on the page re-rendered. - Every filter change resets to page 0. Staying on page 4 of a result set that just shrank to one page shows an empty table and reads as "no matches". - The activity chart draws BARS and fills the gaps itself. The series is a count per fixed interval, so a line between two counts implies values that were never measured; and the server omits empty buckets rather than sending zeroes, so plotting only what came back would compress a quiet weekend into nothing and silently rescale the time axis. A bucket with traffic never renders as nothing — 1px of bar is the difference between "one search" and "none", which is the distinction the chart exists to show. It anchors to the newest bucket that exists rather than to the clock, so the right edge is real data instead of a partial bucket that always reads as a drop-off. - The window selector offers no option longer than 7 days, because the bucket tier retains 7 days and there would be nothing behind it. `all` reads the cumulative tier; the chart falls back to 7d there, since the totals carry no buckets. - The per-file bar widths are relative to the project's OWN top file, so a quiet project's shape stays readable next to a busy one instead of collapsing to a sliver. - A 503 is rendered as "statistics are switched off" rather than the red "could not load" a real failure earns — it is a configuration state. Verified in the browser against a running server: sorting, filtering, the empty state, dark mode, and the request parameters on the wire. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
New doc/SEARCH_STATISTICS.md covering what is counted, the per-search (not per-match) definition of a file hit, workspace attribution, the two retention tiers, the access model, and the reasoning behind the separate database file. Also registers the feature on the three surfaces that go stale silently when a feature is added and only one of them is updated: the README documentation map, the CONFIG_REFERENCE env-var table (CIX_SEARCH_STATS_ENABLED, plus a note that there is deliberately no separate path variable — the two database files have to share a volume or the counters vanish on the next container restart), and the DASHBOARD page list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ed pages Four defects from review, one of which would hang the process on boot. 1. Recorder.Stop() deadlocked on a recorder that was never started. Stop closed r.stop then blocked on <-r.stopped, and r.stopped is only closed by loop(), which only runs if Start() was called. main.go registers the deferred Stop the moment the store opens (line 498) and calls Start 281 lines later (774), and FOUR fatal returns sit between them — bootstrap auth, two secrets failures, and the encryption-key mismatch. Any of them unwound into Stop and hung the process instead of exiting. The worst is the key mismatch: that error exists precisely to fail loudly on a wrong CIX_SECRET_KEY, and this turned it into a silent hang with no message. Stop now drains inline when the loop never ran, so counters buffered before an aborted boot are written rather than dropped. Start is guarded by a Once as well — a second loop would close r.stopped twice and panic. Regression test fails in 5 s against the old code. 2. search_file_buckets had no (project_id, bucket) index. EXPLAIN QUERY PLAN reported `SEARCH search_file_buckets USING PRIMARY KEY (bucket>?)` for both the files CTE and attachTopFiles: every per-project read on the windowed tier walked the whole retained range across all projects and filtered afterwards. Retention caps how far back that range goes, not how wide it is. One dashboard render at limit=25 issues about twenty-seven of these on a 30-second refresh. The comment that justified having only one index claimed it "covers the case anyway for the smaller of the two tables" — but the index was ON the smaller table, and the larger one had nothing. Both tables now carry the index, and a test asserts the plan uses it rather than trusting the comment. 3. The file was created with auto_vacuum=NONE, while the docs claimed INCREMENTAL. Measured: PRAGMA auto_vacuum returned 0. The mode can only be chosen while the header is unwritten, and the pool's DSN carries journal_mode=WAL, which writes it — so a pragma issued there is silently ignored however early it appears. Fixed the way internal/db already does it for the system database: a dedicated connection before the pool, on a file that does not exist yet. This one is worse than a doc error. The bucket tables are delete-heavy by design, so without a reclaim mode their pages went to the free list and stayed there — the file could only ever grow. That is exactly the free-list inflation this package cites as reason #3 for not living inside projects.db. Prune now runs PRAGMA incremental_vacuum after deleting, so the space actually returns. 4. The delete-cleanup test passed with the cleanup removed. It asserted through GET /search-stats, which is scoped to projects the caller can see — so a deleted project is absent whether or not its counters were discarded. Stubbing forgetSearchStats to a bare `return` left the test green. It now asserts against the store, and re-running that mutation fails it. The underlying hole was real, and removing a vacuous test does not close it: Forget is best-effort and runs after the delete has committed, so a failed call strands counters that NO api read can surface, in a tier that is never pruned. Added Store.ForgetAllExcept and a sweep on the prune schedule, so an orphan lives at most one night. An empty live-project list is treated as "don't know" and sweeps nothing — a server with zero projects exists, but so does a failed query, and orphans beat wiping every counter. ALSO FROM REVIEW - Removed `exists` from the API. It could never be false: the query is scoped to the same project set that fills the metadata map, so every returned row resolves. The field, the "deleted" badge in the table, and three paragraphs of documentation all described a state that could not occur. The self-healing sweep above is the honest version of what those docs promised. - Dropped Store.Path() and Store.FileBytes() — no callers, and Path()'s comment claimed the resources screen reported it while the docs say it does not. - Dropped Query.MinDistinct/MaxDistinct: wired into the SQL, reachable only from Go tests, no query parameter behind them. - Corrected the file_hits/results wording. It is not the integrity check the spec claimed — the buffer cap drops file detail while results keeps accruing, and empty paths are skipped on one tier and counted on the other. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…location
The counters sit on the critical path of the operation this server exists to
perform, and every search on the box takes the same mutex — so "does it slow
search down" and "does it collapse under concurrency" are two questions, and
neither had a number. Now both do, and the benchmarks are committed so the
answer can be re-checked rather than remembered.
Measured on a 14-core Mac, Go 1.26, -cpu=8, medians of 3 runs:
off on delta
file search, sequential 74.6 us 77.2 us +2.6 us (+3.5%)
file search, 8 concurrent 93.3 us 96.4 us +3.1 us (+3.3%)
The two percentages being the same is the result that matters: the overhead is a
constant per call, not contention that grows with load. The micro-benchmark
agrees from the other side — Record is 582 ns uncontended, 761 ns with every
core hammering a SINGLE project, and still 761 ns with a flusher running every
2 ms, which is 200x more often than production's 10 s. It allocates nothing.
+3.3% is measured against the cheapest endpoint that exists. File search is
~75 us, so a fixed few microseconds is a visible fraction of it. Against the
searches people actually wait for, using the latencies already measured on the
45-repo / 1.9M-chunk fixture in loadtests/SEARCH_PERF_CONTEXT.md:
single-project semantic 1,422 ms ~3 us 0.0002%
workspace, 45 repos 10,544 ms 26 us 0.0002%
Workspace search is the case with the most recording to do, one Record per
project the fan-out scanned, so its cost scales with the WORKSPACE and not with
the repositories in it: 4 us at 8 projects, 26 us at 45, 57 us at 100. Nothing
scales with repository size, because recording is driven by the result set,
which the caller's limit bounds.
The background flush costs ~12 ms for 200 projects x 10 files — far larger than
a 10-second window produces — and holds no lock a search needs: the pending maps
are swapped under the mutex and written outside it. The flat during-flush number
above is what that looks like from the search side.
One optimisation came out of the measurement. dedupePaths built a map, which for
twenty strings allocates ~1.5 kB and throws it away microseconds later, on every
search. Below 32 paths it now scans instead: +8 allocations and +1,910 B per
request became +1 allocation and +333 B. Result sets are bounded by the caller's
limit, so the linear branch is the one that runs. Both branches are tested,
including the boundary, and a test pins that the input is not modified in place —
every caller happens to pass a throwaway slice today, which would make an
in-place dedupe work now and trap the next call site.
A note on what was NOT measured: the 50k-file "heavy repo" fixture turned out not
to be heavy. LIMIT 20 stops the scan early, so it timed the same as the 500-file
one. The heavy-latency case is covered by the fixture figures above rather than
by a benchmark, and the file-search numbers stand as the pessimistic bound.
Also documents all of the above in doc/SEARCH_STATISTICS.md, and adds the three
endpoints to the site's API reference — the surface that goes stale silently.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…es not
Asked how the size of the statistics database affects the response, and
measured it instead of reasoning about it. Two different answers, and only one
of them was acceptable.
RECORDING IS UNAFFECTED BY SIZE — CONFIRMED, NOT ASSUMED
The intuition that a bigger database means slower writes means slower responses
is reasonable, and it is worth showing why it does not hold here rather than
asserting the architecture. Record writes to a map and returns; the flush runs
on a background goroutine that takes the shared mutex only to swap two map
pointers and does its I/O outside the lock.
Measured directly: a flush of 40,000 upserts into a 37 MB database, taking
284 ms, with 621,108 Record calls sampled entirely inside that window.
p50 p99 max
idle database 458 ns 87.6 us 331 us
during the flush 375 ns 81.5 us 600 us
Unchanged. The tail is mutex hand-off between eight goroutines, present with or
without a flush in flight. Across sizes, Record is flat at 292 ns from 3.7k rows
to 1.8M, and a flush stays near 1 ms — a b-tree upsert grows with the logarithm
of the table, not its size. Against a 10-second interval that is four orders of
magnitude of headroom before the writer could fall behind itself.
THE DASHBOARD DID SCALE WITH THE DATABASE, BADLY
The admin view aggregates every visible project, and an admin's scope is every
project on the server:
rows before after
3.7 k 5.1 ms 3.4 ms
90 k 109 ms 36 ms
450 k 542 ms 105 ms
1.8 M 2,267 ms 211 ms
At 1.8M rows that was a 2.3-second query on a page that auto-refreshes every 30
seconds. Two causes, both found by reading query plans and timing the halves
rather than guessing:
1. The aggregate ran TWICE. The row count and footer sums were a second
statement wrapping the identical CTE chain — measured at 519 ms for the pair
where each pass was ~260 ms. They are now window functions over the page's
own result set, so it is one pass. The property that made the wrapper worth
having is kept: the three figures still cannot disagree about what matched,
because they are still one query.
2. The per-file aggregate was unconditional. Computing file_hits,
distinct_files and top_file_hits for EVERY scoped project is only necessary
when the ORDER or a filter depends on them. The default view sorts by query
count, which lives in search_totals at one row per project — so those columns
are now computed for the ~25 projects on the page instead, in one statement.
Sorting by a file column still pays the full cost, correctly and by choice.
What remains scales with projects-on-the-page x files-per-project, not with the
size of the database.
The obvious hazard in (2) is that the two shapes drift and the same table starts
reporting different numbers depending on which column the user clicked.
TestFileAggregateShapesAgree pins them against each other across both tiers and
with and without a kind filter, including a project that was searched and
returned nothing.
ALSO FROM REVIEW ROUND 2
- Data race in my own benchmark. BenchmarkRecordParallel incremented a plain
int inside RunParallel, which every worker goroutine enters at once. It is
not only a race: the sub-benchmark exists to spread load over distinct keys
and show the mutex is not serialising, and workers reading the same value
pile onto one project and measure the contended case twice. It did not
surface in `go test -race ./...` because benchmarks need -bench to run. Now
an atomic; re-measured, projects=8 is 771 ns against projects=1 at 748 ns —
within noise of each other, which is the result the race was obscuring.
- dedupePaths' `len(files) < 2` shortcut returned the input untouched, so a
lone empty path survived a function whose comment says it drops them. The
single-element case is the only one where the shortcut and the loop could
disagree, so it is now spelled out.
- Removed a pointer to loadtests/SEARCH_PERF_CONTEXT.md from two tracked files.
/loadtests/ is gitignored, so no clone has it and neither reference could be
followed. The fixture's shape — 45 repos, 1.9M chunks — is what travels, and
it is now stated inline.
- Commits scale_test.go, the harness that produced the size curve above. It was
written during the previous round and left untracked; it is skipped unless
CIX_SCALE_TEST=1 because it populates up to a million rows.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
One regression from the previous round, plus the structural work to stop the
same class of mistake happening quietly.
TOTALS LIED WHEN OFFSET WAS PAST THE END
Moving the row count and footer sums onto window functions over the page's own
rows made them unreachable when the page had no rows:
offset=0 rows=2 Total=5 TotalQueries=5
offset=99 rows=0 Total=0 TotalQueries=0
Five projects matched and the response said zero, contradicting what the spec
documents `total` to be — the count BEFORE limit and offset — and what
SearchStatsTotals says it sums. The previous shape got this right for free,
because the count was its own statement.
The comment I left arguing the caller "has already seen the size of" the set was
a rationalisation, and it was wrong twice over. Any caller constructing a URL
with an offset (a script, a bookmark, a retry) cannot tell "past the end" from
"nothing matched". And the dashboard polls every thirty seconds: sit on page
four, have the set shrink underneath you, and the next poll would report
total=0, collapse the pager, and render "nothing recorded yet" over projects
that are still there.
Fixed by re-reading the window values from a single row at offset 0 when an
offset request comes back empty — the same statement with different bounds, not
a second count query, so the two still cannot disagree about what matched. It
costs a query only on a page that was already empty. A filter that genuinely
matches nothing still reports zeroes, and there is a test for each case.
STOPPING THE CONDITIONAL AGGREGATE FROM DRIFTING
needsFileAggregate decides whether the expensive per-project file aggregate
runs. Adding a file-derived sort key or filter without teaching it would not
raise an error — it would serve the page from the stub CTE and return zeros in
the very columns the request was ordered by. Wrong numbers, silently.
It duplicated knowledge held in two other places, so it no longer does: the
range filters are one list that both the WHERE clause and the predicate are
built from, with a `fromFiles` flag that is the only thing the predicate reads;
and TestFileDerivedSortsMatchSortColumns pins fileDerivedSorts against
sortColumns in both directions.
ORDERING NO LONGER RELIES ON A SUBQUERY'S ORDER SURVIVING
The rewrite left ORDER BY only on the inner subquery. SQL does not guarantee
that ordering reaches the consumer; SQLite preserves it here because the window
functions block flattening, and it was verified across every sort key and
direction — but that is a property of today's planner, not of the query. The
outer statement now repeats it, which the planner collapses. This forced
sortColumns onto output column names rather than table-qualified ones, since
only the inner scope can see the source tables; a test runs every sort key in
both directions to keep that true.
TESTS THAT DID NOT PIN WHAT THEY CLAIMED
TestFileAggregateShapesAgree ran windows of {0, retention} against data seeded
two hours back, so every row fell inside every window: the bucket predicate in
fillFileAggregates was present but never excluded anything, and deleting it
changed no output. Added a one-hour window, and confirmed by mutation that
dropping the filter now fails loudly. Also added a limit=2 case against three
matching projects, so the page is a strict SUBSET of the matched set — the only
shape where filling the file columns per page can disagree with computing them
for everything.
THE EXPENSIVE PATH NOW HAS A NUMBER
Sorting by a file column is the one request that cannot be served from the page
alone, and the docs said so without saying what it costs. Measured rather than
extrapolated:
rows default sort sorted by a file column
90 k 36 ms 63 ms
450 k 105 ms 295 ms
1.8 M 211 ms 1,176 ms
~1.2 s is a fair price for a click. It is not a fair price every thirty seconds
for a tab left open on that sort, so the dashboard's auto-refresh backs off to
two minutes when the sort or a filter is file-derived. Poll frequency follows
query cost.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… page
Collection is now OFF by default and turned on either from the statistics page
or, for a fleet, at deploy time. Previously it was on for anyone who upgraded,
which is the wrong default for a feature that records what people search for —
even as counters, that is a decision somebody should make rather than inherit.
TWO WAYS TO ASK, IN A DELIBERATE ORDER
1. The switch on the statistics page (admin only). Immediate, no restart.
2. CIX_SEARCH_STATS_ENABLED=true at deploy time — the starting position for
servers that should collect from their first boot.
A stored decision OUTRANKS the environment, and that ordering is the whole
point: an admin who turns collection on in the dashboard must not have it turned
off again by the next container start carrying the old environment. The variable
gives a server its starting position; it does not re-assert itself forever.
The resolution is therefore: saved decision, then environment, then off. The
config table deliberately seeds NO row — an absent row is the "nobody has
decided" state that lets the variable still speak, and inserting a default would
silently pin every install to it and make the variable dead on arrival. The API
reports which of the three is speaking, because "off" means something different
when somebody chose it than when nobody has.
A LIVE SWITCH MEANS A HOLDER, NOT A BOOLEAN
The straightforward implementation is a flag checked inside Record, with the
database opened at boot regardless. That would leave a server with the feature
switched off still carrying a database file — a surprising thing to find on disk
after turning something off — and would make "off" a property of the write path
rather than of the feature.
So searchstats.Holder owns the store and recorder, and off means the file is
closed and, on a server that never enabled it, never created. Verified: a fresh
server with no environment variable logs `search statistics are off
source=default` and creates no searchstats.db.
Reads on the search path go through an atomic pointer rather than the holder's
mutex. Recording is the one thing here that must not queue behind an
administrator clicking a toggle, and an RWMutex read lock — cheap as it is — is
still a contended cache line on every search. The mutex serialises only the
transitions, which are rare and slow. Recorder's methods are nil-safe, so a
search racing a Disable is a no-op rather than a panic.
Enable publishes the store before the recorder and Disable retires them in the
reverse order, so nothing can record into a store the endpoints cannot see, or
into one that is about to close. Disable DRAINS rather than discards: switching
the feature off is not a request to lose the last few seconds it collected, and
those counters are what the dashboard shows if it is switched back on. There is
a test for exactly that.
The maintenance task is registered whether or not the feature is on, and
resolves the store on each run. A task bound to whatever was open at boot would
stop working the moment an admin toggled the feature, and re-registering on each
toggle would mean carrying an admin's saved schedule across — for a task that is
a no-op while the feature is off anyway.
A DEFECT THE LIVE TOGGLE EXPOSED IN THE PAGE
With collection off the two data queries were still issued, got 503s, and sat in
an error state. Invalidating on the toggle refetched the settings but left the
errored table where it was, so switching collection on showed an empty table
over a database that had rows in it — reproduced in the browser. They are now
gated on the switch, which is both correct and cheaper: a page load with
collection off issues one request instead of three plus retries. Verified on the
wire, before and after the toggle.
Applying comes BEFORE persisting in the PUT handler. If opening the database
fails, the stored setting must not claim the feature is on — an admin who sees
the switch stay off with an error explaining why is better served than one who
sees it flip and nothing happen.
Reading the setting is SelfAuth, not Admin: the statistics page has to be able
to explain why it is empty, and "the feature is off" is not a secret. Changing
it is Admin. Both are gated in tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The headline is a defect the new tests found while being written for something else, and it is the exact failure the conditional aggregate was supposed to be guarded against. A BARE COLUMN NAME IN WHERE IS NOT THE SELECT LIST'S ALIAS The range filters compared against `file_hits` and `top_file_hits`, reading like the output columns two lines above them. SQLite resolves an identifier in WHERE against the source relations FIRST and only then considers the SELECT list, so those bound to `files.file_hits` — the un-coalesced column of a LEFT JOIN — and not to the `COALESCE(..., 0)` the row actually carries. For any project that was searched and returned nothing, that column is NULL. `NULL <= 2` is NULL, not true, so `max_file_hits` silently excluded exactly the projects whose indexes have stopped answering: the ones the filter exists to find. ORDER BY behaves the opposite way — it prefers the alias — which is why sorting looked right while filtering did not, and why nothing caught it. Filters now carry the COALESCE explicitly, with the reason written down. THE SEAM WAS UNPINNED, AND THAT IS HOW THE ABOVE SURVIVED `fromFiles` is the only thing needsFileAggregate reads about filters, and dropping it from either entry passed the ENTIRE suite — while making every file-column filter report "nothing matches" over data that does, because the bounds were then evaluated against the stub aggregate's zeros. Wrong numbers, no error, green tests. TestFileDerivedFiltersRequireTheAggregate now asserts the flag from the same list the WHERE clause is built from, and fails if a new entry sits in neither camp. TestFileDerivedFiltersSelectTheRightProjects runs the cheap-vs-full agreement driven by a filter rather than a sort key — and it is the test that found the NULL bug, because it included a project that was searched and returned nothing. ANY AUTHENTICATED USER COULD READ AN ADMIN'S EMAIL GET /search-stats/settings is deliberately readable by everyone: the page has to be able to explain why it is empty. But `enabled` and `source` are that explanation, and the payload also carried `updated_by` — an admin's email address — and `updated_at`, when this server was last administered. Neither answers "why is this page empty". Both are now admin-only; the fields are optional in the schema, so the wire contract is unchanged. STOPPING COLLECTION AND DISPOSING OF IT WERE THE SAME LEVER Switching collection off closes the database, so the reset endpoint 503'd and the dashboard hid the Clear counters button. An admin who stopped collecting could then neither view nor delete what had been collected without switching collection back on — resuming the very thing they had stopped. On an opt-in feature that records what people search for, that is the wrong shape for the consent path. Holder.Reset now works either way, opening the file on demand when collection is off and closing it again. A server that never enabled the feature has nothing to discard and gets a 204 without a database being created. The button stays visible while off, and the empty state says what "off" means for what was already collected instead of implying the counters are readable. A FAILED SAVE LEFT THE RUNTIME DISAGREEING WITH THE ANSWER The PUT applies before persisting, which is right for the case it was written for: a failed Open must not leave a stored setting claiming the feature is on. The mirror case was wrong. Apply succeeds, persist fails, the admin gets a 500 — and the server is now collecting while both the storage and the admin believe it is not, until a restart quietly reverts it. Clicking "on", seeing an error and concluding nothing happened should not be a mistake. The holder is rolled back before the 500 so the error tells the truth. SMALLER, ALL FROM REVIEW - HolderTasks built a zero-value Store purely to harvest task metadata, then overwrote the one field that would have used it. It worked only because nothing outside the handler read the receiver; the day a Description names the database path it is a nil-pointer panic at boot with nothing in the type system to warn anyone. Both constructors now share pruneTaskMeta(). - The settings query is polled. Without it a second admin sits on "not collecting" over a server that has been collecting for an hour. - Documented the invariant that actually makes the Holder's races safe: it is not the nil check but that Record never touches a database, so a search that loads the pointer just before Disable swaps it away writes to a map on a stopped recorder rather than using a closed pool. If Record ever grows a database call, the atomic pointer will not save it. - Narrowed Disable's "always drains" docstring, which does not hold in the sliver after the background context is cancelled. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… hidden
Two edges from review, neither blocking, both cheap to close properly.
THE PRUNE TASK RESOLVED THE STORE AND THEN USED IT
store := h.Store()
if store == nil { return nil }
return store.maintain(ctx, ...) // a Disable lands here
An admin toggling collection off between those lines turned a routine prune into
`sql: database is closed` and a red row in the scheduled-tasks list. Nothing
worse — no corruption, and the next night's run is fine — and the window is
genuinely tiny, since the task runs once a day for milliseconds. But the task
already meant to skip while the feature is off; it just decided half a line too
early.
Holder.WithOpenStore now holds the toggle mutex for the duration of the run, so
the store cannot close underneath it. Holding a mutex across work is a trade
rather than a reflex, and the comment says why it is the right one here: a prune
is milliseconds once a day, and an admin's toggle waiting for it beats a failed
task. The method carries a warning not to reach for it on anything that takes
real time, because it blocks Enable and Disable outright.
The regression test races 200 maintenance runs against a goroutine flipping the
feature on and off, and fails against the old code on the first iteration.
DISPOSAL WAS HIDDEN ON A SERVER NOBODY HAD TOGGLED
The Clear counters button was keyed on the setting's PROVENANCE — visible when
collection was on, or when an admin had made a decision. That misses the server
enabled purely by CIX_SEARCH_STATS_ENABLED which collected data and was then
redeployed with the variable flipped: nobody ever touched the toggle, so the
source still reads `environment`, the file is full, and the button is gone. The
same "cannot dispose without resuming" corner as the last round, reached from
the other direction.
Provenance was a proxy for the fact, and a leaky one. The settings endpoint now
reports `has_stored_counters` — whether a database exists — and the dashboard
keys on that. The empty state distinguishes the two cases it was conflating:
counters are on disk but unreadable while off, versus nothing has ever been
recorded here.
ON THE NULL BUG FROM THE LAST COMMIT
Review reproduced it independently and found it one notch broader than reported:
`min_file_hits=0` dropped rows too. A lower bound of zero is semantically a
no-op — every project has at least zero file hits — and it silently removed a
row that the unfiltered table renders as `0`. `NULL >= 0` is NULL the same as
`NULL <= 2` is. The COALESCE fix covers it structurally, and the query plans
confirm the cause rather than the symptom: before, the plan for a file-filtered
request lacked the LEFT-JOIN tag entirely, because a bare `file_hits >= ?` on
the right-hand relation is NULL-rejecting and SQLite legally strength-reduced
the outer join to an inner one, discarding the unmatched rows before the WHERE
clause ever saw them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…min-only Round 4 gave this endpoint a rule when it restricted updated_by/updated_at: `enabled` and `source` are the whole of why a page is empty, and that is the entire justification for the endpoint being readable by everyone. Round 5 then added `has_stored_counters` above the admin check, which quietly widened it again — whether there is a file to dispose of is operator information, not part of the explanation, and no control a regular user has acts on it. The dashboard reads the field only inside admin-gated branches, so restricting it changes nothing on screen. It leaks nothing about content and a non-admin could infer it from whether the table has rows, so this is consistency rather than a hole. But a rule that is applied to two fields and not the third stops being a rule, and the next field added to this payload should have something to be measured against. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Unrelated to this branch — develop carries the same stale constant and the same red check. Fixed here because the PR cannot go green otherwise. server/v0.14.1 was tagged on main and its release workflow completed, so the images are published; only the last step of the release flow, bumping the site's constant, was skipped. versions.js is hand-maintained precisely so the site can never advertise a version that is not tagged, and the CI gate compares against the NEWEST tag rather than merely checking the version exists — which is what caught this. The other three streams were already current (cli 0.10.2, mac 0.1.1, plugin 0.4.0 against plugin.json), so this is the only drift. Verified by replaying the gate's own comparison locally, and the site builds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
feat(server): per-project search statistics
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // reverted it. Put the runtime back so the error tells the truth. | ||
| if _, rerr := s.Deps.SearchStats.Set(!body.Enabled); rerr != nil { | ||
| s.Deps.Logger.Error("search statistics: could not roll back after a failed save", | ||
| "wanted", body.Enabled, "err", rerr) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promotes
developtomainsoserver/v0.15.0can be tagged here.What ships
Per-project search statistics (#278) — how often each project is
searched and which of its files keep coming back in the results, with a
dashboard page that filters and sorts server-side.
Counters only: no query text is stored and there is no per-request log. What is
kept is a set of integers keyed by
(project, kind)and(project, kind, file).Opt-in. Collection is off by default. An admin turns it on from the
statistics page — effective immediately, no restart — or an operator sets
CIX_SEARCH_STATS_ENABLED=trueat deploy time to start a fleet collecting. Astored decision outranks the environment, so a redeploy cannot silently undo it.
The counters live in their own SQLite file next to
projects.db, because searchmust keep serving while the system database is frozen for a compaction, and a
counter has no business queueing behind the indexer's write lock. Recording is
in-memory and flushed in batches: measured at +3.3% on the cheapest search
endpoint and 0.0002% of a workspace query, with no degradation under concurrency.
New endpoints, all documented in the spec:
/api/v1/search-stats/api/v1/search-stats/series/api/v1/search-stats/settings/api/v1/admin/search-stats/settings/api/v1/admin/search-stats/resetAlso carries a site fix:
versions.jswas advertising server 0.14.0 after0.14.1 shipped.
Release checks
-race;go vetclean;openapi.gen.goinsync with the spec; dashboard typecheck and build clean.
HIGH/CRITICAL against
v0.14.1-cu128, and one fewer (a libssl fix landedupstream).
cli/v0.10.2is reachable from this commit, which the macOS runtime jobrequires.
Docs:
doc/SEARCH_STATISTICS.md.🤖 Generated with Claude Code