Skip to content

feat: add frecency support #671

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

lalvarezt
Copy link
Contributor

@lalvarezt lalvarezt commented Jul 28, 2025

📺 PR Description

This PR add frecency support to already selected entries (opt-in), they will be boosted over the nucleo matcher results

Checklist

  • my commits and PR title follow the conventional commits format
  • if this is a new feature, I have added tests to consolidate the feature and prevent regressions
  • if this is a bug fix, I have added a test that reproduces the bug (if applicable)
  • I have added a reasonable amount of documentation to the code where appropriate

@alexpasmantier
Copy link
Owner

While this does the job,I think we can implement the feature in a much simpler way (a couple of hundred lines of code) by using the intermediate matcher layer on top of nucleo but I might be mistaken 🤔. Will try to have a more thorough look tomorrow.

@lalvarezt
Copy link
Contributor Author

lalvarezt commented Jul 30, 2025

In the meantime I've ironed out some performance problems here and there and added some docs as well. Based on some informal benchmarks I don't see any performance degradation in a directory with 80k+ files.

That aside; I was thinking that, in a way, the frecency feature is just a special case of sorting. What are your thoughts on instead of --frecency, --global-frecency we add a proper --sort flag with an enum like

pub enum SortMode {
    /// Score-based nucleo-matcher strategy
    #[default]
    Fuzzy,
    /// Frecency based (channel specific)
    Frecency,
    /// Frecency based (global)
    FrecencyGlobal,
    /// Disable sorting
    None,
}

And we could expose actions to toggle modes, live, while running the app

This could play nicely with @raylu PR #672 disabling sorting

@lalvarezt lalvarezt force-pushed the frecency-support branch 3 times, most recently from 5067a20 to 48c941f Compare August 5, 2025 13:21
@lalvarezt
Copy link
Contributor Author

lalvarezt commented Aug 5, 2025

cache handling should be significantly better now

2025-08-05T13:19:21.940886Z DEBUG television/main.rs:112: Running application...
2025-08-05T13:19:21.940910Z DEBUG television/app.rs:403: Starting rendering loop
2025-08-05T13:19:21.940934Z DEBUG television/app.rs:414: Rendering to stdout
2025-08-05T13:19:21.941346Z DEBUG television/app.rs:422: Entering tui
2025-08-05T13:19:21.941553Z DEBUG television/app.rs:435: Starting backend event loop
2025-08-05T13:19:21.941696Z DEBUG television/app.rs:445: Starting event handling loop
2025-08-05T13:19:22.005504Z DEBUG television/channels/channel.rs:199: Calculated intersection: dataset=38700, frecency=14, result=2
2025-08-05T13:19:22.005543Z DEBUG television/channels/channel.rs:304: Repopulating frecency matcher with 2 items
2025-08-05T13:19:22.063326Z DEBUG television/channels/channel.rs:157: Cache miss: dataset 38700 -> 73500, item: 2
2025-08-05T13:19:22.063397Z DEBUG television/channels/channel.rs:199: Calculated intersection: dataset=73500, frecency=14, result=2
2025-08-05T13:19:22.063788Z DEBUG television/previewer/mod.rs:188: Previewer received 1 request(s)!
2025-08-05T13:19:22.064018Z DEBUG television/previewer/mod.rs:252: Preview command: [bat -n --color=always '{}']
2025-08-05T13:19:22.134312Z DEBUG television/previewer/mod.rs:222: Preview job completed successfully
2025-08-05T13:19:23.255379Z DEBUG television/channels/channel.rs:157: Cache miss: dataset 73500 -> 79700, item: 2
2025-08-05T13:19:23.255440Z DEBUG television/channels/channel.rs:199: Calculated intersection: dataset=79700, frecency=14, result=2
2025-08-05T13:19:23.276275Z DEBUG television/channels/channel.rs:157: Cache miss: dataset 79700 -> 80100, item: 2
2025-08-05T13:19:23.276336Z DEBUG television/channels/channel.rs:199: Calculated intersection: dataset=80100, frecency=14, result=3
2025-08-05T13:19:23.276344Z DEBUG television/channels/channel.rs:304: Repopulating frecency matcher with 3 items
2025-08-05T13:19:23.276892Z DEBUG television/previewer/mod.rs:188: Previewer received 1 request(s)!
2025-08-05T13:19:23.277082Z DEBUG television/previewer/mod.rs:252: Preview command: [bat -n --color=always '{}']
2025-08-05T13:19:23.378797Z DEBUG television/previewer/mod.rs:222: Preview job completed successfully
2025-08-05T13:19:24.337322Z DEBUG television/channels/channel.rs:439: Dataset update task exiting - channel closed
2025-08-05T13:19:24.346413Z DEBUG television/channels/channel.rs:157: Cache miss: dataset 80100 -> 82866, item: 3
2025-08-05T13:19:24.346447Z DEBUG television/channels/channel.rs:199: Calculated intersection: dataset=82866, frecency=14, result=3
2025-08-05T13:19:26.024583Z DEBUG television/app.rs:550: Keybinding found: [SelectPrevEntry]
2025-08-05T13:19:26.024634Z DEBUG television/app.rs:465: Queuing new action: SelectPrevEntry
2025-08-05T13:19:26.024908Z DEBUG television/previewer/mod.rs:188: Previewer received 1 request(s)!
2025-08-05T13:19:26.024961Z DEBUG television/previewer/mod.rs:252: Preview command: [bat -n --color=always '{}']
2025-08-05T13:19:26.054507Z DEBUG television/previewer/mod.rs:222: Preview job completed successfully
2025-08-05T13:19:26.744227Z DEBUG television/app.rs:550: Keybinding found: [Quit]
2025-08-05T13:19:26.744308Z DEBUG television/app.rs:465: Queuing new action: Quit
2025-08-05T13:19:26.744346Z DEBUG television/app.rs:320: Stopped watch timer
2025-08-05T13:19:26.744569Z DEBUG television/render.rs:142: Exiting rendering loop
2025-08-05T13:19:26.744623Z DEBUG television/tui.rs:256: Exiting terminal
2025-08-05T13:19:26.744736Z DEBUG television/tui.rs:321: Successfully exited terminal
2025-08-05T13:19:26.745425Z  INFO television/main.rs:114: App output: AppOutput { selected_entries: None, expect_key: None, external_action: None }

@alexpasmantier if have some time can you review the latest version 🙏. wdyt?

@lalvarezt
Copy link
Contributor Author

some benchmarking in a directory with 400k files

lalvarezt at FedoraLinux42-wsl in ~/git-repos/television
➜ hyperfine -w10 -r20 'cargo run -q -- --cable-dir ./cable/unix --config-file ./.config/config.toml $HOME --take-1 --frecency --global-frecency'
Benchmark 1: cargo run -q -- --cable-dir ./cable/unix --config-file ./.config/config.toml $HOME --take-1 --frecency --global-frecency
  Time (mean ± σ):     951.2 ms ±  55.2 ms    [User: 1445.1 ms, System: 657.9 ms]
  Range (min … max):   865.7 ms … 1072.3 ms    20 runs


lalvarezt at FedoraLinux42-wsl in ~/git-repos/television
➜ hyperfine -w10 -r20 'cargo run -q -- --cable-dir ./cable/unix --config-file ./.config/config.toml $HOME --take-1'
Benchmark 1: cargo run -q -- --cable-dir ./cable/unix --config-file ./.config/config.toml $HOME --take-1
  Time (mean ± σ):     982.5 ms ±  25.4 ms    [User: 1444.2 ms, System: 662.7 ms]
  Range (min … max):   938.4 ms … 1038.9 ms    20 runs

@lalvarezt lalvarezt force-pushed the frecency-support branch 4 times, most recently from 73296df to cd3b936 Compare August 7, 2025 08:16
@lalvarezt lalvarezt marked this pull request as ready for review August 7, 2025 08:20
@alexpasmantier
Copy link
Owner

alexpasmantier commented Aug 7, 2025

Sorry I haven't taken any time to review this yet.

As I said earlier, I was caught up in the config refactor and wanted to lay that down before doing anything else.

A couple of things on this frecency PR though:

  • I'd rather have feat(channel): disable sorting #672 move forward before I take a look at this
  • as I said above, I'm kind of intrigued by the number of lines of this PR for such a feature, but I might be underestimating the complexity of the task.
  • my main goal as tv accumulates new features is to keep the codebase as maintainable and easy to work with as possible, and that includes being extra cautious with PRs like this one.

I know you've been working quite a lot on this and don't mean to disregard that in any way. We'll come back to this when the time is right.

@lalvarezt
Copy link
Contributor Author

It looks big by the +- numbers, but in reality it's mostly documentation and tests. In any case, let's proceed with your strategy

@alexpasmantier alexpasmantier force-pushed the main branch 2 times, most recently from da3dfed to 7ff55dc Compare August 12, 2025 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants