feat(logs): support `--since <humantime>`, use pager by default by gaojunran · Pull Request #204 · jdx/pitchfork

gaojunran · 2026-01-31T08:37:30Z

Refactor logs command in CLI to make it fully usable. I will deal with TUI and Web UI later.

…ix bugs

gemini-code-assist · 2026-01-31T08:37:56Z

Summary of Changes

Hello @gaojunran, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly upgrades the logs command in the CLI, transforming it into a more powerful and user-friendly tool for managing daemon logs. The changes introduce flexible time-based filtering, integrate a default pager for improved readability in interactive environments, and provide options for raw output, making log analysis more efficient and adaptable to various workflows.

Highlights

Enhanced Time Filtering: The logs command now supports a --since <humantime> option, allowing users to filter logs using human-readable relative times (e.g., '5min', '2h', '1d'), time-only inputs (e.g., '10:30'), and full datetimes. The --until option has also been updated with similar capabilities.
Default Pager Integration: For interactive terminals, the logs command now utilizes a pager (like less) by default to improve the viewing experience for large log outputs. The pager starts at the end of the logs for convenience.
New Output Control Options: Two new flags have been introduced: --no-pager to explicitly disable the default pager behavior, and --raw to output log lines without any color or formatting, which is useful for piping output to other tools or saving to files.
Refactored Log Handling Logic: The internal implementation of the logs command has been significantly refactored to support the new filtering and output features, including optimized methods for reading log lines within specific time ranges using binary search.
Updated Command-line Arguments and Documentation: The command-line arguments for logs have been updated, renaming --from and --to to --since and --until respectively, and adding short aliases. All related documentation and usage examples have been revised to reflect these changes.
Comprehensive End-to-End Tests: A new dedicated test file (tests/test_e2e_logs.rs) has been added, containing extensive end-to-end tests for all new and modified logs command functionalities, ensuring robustness and correctness.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request significantly enhances the logs command by introducing time-based filtering with humantime, and improving user experience by using a pager by default. The implementation of time-based filtering using a binary search on log files is a clever optimization. The code is well-structured, and the accompanying documentation and test updates are comprehensive. I've identified a few potential panic points and areas where memory usage could be optimized in streaming scenarios. Addressing these will further improve the robustness of the new features.

gemini-code-assist · 2026-01-31T08:40:27Z

src/cli/logs.rs

+
+        match pager_result {
+            Ok(mut child) => {
+                let stdin = child.stdin.as_mut().expect("Failed to open pager stdin");


Using expect() here can cause the program to panic if it fails to get the pager's standard input. It would be more robust to handle this case gracefully, for instance by using if let Some(stdin) = child.stdin.as_mut() and falling back to printing directly to standard output if it's None, similar to how a failure to spawn the pager is already handled.

gemini-code-assist · 2026-01-31T08:40:27Z

src/cli/logs.rs

+
+        match pager_result {
+            Ok(mut child) => {
+                let stdin = child.stdin.take().expect("Failed to open pager stdin");


Using expect() here can cause the program to panic if it fails to get the pager's standard input. It would be more robust to handle this case gracefully, for example by using if let Some(stdin) = child.stdin.take() and falling back to direct streaming if it's None.

gemini-code-assist · 2026-01-31T08:40:27Z

src/cli/logs.rs

+            pager_args.push("+G");
+        }
+
+        pager_args.push(path.to_str().unwrap());


Using unwrap() here can cause a panic if the log file path contains non-UTF-8 characters. While this might be unlikely for daemon IDs, it's safer to handle this possibility. Consider using to_string_lossy() to avoid a potential panic.

Suggested change

pager_args.push(path.to_str().unwrap());

pager_args.push(path.to_string_lossy().as_ref());

gemini-code-assist · 2026-01-31T08:40:27Z

src/cli/logs.rs

+                        let lines: Vec<String> = reader.lines().map_while(Result::ok).collect();
+                        let parsed = parse_log_lines(&lines);


Reading the entire file into memory with collect() can lead to high memory usage for large log files. To make this a true streaming implementation, consider processing the file line-by-line instead of collecting all lines into a vector first. You could adapt the logic from parse_log_lines to be stateful and process lines as they are read.

gemini-code-assist · 2026-01-31T08:40:27Z

src/cli/logs.rs

+            let lines: Vec<String> = reader.lines().map_while(Result::ok).collect();
+            let parsed = parse_log_lines(&lines);


This implementation reads the entire log file into memory before printing, which can be inefficient for large files. To improve memory usage, you could process and print the lines as they are read from the file, similar to the suggestion for stream_logs_to_pager.

Copilot

Pull request overview

This PR refactors the logs command to enhance usability by adding human-readable time parsing, implementing automatic paging, and organizing log-related tests into a dedicated file.

Changes:

Added support for --since and --until with multiple time formats (humantime, time-only, full datetime)
Implemented automatic pager usage by default when viewing logs in interactive terminals
Added --raw and --no-pager flags for more control over output formatting
Moved log-related tests from test_e2e.rs to dedicated test_e2e_logs.rs file
Renamed --from/--to to --since/--until (breaking change)
Changed -n from having a default value of 100 to being optional (breaking change)
Added humantime dependency for relative time parsing

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
`src/cli/logs.rs`	Major refactor: added time parsing (humantime, time-only, datetime), implemented pager support, added raw output mode, implemented binary search for efficient time-range filtering
`tests/test_e2e_logs.rs`	New file containing comprehensive tests for logs command including tail, follow, time filters, raw output, and pager functionality
`tests/test_e2e.rs`	Removed log-related tests that were moved to dedicated file
`docs/guides/logs.md`	Updated documentation with new time filter examples and pager usage
`docs/cli/logs.md`	Auto-generated documentation updates for new flags and options
`pitchfork.usage.kdl`	Updated usage specification for logs command with new flags
`docs/cli/commands.json`	Auto-generated JSON schema updates for CLI commands
`Cargo.toml`	Added `humantime = "2"` dependency
`Cargo.lock`	Lockfile updates for humantime dependency

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/cli/logs.rs

tests/test_e2e_logs.rs

src/cli/logs.rs

Copilot

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 9 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/cli/logs.rs

Copilot · 2026-01-31T08:55:21Z

src/cli/logs.rs

+        // If the interpreted time for today is in the future, assume the user meant yesterday
+        if dt > now
+            && let Some(yesterday) = today.pred_opt()
+        {
+            naive_dt = NaiveDateTime::new(yesterday, time);
+            dt = Local
+                .from_local_datetime(&naive_dt)
+                .single()
+                .ok_or_else(|| miette::miette!("Invalid or ambiguous datetime: '{}'", s))?;
+        }
+        return Ok(dt);


The logic for assuming "yesterday" when a time-only input is in the future (lines 949-958) might be confusing in edge cases. For example, if a user queries at 23:59 for logs "since 23:58", they would get logs from yesterday at 23:58, not from one minute ago today. Consider documenting this behavior more explicitly or reconsidering the heuristic.

wrong explanation. It should be querying 23:59 at 23:58, and I think starting from yesterday is reasonable

pitchfork.usage.kdl

Copilot · 2026-01-31T08:55:22Z

src/cli/logs.rs

+                    }
+                }
+            }
+            None => {


When encountering a log line without a timestamp (line 717-719), the binary search advances low forward. This assumes that lines without timestamps should be included in the results. However, this could lead to incorrect behavior if there are many consecutive lines without timestamps at the beginning of the file, potentially causing the search to skip over valid timestamped lines. Consider documenting this assumption or handling this case more explicitly.

Suggested change

None => {

None => {

// Lines without a parseable timestamp are treated as if they should be

// included in the final results. To preserve that behavior, the binary

// search treats them as "within range" and simply advances `low` past

// the current line, continuing the search from the next line.

//

// This means that if a file starts with many non-timestamped lines,

// the binary search may converge to an offset after those lines. Callers

// relying on different semantics (e.g. always starting at byte 0 when

// the target time is before the first timestamp) must handle that

// explicitly outside this function.

We assume every line have timestamp because it is stored by pitchfork itself

Copilot · 2026-01-31T08:55:22Z

src/cli/logs.rs

+            if Command::new("bat").arg("--version").output().is_ok() {
+                "bat".to_string()


The check for bat availability (Command::new("bat").arg("--version").output().is_ok()) is performed synchronously every time a pager is needed, which could add latency. Consider caching this check or using a more efficient detection method, especially since the pager selection logic is duplicated three times in the code.

src/cli/logs.rs

Copilot · 2026-01-31T08:55:23Z

src/cli/logs.rs

+    if let Ok(output) = Command::new("tput").arg("lines").output()
+        && output.status.success()
+        && let Ok(s) = String::from_utf8(output.stdout)
+        && let Ok(h) = s.trim().parse::<usize>()
+    {
+        return Some(h);
+    }


The tput command is Unix-specific and not available on Windows. This will cause the terminal height detection to silently fail on Windows, falling back to the default of 24 lines. Consider using a cross-platform solution like the terminal_size crate or adding platform-specific implementations.

we can consider Windows later because on Windows there are so many things to consider

jdx · 2026-01-31T11:49:57Z

bugbot run verbose=true

cursor · 2026-01-31T11:50:00Z

Bugbot request id: serverGenReqId_feeefa0a-964d-4053-a499-4d2b32d379c7

jdx

Code Review (AI-generated)

Nice feature set — the --since humantime support, pager integration, and --raw mode are solid additions. A few things worth addressing:

1. Pager logic duplicated 3x

The pager selection + spawn logic (including the bat --version subprocess probe) is copy-pasted across output_with_pager, stream_logs_to_pager, and open_file_in_pager. Should be extracted into a shared helper.

2. Output formatting duplicated 5+ times

The if single_daemon { ... } else { ... } println pattern appears in output_logs, output_with_pager (Ok + Err branches), stream_logs_to_pager, and stream_logs_direct. A format_log_line helper would clean this up.

3. `stream_logs_to_pager` collects everything into memory

The default path (no -n, no time filter) collects all log lines into a Vec before writing to the pager. This defeats the purpose of streaming and could be problematic for large log files. Consider a streaming merge approach.

4. `edim()` in pager output path

edim() is applied to timestamps when piping to pager stdin. This works for less -R / bat, but if $PAGER is set to something that doesn't handle ANSI codes, it'll show garbage.

5. Binary search edge case with non-timestamped lines

In binary_search_log_position, the None (no parseable timestamp) case always advances low. This could skip valid lines that follow a multi-line log entry's continuation line sitting at the search boundary.

6. `bat` doesn't get `+G` equivalent

less gets +G to start at the end of output, but bat users will always start at the top. Minor UX inconsistency.

7. `parse_time_input` fallthrough precedence

If parse_datetime happens to match an input like "10:30", it won't reach the parse_time_only branch. The precedence chain works but could surprise — worth a comment.

8. `&PathBuf` → `&Path`

read_lines_in_time_range takes &PathBuf — idiomatic Rust prefers &Path.

9. `has_time_filter` passed redundantly

print_raw_lines receives from, to, and has_time_filter, but has_time_filter is just from.is_some() || to.is_some(). Could derive it internally.

Overall the features are good. Main structural concern is the duplication — extracting pager + formatting helpers would cut ~100 lines. The binary search edge case (5) is the most important correctness item.

gaojunran · 2026-01-31T13:52:51Z

@jdx ready for a review

jdx

This review was AI-generated using Claude Code.

Nice work on this refactor! The streaming merger with min-heap and binary search for time filtering are well-designed. A few items to address:

Bug

Regex capture group issue in StreamingLogParser::next() (~line 618):

let re = regex!(r"^(\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}) (\w)+ (.*)$");

(\w)+ captures only the last character of the log level. Should be (\w+):

let re = regex!(r"^(\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}) (\w+) (.*)$");

Suggestions

bat detection (~line 443): output().is_ok() can return true even when bat isn't found on some systems. Consider:
```
Command::new("bat").arg("--version").output().map(|o| o.status.success()).unwrap_or(false)
```
Raw mode multi-daemon sorting: print_raw_lines without time filter outputs files sequentially rather than interleaved by timestamp (unlike non-raw mode). Is this intentional?
Help text formatting: The --since long_help uses inline - separators which render as one long line. Consider actual newlines for readability.

Minor

Verify all imports in the use std::io::{...} block are needed
The spawned thread for pager streaming can't propagate errors back (acceptable for CLI, just noting)

Overall this is solid - the architecture is clean and tests are comprehensive. The regex bug should be fixed before merge.

gaojunran · 2026-02-01T04:16:38Z

@jdx ready for a review

jdx · 2026-02-01T11:02:08Z

bugbot run

jdx

This review was generated by Claude Code.

Nice work on this. The streaming merger, binary search for time filtering, and pager abstraction are well-designed. A few items:

1. Use crossterm::terminal::size() instead of spawning tput

get_terminal_height() spawns a tput subprocess on every invocation. crossterm is already a dependency — use it directly:

fn get_terminal_height() -> Option<usize> {
    crossterm::terminal::size().ok().map(|(_, h)| h as usize)
}

2. Binary search skips continuation lines incorrectly

In binary_search_log_position, when a line has no parseable timestamp (multi-line log continuation), the None arm always advances low. If the search lands on a continuation line right at the boundary of the target time, this can overshoot and skip the entry it belongs to. Consider scanning forward to the next timestamped line before making the comparison, rather than blindly advancing.

3. Yesterday fallback in parse_time_input applies to --until too

If --since 23:59 at 00:01 → yesterday makes sense. But --until 23:59 at 00:01 should mean today at 23:59, not yesterday. The fallback should only apply for --since.

4. Single-file pager path skips formatting

When there's one daemon and no -n/--since/--until, open_file_in_pager sends the raw file directly to the pager — no edim() on timestamps, and the daemon label token is visible. Every other code path formats the output. This is a UX inconsistency users will notice.

gaojunran · 2026-02-01T14:02:38Z

@jdx Addressed 1, 3 and 4. For problem 4, I used to send the raw file to the pager to get a higher speed. But it can not add style to the timestamp part, which is bad in UX. So I rollbacked this quick path after a decision, and if user meet the case to open a large log, they should do logrotate (which will be implemented later) or just use -n to limit.

For problem 2, as every log is created by pitchfork itself, we can assume every line has a timestamp to reduce complexity on binary search.

Copilot

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/cli/logs.rs

tests/test_e2e_logs.rs

src/cli/logs.rs

tests/test_e2e_logs.rs

gaojunran · 2026-02-01T14:34:34Z

btw, the CI keep failing on the same test, but I cannot reproduce it in my local environment. And the github action for the same branch in my forked repo runs successfully: https://github.com/gaojunran/pitchfork/actions/runs/21559857662/job/62122424249

I think it might be caused by the rust cache? not sure and wait for your idea.

## Summary - Fixes intermittent `test_stop_already_stopped` failure in CI (seen in #204 and potentially other PRs) - Root cause: test asserted daemon status immediately after `stop` returned, before the supervisor fully persisted the state file — under CI load, reading a partially-written state file caused a TOML parse error - Adds `wait_for_status` helper to `TestEnv` that polls daemon status with retries (up to 2s) - Applies the fix to all three stop command tests: `test_stop_transitions_to_stopped`, `test_stop_kills_child_processes`, and `test_stop_already_stopped` ## Test plan - [x] All 115 tests pass locally with `cargo nextest run` - [x] The three fixed tests pass reliably when run together (`cargo nextest run test_stop`) - [ ] CI passes on this branch 🤖 Generated with [Claude Code](https://claude.com/claude-code)  --- > [!NOTE] > **Low Risk** > Low risk: test-only changes that add a small retry loop to tolerate CI timing/state persistence delays; no production code paths are modified. > > **Overview** > Makes stop-related E2E tests less flaky under CI load by adding `TestEnv::wait_for_status` (polls `pitchfork status` up to ~2s) and using it to assert the final `stopped` state. > > Updates `test_stop_transitions_to_stopped`, `test_stop_kills_child_processes`, and `test_stop_already_stopped` to wait for `stopped` after `stop`, avoiding transient reads while the supervisor is still writing the state file. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit ba00400. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).  Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

jdx · 2026-02-01T20:34:00Z

bugbot run

## Summary - Fixes intermittent `test_stop_already_stopped` failure in CI (seen in #204) - Root cause: test asserted daemon status immediately after `stop` returned, before the supervisor fully persisted the state file — under CI load, reading a partially-written state file caused a TOML parse error (`None` status) - Adds `wait_for_status` helper to `TestEnv` that polls daemon status with retries (up to 5s) - Applies the fix to all three stop command tests ## Test plan - [x] All 115 tests pass locally with `cargo nextest run` - [x] The three fixed tests pass reliably when run together - [ ] CI passes 🤖 Generated with [Claude Code](https://claude.com/claude-code)  --- > [!NOTE] > **Low Risk** > Low risk: test-only change that increases retry window for daemon status polling; primary impact is slightly longer worst-case test runtime on failures. > > **Overview** > Extends the `TestEnv::wait_for_status` polling window from 2s to 5s (25 x 200ms) and updates the related docstring/error message, to better tolerate CI delays when persisting daemon state. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 21a6a09. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).  Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

docs/guides/logs.md:1

The long help text for the --since flag lacks proper formatting with newlines or bullet points between format types. The text runs together making it harder to read. Consider adding newlines (\n) between the format types to improve readability in the help output.

# Log Management

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-01T21:21:42Z

src/cli/logs.rs

+                    l.2.push('\n');
+                    l.2.push_str(&line);


When appending continuation lines to multi-line log entries, consider whether the message originally included a trailing newline. Currently, this unconditionally adds \n before appending the continuation line, which may result in extra newlines if the original message already had one. Verify that the log parsing regex and line handling correctly trim newlines before this point to avoid duplicate blank lines in output.

Suggested change

l.2.push('\n');

l.2.push_str(&line);

if !l.2.ends_with('\n') {

l.2.push('\n');

}

let trimmed = line.trim_end_matches('\n');

l.2.push_str(trimmed);

Copilot · 2026-02-01T21:21:43Z

src/cli/logs.rs

+        return false;
+    }
+
+    let terminal_height = get_terminal_height().unwrap_or(24);


The magic number 24 is used as a fallback terminal height. Consider defining this as a named constant (e.g., DEFAULT_TERMINAL_HEIGHT) to improve code readability and make it easier to adjust if needed.

Copilot · 2026-02-01T21:21:43Z

src/cli/logs.rs

+
+    // Start searching from the byte just before `pos`.
+    let mut search_pos = pos.saturating_sub(1);
+    const CHUNK_SIZE: usize = 8192;


The chunk size of 8192 bytes for reading log files is a magic number. Consider defining this as a module-level constant with a descriptive name like LOG_SEARCH_CHUNK_SIZE to clarify its purpose and make it easier to tune performance.

Copilot · 2026-02-01T21:21:43Z

pitchfork.usage.kdl

-        arg <FROM>
+    flag "-t --tail --follow" help="Show logs in real-time"
+    flag "-s --since" help="Show logs from this time" {
+        long_help "Show logs from this time\n\nSupports multiple formats: - Full datetime: \"YYYY-MM-DD HH:MM:SS\" or \"YYYY-MM-DD HH:MM\" - Time only: \"HH:MM:SS\" or \"HH:MM\" (uses today's date) - Relative time: \"5min\", \"2h\", \"1d\" (e.g., last 5 minutes)"


The long help text lacks proper formatting with bullet points appearing as plain text. Consider adding newlines between the format types (e.g., \n - Full datetime:...) to improve readability in the CLI help output.

## 🤖 New release * `pitchfork-cli`: 1.2.0 -> 1.3.0 <details><summary>Changelog</summary> <blockquote> ## [1.3.0](v1.2.0...v1.3.0) - 2026-02-01 ### Added - *(list)* show available daemons and align logics with TUI ([#206](#206)) - *(logs)* support `--since <humantime>`, use pager by default ([#204](#204)) - support `pitchfork.local.toml` ([#198](#198)) - impl `stop --all` ([#195](#195)) - beautify web ui ([#191](#191)) - add ready_cmd option ([#187](#187)) ### Fixed - refactor the logic of stopping a daemon and add tests ([#192](#192)) ### Other - re-order code to suit for multi-frontend structure ([#197](#197)) - *(deps)* update rust crate xx to v2.3.1 ([#203](#203)) - *(deps)* update rust crate clap to v4.5.56 ([#202](#202)) - *(ci)* run linting on all files in CI ([#196](#196)) - Update README.md logo ([#184](#184)) </blockquote> </details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/).  --- > [!NOTE] > **Low Risk** > Low risk: this PR only updates version numbers and release notes/docs, with no runtime code changes. > > **Overview** > Releases **v1.3.0** by bumping the crate version from `1.2.0` to `1.3.0` (including `Cargo.lock`) and updating the `pitchfork.usage.kdl`/generated CLI docs to match. > > Updates `CHANGELOG.md` with the `1.3.0` release notes and links to the included feature/fix PRs. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit d278b90. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).  --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

feat(logs): support --since <humantime>, use pager by default and f…

ed9e692

…ix bugs

Copilot AI review requested due to automatic review settings January 31, 2026 08:37

Copilot started reviewing on behalf of gaojunran January 31, 2026 08:37 View session

gemini-code-assist bot reviewed Jan 31, 2026

View reviewed changes

Copilot AI reviewed Jan 31, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings January 31, 2026 08:51

gaojunran force-pushed the feat-logs branch from 82e632a to 29a7eb0 Compare January 31, 2026 08:51

Copilot started reviewing on behalf of gaojunran January 31, 2026 08:51 View session

Copilot AI reviewed Jan 31, 2026

View reviewed changes

jdx reviewed Jan 31, 2026

View reviewed changes

fix(logs): misc

d225e0f

gaojunran force-pushed the feat-logs branch from 29a7eb0 to 9f252ae Compare January 31, 2026 13:01

jdx reviewed Jan 31, 2026

View reviewed changes

feat(logs): use min heap to stream parse logs

c2748ce

gaojunran force-pushed the feat-logs branch from 9f252ae to c2748ce Compare January 31, 2026 15:45

[autofix.ci] apply automated fixes

70a223d

jdx reviewed Feb 1, 2026

View reviewed changes

gaojunran added 2 commits February 1, 2026 21:39

fix(logs): misc

20e999c

fix: prefer UX for single daemon log display

41aab68

Copilot AI review requested due to automatic review settings February 1, 2026 13:57

Copilot started reviewing on behalf of gaojunran February 1, 2026 13:57 View session

Copilot AI reviewed Feb 1, 2026

View reviewed changes

jdx mentioned this pull request Feb 1, 2026

test: fix flaky test_stop_already_stopped in CI #208

Merged

3 tasks

Merge branch 'main' into feat-logs

17574e0

jdx mentioned this pull request Feb 1, 2026

fix(test): fix flaky test_stop_already_stopped in CI #209

Merged

3 tasks

Merge branch 'main' into feat-logs

f2a06c1

Copilot AI review requested due to automatic review settings February 1, 2026 21:20

Copilot AI reviewed Feb 1, 2026

View reviewed changes

jdx merged commit d2ff7af into jdx:main Feb 1, 2026
4 checks passed

jdx mentioned this pull request Feb 1, 2026

chore: release v1.3.0 #185

Merged

	pager_args.push(path.to_str().unwrap());
	pager_args.push(path.to_string_lossy().as_ref());

		let lines: Vec<String> = reader.lines().map_while(Result::ok).collect();
		let parsed = parse_log_lines(&lines);

-            None => {
+            None => {
+                // Lines without a parseable timestamp are treated as if they should be
+                // included in the final results. To preserve that behavior, the binary
+                // search treats them as "within range" and simply advances `low` past
+                // the current line, continuing the search from the next line.
+                //
+                // This means that if a file starts with many non-timestamped lines,
+                // the binary search may converge to an offset after those lines. Callers
+                // relying on different semantics (e.g. always starting at byte 0 when
+                // the target time is before the first timestamp) must handle that
+                // explicitly outside this function.

		if Command::new("bat").arg("--version").output().is_ok() {
		"bat".to_string()

Conversation

gaojunran commented Jan 31, 2026

Uh oh!

gemini-code-assist bot commented Jan 31, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

gaojunran Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

gaojunran Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

gaojunran Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

jdx commented Jan 31, 2026

Uh oh!

cursor bot commented Jan 31, 2026

Uh oh!

jdx left a comment

Choose a reason for hiding this comment

Code Review (AI-generated)

1. Pager logic duplicated 3x

3. `stream_logs_to_pager` collects everything into memory

4. `edim()` in pager output path

6. `bat` doesn't get `+G` equivalent

7. `parse_time_input` fallthrough precedence

8. `&PathBuf` → `&Path`

9. `has_time_filter` passed redundantly

gaojunran commented Feb 1, 2026 •

edited

Loading