feat(web): structured log display with filters in webui - #630
Merged
Conversation
Convert nested if-let patterns to Rust 2024 let-chain syntax across env, supervisor, pitchfork_toml, log_parse, log_store, cli, and web modules.
Add structured log rendering to the web UI, matching the CLI's display
style with level badges, logger names, and key=value field formatting.
Backend:
- Tail and SSE endpoints now emit JSONL (application/x-ndjson) with
include_structured: true
- Extract From<LogEntry> for JsonLogEntry to centralize conversion
- Add filter query params: since, until, level, grep, regex, logger,
case_sensitive, field (KEY=VALUE), jq expression
- Reuse CLI's MessageFilter, FieldFilter, and JqFilter for filtering
- Add GET /api/logs/{id}/loggers endpoint for distinct logger names
- Add GET /api/logs/{id}/field-keys endpoint for jq autocomplete
- Add FieldFilter::LoggerContains variant with SQLite LIKE query
- Add distinct_loggers() and distinct_field_keys() to SqliteLogStore
- Pre-validate regex and jq expressions, returning 400 on parse error
- Fix logfmt parser to reject non-logfmt lines (key character validation
+ bare key ratio check) so Go standard log format doesn't get
misparse as logfmt
Frontend:
- Add StructuredLogEntry type and JSONL parsing in useLogStream
- Structured rendering in log.ts: level badge with CLI color mapping,
logger italic dim, message bold, fields key=value with type-colored
values
- Filter bar with Level, Logger (dropdown from API), Search (with Regex
and Case-sensitive toggles), jq (with autocomplete from builtins +
field keys), Since, Until datetime pickers
- Responsive layout: 3-column grid on desktop, stacked on mobile
- Fix Vue scoped style not applying to v-html content by moving
v-html-related CSS to non-scoped style block
- Mobile timestamp: stacked date/time layout
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe pull request adds structured log serialization, filtered NDJSON streaming, SQLite logger and field-key discovery, and interactive UI filtering and rendering. It also improves logfmt parsing and consolidates existing conditional control flow across configuration, storage, and supervisor code. ChangesStructured log flow
Conditional control-flow cleanup
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant LogView
participant TailAPI
participant SqliteLogStore
participant JsonLogEntry
LogView->>TailAPI: request filters and stream logs
TailAPI->>SqliteLogStore: query structured entries
SqliteLogStore-->>TailAPI: return raw entries
TailAPI->>JsonLogEntry: convert entries to JSON
JsonLogEntry-->>LogView: emit NDJSON records
LogView->>LogView: parse and render structured fields
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Greptile SummaryThe PR adds structured JSONL log streaming, server-side filtering and pagination, structured log rendering, and clear-generation synchronization.
Confidence Score: 5/5The PR appears safe to merge because no blocking failures remain within the scope of the previous review threads. No blocking failure remains. Important Files Changed
Reviews (25): Last reviewed commit: "fix(web): use generation-based staleness..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/log_store/sqlite.rs (1)
796-840: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUnbounded full-history scans for autocomplete queries.
distinct_loggersanddistinct_field_keysscan every row for a daemon (the latter also expanding each row viajson_each), with noLIMITand no supporting index onlogger/fields_json. These are invoked from the new/loggersand/field-keysendpoints on every LogView mount/daemon switch, so cost scales with total log history rather than typical field-key cardinality.Consider capping the scan (e.g.
ORDER BY id DESC LIMIT Nsubquery before taking distinct values) or adding a(daemon_id, logger)index, since autocomplete only needs a representative sample, not the full history.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/log_store/sqlite.rs` around lines 796 - 840, Bound the autocomplete work in distinct_loggers and distinct_field_keys by limiting the log-entry sample before applying DISTINCT and json_each, using an appropriate fixed cap while preserving daemon filtering and sorted unique results. Avoid scanning the daemon’s entire history on each request; keep both methods’ existing Result<Vec<String>> behavior unchanged.ui/src/views/LogView.vue (1)
140-159: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winInitial log stream connection is needlessly delayed by 500ms.
The filter watcher uses
{ immediate: true }, but the callback body still always schedules the update viasetTimeout(..., 500), so even the very first (no-op) filter evaluation on mount delaysdebouncedFilters— and therefore the initialuseLogStreamconnection — by 500ms.💡 Proposed fix: skip the debounce on the immediate/no-filter case
watch( [filterLevel, filterLogger, filterSearch, filterSearchRegex, filterSearchCase, filterJq, filterSince, filterUntil], - () => { + (_, oldValues) => { if (debounceTimer) clearTimeout(debounceTimer) - debounceTimer = setTimeout(() => { - const search = filterSearch.value.trim() - debouncedFilters.value = { - level: filterLevel.value || undefined, - logger: filterLogger.value || undefined, - grep: search && !filterSearchRegex.value ? search : undefined, - regex: search && filterSearchRegex.value ? search : undefined, - caseSensitive: search && filterSearchCase.value ? true : undefined, - jq: filterJq.value.trim() || undefined, - since: filterSince.value || undefined, - until: filterUntil.value || undefined, - } - }, 500) + const apply = () => { + const search = filterSearch.value.trim() + debouncedFilters.value = { + level: filterLevel.value || undefined, + logger: filterLogger.value || undefined, + grep: search && !filterSearchRegex.value ? search : undefined, + regex: search && filterSearchRegex.value ? search : undefined, + caseSensitive: search && filterSearchCase.value ? true : undefined, + jq: filterJq.value.trim() || undefined, + since: filterSince.value || undefined, + until: filterUntil.value || undefined, + } + } + if (oldValues === undefined) { + apply() // first run on mount: apply immediately, no debounce + } else { + debounceTimer = setTimeout(apply, 500) + } }, { immediate: true }, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/src/views/LogView.vue` around lines 140 - 159, Update the filter watcher around debouncedFilters so the immediate initial evaluation with no active filters applies the filter state synchronously instead of scheduling the 500ms debounce. Preserve the existing debounce behavior for subsequent filter changes and for an initial evaluation containing active filters, using the existing filter values and debounceTimer flow.ui/src/composables/useApi.ts (1)
218-234: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
LogStreamFiltersdoesn't expose the backend'sfield(KEY=VALUE) filter.The backend tail endpoint's
build_filtersaccepts repeatedfieldquery params forFieldFilter::FieldEq, and a/logs/{id}/field-keysendpoint was added specifically to support field-value filtering in the UI.LogStreamFilters/connect()here have no corresponding property, so this filter type can never be sent from the composable regardless of what LogView.vue wires up.♻️ Suggested addition
export interface LogStreamFilters { since?: string until?: string level?: string grep?: string regex?: string logger?: string jq?: string caseSensitive?: boolean lines?: number + fields?: Record<string, string> }if (f.caseSensitive) params.set('case_sensitive', 'true') if (f.jq) params.set('jq', f.jq) + if (f.fields) { + for (const [key, value] of Object.entries(f.fields)) { + params.append('field', `${key}=${value}`) + } + } if (f.lines !== undefined) params.set('lines', String(f.lines))If field-value filtering is intentionally out of scope for this iteration, please disregard.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/src/composables/useApi.ts` around lines 218 - 234, Extend LogStreamFilters and the connect() query construction in useApi.ts to support field-value filters as repeated `field` query parameters, preserving all existing filter handling. Ensure the composable exposes the new field filter so LogView.vue wiring can send each KEY=VALUE entry to the backend tail endpoint.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/web/routes/api/logs.rs`:
- Around line 88-98: Validate each user-supplied field key in the web route’s
field-filter parsing before pushing FieldFilter::FieldEq, mirroring the
whitelist used by build_field_filters: allow only alphanumeric characters,
underscores, and periods. Ignore or reject entries with any other characters so
build_query_sql cannot receive an unsafe JSON path key.
- Around line 17-31: The TailQuery field cannot parse repeated query keys with
Axum’s standard Query extractor. Update the tail route’s query extraction and
TailQuery handling to use a repeated-value-compatible extractor or dependency,
preserving field=KEY=VALUE&field=KEY2=VALUE2 as a Vec<String> before
build_filters processes it.
---
Nitpick comments:
In `@src/log_store/sqlite.rs`:
- Around line 796-840: Bound the autocomplete work in distinct_loggers and
distinct_field_keys by limiting the log-entry sample before applying DISTINCT
and json_each, using an appropriate fixed cap while preserving daemon filtering
and sorted unique results. Avoid scanning the daemon’s entire history on each
request; keep both methods’ existing Result<Vec<String>> behavior unchanged.
In `@ui/src/composables/useApi.ts`:
- Around line 218-234: Extend LogStreamFilters and the connect() query
construction in useApi.ts to support field-value filters as repeated `field`
query parameters, preserving all existing filter handling. Ensure the composable
exposes the new field filter so LogView.vue wiring can send each KEY=VALUE entry
to the backend tail endpoint.
In `@ui/src/views/LogView.vue`:
- Around line 140-159: Update the filter watcher around debouncedFilters so the
immediate initial evaluation with no active filters applies the filter state
synchronously instead of scheduling the 500ms debounce. Preserve the existing
debounce behavior for subsequent filter changes and for an initial evaluation
containing active filters, using the existing filter values and debounceTimer
flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: b940ec86-476b-43dc-ab91-95f703667587
📒 Files selected for processing (21)
src/cli/json_output.rssrc/cli/logs.rssrc/cli/mod.rssrc/cli/settings.rssrc/config_types.rssrc/env.rssrc/log_parse.rssrc/log_store/mod.rssrc/log_store/sqlite.rssrc/pitchfork_toml.rssrc/supervisor/lifecycle.rssrc/supervisor/mod.rssrc/supervisor/watchers.rssrc/template.rssrc/web/routes/api/logs.rssrc/web/routes/logs.rssrc/web/server.rsui/src/composables/useApi.tsui/src/types/api.tsui/src/utils/log.tsui/src/views/LogView.vue
Contributor
|
This PR currently has failing checks. If this continues for 7 days, it will be closed automatically. This is warning day 1 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
Contributor
|
This PR currently has failing checks. If this continues for 7 days, it will be closed automatically. This is warning day 2 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
…plain-text daemons Backend: - Validate field keys in build_filters (alphanumeric/underscore/period only) - Return 400 on invalid level value instead of silently ignoring - Return 400 on invalid since/until datetime instead of silently broadening - Keep last_clear_gen on generation lookup error to prevent log replay - Add LIMIT 5000 to distinct_loggers and distinct_field_keys queries - Replace unwrap_or_default() on serde_json::to_string with filter_map/ok - Add tests for logfmt bare key ratio edge cases and Go standard log rejection Frontend: - Skip 500ms debounce on initial mount, apply filters immediately - Hide Level/Logger/jq filters when daemon has no structured logs (determined by empty availableLoggers and jqFieldKeys) Format: - cargo fmt normalization on let-chain syntax from previous commit
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/log_parse.rs (1)
222-228: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve whether a pair used
=before applying the heuristic.Bare
fooand explicitfoo=""both become an empty string, so valid logfmt containing empty values can be rejected as mostly bare keys. Track an explicit key/value flag (or quoted-value flag) while parsing instead of inferring it fromv.is_empty().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/log_parse.rs` around lines 222 - 228, Update the pair parsing and heuristic in the log parsing flow around the `proper` calculation to preserve whether each pair explicitly used `=` (including quoted empty values). Count that recorded key/value indicator rather than inferring validity from `v.is_empty()`, so bare keys remain distinguishable from valid empty-value pairs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/log_parse.rs`:
- Around line 175-177: Update the invalid key=value handling in the log parsing
loop so that when validation fails with i at the equals delimiter, it consumes
the delimiter and skips the associated value through the end of that token
instead of parsing it as a bare key; alternatively reject the line. Preserve
normal parsing for subsequent valid tokens such as level=info.
- Around line 171-174: Align the key grammar used by parse_logfmt_pairs with the
web API field validation: either remove hyphen support from the parser predicate
or consistently allow hyphens in the API filter grammar. Prefer a shared
predicate where practical, and add a cross-layer validation test covering
request-id to prevent future mismatches.
---
Outside diff comments:
In `@src/log_parse.rs`:
- Around line 222-228: Update the pair parsing and heuristic in the log parsing
flow around the `proper` calculation to preserve whether each pair explicitly
used `=` (including quoted empty values). Count that recorded key/value
indicator rather than inferring validity from `v.is_empty()`, so bare keys
remain distinguishable from valid empty-value pairs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 74860d3a-5e0d-4f41-9c38-5cc4ea2fd13f
📒 Files selected for processing (13)
src/cli/settings.rssrc/config_types.rssrc/env.rssrc/log_parse.rssrc/log_store/sqlite.rssrc/pitchfork_toml.rssrc/supervisor/lifecycle.rssrc/supervisor/mod.rssrc/supervisor/watchers.rssrc/template.rssrc/web/routes/api/logs.rssrc/web/server.rsui/src/views/LogView.vue
🚧 Files skipped from review as they are similar to previous changes (11)
- src/web/server.rs
- src/config_types.rs
- src/pitchfork_toml.rs
- src/template.rs
- src/supervisor/lifecycle.rs
- src/supervisor/watchers.rs
- src/supervisor/mod.rs
- ui/src/views/LogView.vue
- src/cli/settings.rs
- src/log_store/sqlite.rs
- src/web/routes/api/logs.rs
- Fix dotted field key matching: FieldEq now uses json_each with
parameterized key instead of json_extract path interpolation, which
treated '.' as nested traversal (e.g. request.id never matched)
- Fix invalid key token handling: skip entire =value when key validation
fails, not just 1 byte
- Fix bare key vs empty value distinction: use Option<String> (None =
bare key, Some("") = explicit empty) so ratio check doesn't misclassify
- Align is_safe_field_key with parser: add hyphen support
- Add tests: explicit empty value, hyphenated key, Go standard log rejection
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/log_store/sqlite.rs`:
- Around line 625-638: Update the field-filter condition construction in the
query-building logic around query_params to compare values according to
json_each.type rather than always comparing json_each.value with text
parameters. Normalize or bind CLI filter values so boolean, null, and numeric
filters match their corresponding JSON scalar types while preserving string and
dotted-key handling. Add tests covering typed filters such as ready=true and
val=null, including the expected matches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: b82f7ddc-77fd-4542-83df-b515b6ea147f
📒 Files selected for processing (3)
src/log_parse.rssrc/log_store/sqlite.rssrc/web/routes/api/logs.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/web/routes/api/logs.rs
- Convert text filter values to JSON literals via text_to_json_literal() so json_each.value (native SQLite types) matches correctly: numbers, booleans, and null now work with --field port=8080, ready=true, val=null - Use IS instead of = for NULL comparison (NULL = NULL is false in SQL) - Return 400 when field param lacks '=' instead of silently ignoring - Align error message to include hyphen in allowed characters
gaojunran
force-pushed
the
feat/webui-structured-logs
branch
from
July 25, 2026 09:40
d547634 to
683fe31
Compare
json(?) returns JSON text (with quotes for strings), which doesn't match json_each.value's native SQLite type. Switch to json_extract(?, '$') which extracts the native value from JSON text, so string/number/boolean/null comparisons all work correctly.
- Add before_id backward pagination to LogQuery and SQLite query layer - Add id field to JsonLogEntry (skip_serializing_if zero, no CLI impact) - Tail endpoint returns one-shot JSONL when before_id is set (no polling) - Frontend: loadMoreHistory() fetches 100 older entries on scroll-to-top - Frontend: scroll position preserved during prepend, loading spinner - Frontend: hasMoreHistory flag stops loading when fewer than 100 returned - Remove clear filter button (activeFilterCount badge remains on Filter toggle)
- cargo fmt: normalize after_id/before_id struct field formatting across logs.rs, mcp.rs, log_store/mod.rs, tui/app.rs, web routes (fixes CI build failure) - Fix text_to_json_literal numeric validation: replace f64::parse with serde_json::from_str so Rust-valid-but-invalid-JSON numbers like "+42", "1.", ".5" are treated as strings (quoted) instead of causing SQLite json_extract to fail and stall the log stream - Fix initial clear generation lookup: on transient failure, store None instead of 0 so the first successful poll records the real generation without treating it as a clear event (which would reset the cursor and replay the entire log history) - Add unit tests for text_to_json_literal covering booleans, null, valid JSON numbers, invalid JSON number spellings, and strings
HTML datetime-local input defaults to 'YYYY-MM-DDTHH:MM' (no seconds), but parse_datetime only tried formats with seconds, causing a 400 error when only 'since' is passed. Add two no-seconds format patterns.
- Fix jq field key suggestions: jqFieldKeys are keys inside the .fields object, not top-level keys. Previously they were mixed with topLevelKeys at root level, so typing '.' suggested '.event_type' (wrong — should be '.fields.event_type'). Now: - Root context (.) → suggest only topLevelKeys (timestamp, fields, etc) - Fields context (.fields.) → suggest only jqFieldKeys - Deeper paths → no suggestions (avoids wrong guesses) - Increase jq dropdown opacity from rgba(255,255,255,0.03) (nearly transparent) to rgba(18,18,21,0.96) (nearly opaque) with a stronger border and drop shadow for readability
…itive
Field filter values were always converted to typed JSON literals (number,
boolean, null) before comparison with json_each.value. This made string
fields storing text like "true", "42", or "null" unreachable via
field filters, since json_each.value (TEXT) would never match
json_extract('true', '$') (INTEGER 1).
Add a raw text comparison path alongside the typed path. Since
json_each.value has no type affinity, the text IS comparison only
matches string values, not native integers or booleans, so there are
no false positives.
When a daemon has never been cleared, log_clear_generations has no row and initial_gen is None. The None branch treated the first successful poll as a clear event, resetting the cursor to 0 and replaying the entire initial history as duplicates in the append-only stream. Now that query_with_generation does both reads atomically, None only means 'never cleared' — a transient failure fails the whole call and returns an error response, never None. Initialize last_clear_gen to initial_gen.unwrap_or(0) and remove the None branch entirely. A never-cleared daemon has generation 0 on both initial fetch and first poll, so no spurious clear is detected.
Contributor
|
This PR currently has failing checks. If this continues for 7 days, it will be closed automatically. This is warning day 1 of 7. Please update the PR when you have a chance. Feel free to reopen or create a new PR if it is closed and you'd like to continue working on it. This comment was generated by an automated workflow. |
The polling loop read the clear generation and new rows in two separate spawn_blocking calls. A clear could interleave after the generation check but before the row query, producing post-clear rows that the next poll would replay when it detected the generation change and reset the cursor to 0. Use query_with_generation for the poll query too, so both reads share a single transaction snapshot. If a clear happened, the generation check in the same atomic batch catches it and skips the rows, preventing duplicates.
The SSE endpoint had the same clear race as the NDJSON endpoint: the clear generation and incremental rows were read in separate spawn_blocking calls, allowing a clear to interleave and produce duplicate streamed entries. Use query_with_generation for both the initial cursor capture and the poll loop, so each read pair shares a single transaction snapshot.
Three issues across the streaming paths:
1. SSE init failure replay (src/web/routes/logs.rs): when the initial
query_with_generation failed, the code fell back to (last_id=0,
last_clear_gen=0). The next successful poll then queried after_id=0,
replaying the entire log history. Now yields an error event and
returns instead.
2. NDJSON clear signal missing (src/web/routes/api/logs.rs): when a
clear was detected during polling, the backend reset its cursor and
skipped the batch, but the frontend never learned about the clear.
Old entries remained in the display buffer permanently. Now emits a
{"_clear":true} sentinel line so the frontend can flush.
3. Frontend clear handling (ui/src/composables/useApi.ts): the NDJSON
stream parser now recognizes the _clear sentinel and clears the
lines buffer, removing stale entries.
is_safe_field_key restricted field filter keys to [a-zA-Z0-9_.-], the logfmt parser charset. But field-keys discovery returns keys extracted via json_each, which can contain any valid JSON key character (e.g. @timestamp, http.status, keys with spaces). The store query uses fully parameterized SQL (json_each.key = ?), so arbitrary keys are safe and the whitelist only blocked valid discoverable fields. Remove the whitelist, keep only a non-empty check.
Contributor
Author
gaojunran
force-pushed
the
feat/webui-structured-logs
branch
from
July 28, 2026 14:16
46245d9 to
c31baf7
Compare
aube v1.8.0 had no GitHub artifact attestations, causing mise 2026.7.15 to fail attestation verification in CI. Regenerating the lockfile with --bump re-resolves the 'latest' selector to aube 1.34.0, which has valid attestations. Other tools also advance: communique 1.2.4, cargo-nextest 0.9.140, cargo-binstall 1.21.1, node 24.15.0.
gaojunran
force-pushed
the
feat/webui-structured-logs
branch
from
July 28, 2026 14:39
c31baf7 to
002b1d4
Compare
The logfmt tokenizer rejected keys containing '@', but extract_level looks for '@Level' as a level field name. So log lines using pino/syslog style (@Level=info @message=hello) had their @Level key discarded before level extraction could process it, losing the level and causing the entry to disappear from level-filtered results. Add '@' to the logfmt key charset alongside the existing alphanumeric, '_', '.', '-' characters.
…path Add comment explaining that execute_built_query is called directly instead of self.query() to avoid query_parallel opening separate connections outside the transaction, which would break the WAL snapshot guarantee.
gaojunran
commented
Jul 29, 2026
gaojunran
left a comment
gaojunran
left a comment
Contributor
Author
There was a problem hiding this comment.
@greptileai This is the same concern raised in the previous review (discussion_r3667516249) and already addressed there.
query_with_generation does not call self.query(). It calls build_query_sql + execute_built_query directly on the locked transaction connection (sqlite.rs:1209-1210). The parallel path (query_parallel) is only reachable through self.query() (sqlite.rs:1057-1064), which query_with_generation never invokes. Both the history query and the generation read execute on the same connection within the same BEGIN/ROLLBACK transaction, sharing one WAL snapshot. No separate connections are opened.
The invariant is documented with a comment (commit 6f01d5d) to prevent future refactors from accidentally introducing this race.
This comment was generated by Claude Code.
Contributor
|
You're right, and I apologize for the repeated concern. The implementation is clear:
The snapshot consistency is properly guaranteed. I should have recognized this was already addressed before raising it again. |
Contributor
Author
Resolve mise.lock conflict by regenerating from scratch with mise lock --bump. aube resolves to 1.34.0 with valid attestations.
gaojunran
marked this pull request as ready for review
July 29, 2026 18:06
When loadMoreHistory fires a before_id request and a log clear happens while that request is in-flight, the clear sentinel empties the buffer but the paginated response arrives afterward with pre-clear entries and unconditionally unshifts them back, permanently restoring deleted logs. Track a clearEpoch counter incremented on each clear sentinel. loadMoreHistory captures the epoch before fetching and discards results if the epoch changed, ensuring pre-clear entries are never merged back.
The clearEpoch counter only incremented when the stream received the clear sentinel. If the stream disconnected before delivering it (or the backward-history response arrived before the sentinel), stale pre-clear entries were merged back, permanently restoring deleted logs. Replace clearEpoch with streamGen, tracking the actual clear generation: - Backend: x-log-generation header on both initial stream and before_id responses; clear sentinel now includes _gen value - Frontend: streamGen updated from initial response header and clear sentinels; loadMoreHistory compares response generation against streamGen and discards if older (clear happened) or if streamGen changed during fetch (disconnect/reconnect with newer generation)
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.
Summary
Add structured log rendering to the web UI, matching the CLI's display style with level badges, logger names, and key=value field formatting.
Backend
application/x-ndjson) withinclude_structured: trueFrom<LogEntry> for JsonLogEntryto centralize conversionsince,until,level,grep,regex,logger,case_sensitive,field(KEY=VALUE),jqMessageFilter,FieldFilter, andJqFilterfor filteringGET /api/logs/{id}/loggers— distinct logger names for dropdownGET /api/logs/{id}/field-keys— distinct field keys for jq autocompleteFieldFilter::LoggerContainsvariant with SQLite LIKE queryFrontend
StructuredLogEntrytype and JSONL parsing inuseLogStreamkey=valuewith type-colored valuesv-htmlcontentTest plan
mise run ci-devpassesThis PR was created by Claude Code.
Summary by CodeRabbit
New Features
Bug Fixes