Conversation
…r, and latency benchmarks - page_visits table tracks navigation events with upsert (one row per page); visitRecencyMultiplier (3-day half-life, ×2.0 at t=0) blended into buildRankedList - StelekitViewModel fires recordPageVisit fire-and-forget on every navigateTo - promoteExactTitleMatch promotes exact-name-match (case-insensitive) to position 0 - pages_ai FTS trigger fixed: last_insert_rowid() → new.rowid - rebuildFts() / integrityCheckFts() added to SearchRepository and SqlDelightSearchRepository - Batch selectPageVisitsByUuids lookup eliminates N+1 in ranking hot path - InstrumentedSearchRepository wired with OTel span attrs (ranking.visit_boost, result.ranked.count) - SearchLatencyTest: p99 < 200ms assertion at 10k pages (100 queries measured) - SearchBenchmark: 6 JMH SampleTime benchmark methods via kotlinx-benchmark - SyntheticGraphDbBuilder: shared in-memory graph fixture for tests and benchmarks Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Android Load BenchmarkInstrumented benchmark on an API 30 x86_64 emulator measuring load performance for the Android app. Comparing
|
JVM Load Benchmark (Desktop)Synthetic in-memory benchmark measuring load performance for the desktop (JVM) app.
Flamegraphs (this PR)**Allocation** — object allocation pressure (JDBC/SQLite churn)Alloc flamegraph not available CPU — method-level hotspots by on-CPU time CPU flamegraph not available Top SQL queries by total time (this PR)| table:operation | calls | p50 | p99 | max | total | |-----------------|-------|-----|-----|-----|-------| | `pages:select` | 2 | 1ms | 1ms | 2ms | 3ms |Top allocation hotspots (this PR)`70.6%` byte[]_[k] `3.6%` java.lang.String_[k] `3.1%` java.lang.StringBuilder_[k] `2.1%` java.lang.invoke.MethodType_[k] `2.1%` java.lang.Object[]_[k]Top CPU hotspots (this PR)`99.3%` /usr/lib/x86_64-linux-gnu/libc.so.6 `0%` SR_handler `0%` org/sqlite/core/CoreStatement.exec_[0] `0%` ClassFileParser::parse_methods `0%` jdk/internal/loader/URLClassPath$JarLoader$2.getBytes_[1] |
There was a problem hiding this comment.
Pull request overview
This PR enhances the search subsystem by adding a navigation-based “visit recency” ranking signal, guaranteeing exact title matches appear first, and introducing FTS repair utilities plus latency/benchmark coverage to validate performance at scale.
Changes:
- Add visit tracking (
page_visits) and apply a visit-recency multiplier during ranking. - Promote exact title matches (case-insensitive, trimmed) to the first result post-scoring.
- Add FTS repair/integrity utilities and introduce new integration + latency + benchmark tests.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/SqlDelightSearchRepository.kt | Applies visit-recency multiplier, exact-title promotion, and adds FTS rebuild/integrity + visit write paths. |
| kmp/src/commonMain/sqldelight/dev/stapler/stelekit/db/SteleDatabase.sq | Fixes pages_ai trigger rowid usage and adds page_visits table + queries. |
| kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/MigrationRunner.kt | Adds migrations to fix the pages_ai trigger and create page_visits. |
| kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/StelekitViewModel.kt | Records page visits on navigation (fire-and-forget). |
| kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/GraphRepository.kt | Extends SearchRepository with visit + FTS maintenance APIs. |
| kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/RestrictedDatabaseQueries.kt | Exposes restricted write helpers for visit tracking queries. |
| kmp/src/commonMain/kotlin/dev/stapler/stelekit/repository/InMemoryRepositories.kt | Adds an in-memory implementation of visit tracking for the in-memory search repo. |
| kmp/src/jvmCommonMain/kotlin/dev/stapler/stelekit/performance/InstrumentedSearchRepository.kt | Adds tracing around searchWithFilters. |
| kmp/src/jvmTest/kotlin/dev/stapler/stelekit/repository/SearchRepositoryIntegrationTests.kt | Adds integration tests for visit tracking + exact-title promotion. |
| kmp/src/jvmTest/kotlin/dev/stapler/stelekit/repository/VisitRecencyMultiplierTest.kt | Unit tests for the visit-recency decay curve. |
| kmp/src/jvmTest/kotlin/dev/stapler/stelekit/repository/ExactTitleMatchTest.kt | Unit tests for exact-title promotion behavior. |
| kmp/src/jvmTest/kotlin/dev/stapler/stelekit/repository/FtsRebuildTest.kt | Integration tests for rebuild/integrity-check flows. |
| kmp/src/jvmTest/kotlin/dev/stapler/stelekit/repository/SearchLatencyTest.kt | Adds latency assertions for cold-start and p99 at 10k pages. |
| kmp/src/jvmTest/kotlin/dev/stapler/stelekit/benchmark/SyntheticGraphDbBuilder.kt | Test utility to populate a DB for latency/benchmark tests. |
| kmp/src/jvmMain/kotlin/dev/stapler/stelekit/benchmarks/SearchBenchmark.kt | Adds JMH/kotlinx-benchmark methods for search and ranking hotspots. |
| kmp/src/commonTest/kotlin/dev/stapler/stelekit/ui/screens/SearchViewModelTest.kt | Updates test double to implement the extended SearchRepository interface. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| latencies.sort() | ||
| val p99 = latencies[(latencies.size * 0.99).toInt()] | ||
| assertTrue( | ||
| p99 < 200, | ||
| "p99 latency ${p99}ms exceeded 200ms at 10k pages (p50=${latencies[50]}ms, max=${latencies.last()}ms)" | ||
| ) |
There was a problem hiding this comment.
Fixed — updated to val p99Idx = (Math.ceil(0.99 * latencies.size)).toInt() - 1 (nearest-rank definition). With 100 samples this now correctly indexes 98 (0-based), not 99.
| @DirectRepositoryWrite | ||
| override suspend fun recordPageVisit(pageUuid: String): Either<DomainError, Unit> { | ||
| visitMap[pageUuid] = System.currentTimeMillis() | ||
| return Unit.right() |
There was a problem hiding this comment.
Fixed — changed to kotlin.time.Clock.System.now().toEpochMilliseconds() which is multiplatform-safe.
| } | ||
|
|
||
| /** In-memory visit store for testing — maps pageUuid to last_visited_at epoch ms. */ | ||
| val visitMap: MutableMap<String, Long> = mutableMapOf() |
There was a problem hiding this comment.
Fixed — visitMap is now internal val so it's accessible within the module for testing but not exposed as a public API.
| suspend fun populate( | ||
| db: SteleDatabase, | ||
| pageCount: Int, | ||
| blocksPerPage: Int = 10, | ||
| seed: Long = 42L, | ||
| ) { |
There was a problem hiding this comment.
Fixed — removed the unused seed parameter from populate(). Content generation is deterministic by index (sequential word-list cycling), so a seed would provide no additional value here.
| if (writeActor != null) { | ||
| writeActor.execute { | ||
| @OptIn(DirectSqlWrite::class) | ||
| restricted.insertPageVisitIfAbsent(pageUuid, nowMs) | ||
| @OptIn(DirectSqlWrite::class) | ||
| restricted.updatePageVisit(nowMs, pageUuid) | ||
| Unit.right() | ||
| } |
There was a problem hiding this comment.
Fixed — recordPageVisit now uses writeActor.execute(priority = DatabaseWriteActor.Priority.LOW) so visit bookkeeping doesn't contend with interactive writes.
| if (writeActor != null) { | ||
| writeActor.execute { | ||
| sqlDriver.execute(null, "INSERT INTO blocks_fts(blocks_fts) VALUES('rebuild')", 0) | ||
| sqlDriver.execute(null, "INSERT INTO pages_fts(pages_fts) VALUES('rebuild')", 0) | ||
| Unit.right() | ||
| } | ||
| } else { |
There was a problem hiding this comment.
Fixed — rebuildFts() now uses writeActor.execute(priority = DatabaseWriteActor.Priority.LOW) so the O(N) rebuild yields to user-initiated writes.
| override fun searchWithFilters(searchRequest: SearchRequest): Flow<Either<DomainError, SearchResult>> { | ||
| val startMs = HistogramWriter.epochMs() | ||
| return delegate.searchWithFilters(searchRequest).map { result -> | ||
| val durationMs = HistogramWriter.epochMs() - startMs | ||
| val span = tracer.spanBuilder("searchWithFilters").startSpan() | ||
| try { | ||
| span.setAttribute("ranking.visit_boost", "true") | ||
| span.setAttribute("result.ranked.count", result.getOrNull()?.ranked?.size?.toLong() ?: 0L) | ||
| span.setAttribute("duration.ms", durationMs) | ||
| } finally { | ||
| span.end() | ||
| } | ||
| result | ||
| } |
There was a problem hiding this comment.
Fixed — the span is now created before invoking the delegate, onStart captures startMs, and onCompletion records duration and ends the span. The span accurately covers the full search latency.
| import kotlin.test.assertFalse | ||
| import kotlin.test.assertNotNull | ||
| import dev.stapler.stelekit.repository.RankedSearchHit | ||
| import kotlin.time.Duration.Companion.hours |
There was a problem hiding this comment.
Fixed — removed the unused kotlin.time.Duration.Companion.hours import.
| // Warm up — excluded from measurement | ||
| repeat(5) { i -> | ||
| repo.searchWithFilters(SearchRequest(query = queryTerms[i % queryTerms.size], limit = 50)).first() | ||
| } | ||
|
|
||
| // Measure 100 queries | ||
| val latencies = mutableListOf<Long>() | ||
| repeat(100) { i -> | ||
| val query = queryTerms[i % queryTerms.size] | ||
| val start = System.currentTimeMillis() | ||
| repo.searchWithFilters(SearchRequest(query = query, limit = 50)).first() | ||
| latencies.add(System.currentTimeMillis() - start) | ||
| } |
There was a problem hiding this comment.
Fixed — added assertTrue(r.isRight(), "Warm-up query $i should succeed") in the warm-up loop and assertTrue(r.isRight(), "Measured query $i should succeed") in the measurement loop. The test now fails fast on any Left result rather than masking it with artificially low latencies.
- Add @DirectRepositoryWrite to rebuildFts/integrityCheckFts in InMemoryRepositories, InstrumentedSearchRepository, and SearchViewModelTest stub - Add @OptIn(DirectRepositoryWrite::class) to InstrumentedSearchRepository class - Use Priority.LOW for recordPageVisit and rebuildFts write-actor calls - Fix OTel span timing in InstrumentedSearchRepository (onStart/onCompletion) - Fix p99 latency formula to nearest-rank definition (ceil(0.99*n)-1) - Add isRight() assertions to warm-up and measured loops in SearchLatencyTest - Remove unused seed parameter from SyntheticGraphDbBuilder.populate() - Remove unused kotlin.time.Duration.Companion.hours import from integration tests - Rename benchmark methods to camelCase to pass Detekt FunctionNaming rule - Add detekt baseline entry for pre-existing UnusedParameter in GraphLoader Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
This PR consolidates the unmerged `stelekit-better-search` work and the new visit-recency / performance work into a single commit against `main`.
Ranking (from `stelekit-better-search`, not yet in main):
Ranking (new in this PR):
FTS health:
Performance test suite:
Test plan
🤖 Generated with Claude Code