Skip to content

chore(web): remove htmx legacy code, fix cargo publish missing ui/dist#473

Merged
jdx merged 4 commits into
jdx:mainfrom
gaojunran:refactor/remove-htmx-legacy
Jun 8, 2026
Merged

chore(web): remove htmx legacy code, fix cargo publish missing ui/dist#473
jdx merged 4 commits into
jdx:mainfrom
gaojunran:refactor/remove-htmx-legacy

Conversation

@gaojunran

@gaojunran gaojunran commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

fixes https://github.com/endevco/pitchfork/actions/runs/27100027149/job/79978936264

Summary by CodeRabbit

  • Refactor

    • Crate now includes pre-built UI assets so a static front-end is shipped and served.
    • Server-rendered dashboard, daemon management pages, and HTML log views have been removed.
    • Client-side SSE log helpers and the legacy theme/styles were removed.
    • Process details simplified: fewer reported fields and no built-in formatted displays.
  • Chores

    • Removed an unused dependency.

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0c7c408f-5a31-485e-aab9-61c9810e5cfa

📥 Commits

Reviewing files that changed from the base of the PR and between 64bcea8 and 1abc0b3.

📒 Files selected for processing (1)
  • src/web/routes/logs.rs

📝 Walkthrough

Walkthrough

Removes legacy server-rendered web UI modules and assets (routes, helpers, CSS, JS), keeps logs SSE streaming only with a local html_escape helper, updates Cargo.toml to include ui/dist/**/* and removes urlencoding, and removes ExtendedProcessStats formatter methods.

Changes

Web UI Modernization

Layer / File(s) Summary
Build configuration and packaging
Cargo.toml
Adds ui/dist/**/* to the crate include list; removes urlencoding = "2" from [dependencies].
Web module surface simplification
src/web/mod.rs
Removes pub mod helpers; and the bp() base-path accessor; port() and normalize_base_path() remain with documentation/comments removed.
Route barrel cleanup
src/web/routes/mod.rs
Removes config, daemons, and index module declarations; leaves api and logs.
Logs route reduced to SSE streaming
src/web/routes/logs.rs
Introduces local html_escape helper; removes server-rendered index, show, lines_partial routes, the clear handler/js_escape, and related comments; preserves SSE streaming implementation (message payload no longer html-escaped before send).
Process stats type reduction
src/procs.rs
Redefines ExtendedProcessStats with fewer fields and removes its inherent display/formatter methods; get_extended_stats now populates only the remaining fields.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • endevco/pitchfork#470: Both PRs modify logs/SSE and log-store behaviors; changes to src/web/routes/logs.rs and src/web/assets/logs.js are directly related.
  • endevco/pitchfork#457: Related migration toward a SPA/API web UI; overlaps in removed server-rendered routes and API surface changes.

Poem

🐰 The old HTML pages hop away,
Dist files tucked into the crate today,
CSS and scripts no longer roam,
Logs still whisper through SSE's home,
A rabbit cheers, "Pack and ship — hooray!"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: removing htmx legacy code and fixing the cargo publish issue with ui/dist directory.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR removes the legacy htmx/server-side-rendered web UI in favour of a pre-built SPA served from ui/dist, and fixes cargo publish by updating the include glob in Cargo.toml to point at that new path.

  • Cargo.toml: include entry changed from src/web/assets/**/*ui/dist/**/*; urlencoding dependency dropped.
  • src/web/: All htmx asset files, the helpers.rs HTML-rendering module, and the config, daemons, index, and HTML portions of logs route handlers are deleted; only the SSE streaming endpoint survives.
  • src/procs.rs: ExtendedProcessStats is slimmed down — fields only consumed by the removed HTML views (exe_path, cwd, environ, start_time, disk I/O counters, parent_pid, user_id) and their display-formatting impl block are removed.

Confidence Score: 5/5

Safe to merge — the change is a targeted deletion of dead code with a single functional fix to the cargo publish asset path.

All deleted code is legacy server-side rendering being replaced by the SPA frontend. The one functional change (ui/dist/**/* in Cargo.toml) directly addresses the linked CI failure. The SSE streaming endpoint that remains is unmodified in logic; dropping the server-side HTML escaping of log lines is correct now that the client handles rendering.

No files require special attention.

Important Files Changed

Filename Overview
Cargo.toml Fixes cargo publish by changing asset include path from src/web/assets/**/* to ui/dist/**/*; also removes unused urlencoding dependency.
src/procs.rs Removes fields no longer needed by the SPA frontend and drops the display-formatting impl block from ExtendedProcessStats.
src/web/routes/logs.rs Removes HTML rendering helpers and the htmx SSE escape; retains only the stream_sse endpoint, which now sends raw (non-HTML-escaped) log text.
src/web/mod.rs Removes pub mod helpers and the bp() base-path helper; keeps normalize_base_path and WEB_PORT.
src/web/routes/mod.rs Removes config, daemons, and index module declarations; retains api and logs.
src/web/helpers.rs Entire file deleted — HTML/URL escaping utilities and daemon row renderers superseded by the SPA frontend.

Reviews (7): Last reviewed commit: "fix(logs): remove html_escape from SSE s..." | Re-trigger Greptile

Comment thread src/procs.rs Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/procs.rs (1)

576-596: 💤 Low value

Remove unnecessary #[allow(dead_code)] and empty impl block.

ExtendedProcessStats is a public struct actively returned by get_extended_stats() and consumed by the API routes. The #[allow(dead_code)] attribute has no effect on a pub struct with pub fields since public items don't trigger dead-code warnings. The empty impl block is just noise.

♻️ Suggested cleanup
 /// Extended process stats with more detailed information
-#[allow(dead_code)]
 #[derive(Debug, Clone)]
 pub struct ExtendedProcessStats {
     pub name: String,
     pub exe_path: Option<String>,
     pub cwd: Option<String>,
     pub environ: Vec<String>,
     pub status: String,
     pub cpu_percent: f32,
     pub memory_bytes: u64,
     pub virtual_memory_bytes: u64,
     pub uptime_secs: u64,
     pub start_time: u64,
     pub disk_read_bytes: u64,
     pub disk_write_bytes: u64,
     pub parent_pid: Option<u32>,
     pub thread_count: usize,
     pub user_id: Option<String>,
 }
-
-impl ExtendedProcessStats {}
🤖 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/procs.rs` around lines 576 - 596, Remove the unnecessary
#[allow(dead_code)] attribute and the empty impl block for ExtendedProcessStats:
locate the public struct ExtendedProcessStats (used by get_extended_stats() and
API routes), delete the leading #[allow(dead_code)] annotation and remove the
empty impl ExtendedProcessStats {} block to clean up noise while preserving the
pub fields and struct definition.
🤖 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.

Nitpick comments:
In `@src/procs.rs`:
- Around line 576-596: Remove the unnecessary #[allow(dead_code)] attribute and
the empty impl block for ExtendedProcessStats: locate the public struct
ExtendedProcessStats (used by get_extended_stats() and API routes), delete the
leading #[allow(dead_code)] annotation and remove the empty impl
ExtendedProcessStats {} block to clean up noise while preserving the pub fields
and struct definition.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 45f42d7b-0910-4871-b80d-a615c3227a41

📥 Commits

Reviewing files that changed from the base of the PR and between 88037d6 and 023d6ad.

⛔ Files ignored due to path filters (4)
  • Cargo.lock is excluded by !**/*.lock
  • src/web/assets/favicon.ico is excluded by !**/*.ico
  • src/web/assets/htmx.min.js is excluded by !**/*.min.js
  • src/web/assets/logo.png is excluded by !**/*.png
📒 Files selected for processing (12)
  • Cargo.toml
  • src/procs.rs
  • src/web/assets/htmx-sse.js
  • src/web/assets/logs.js
  • src/web/assets/style.css
  • src/web/helpers.rs
  • src/web/mod.rs
  • src/web/routes/config.rs
  • src/web/routes/daemons.rs
  • src/web/routes/index.rs
  • src/web/routes/logs.rs
  • src/web/routes/mod.rs
💤 Files with no reviewable changes (9)
  • src/web/routes/config.rs
  • src/web/assets/htmx-sse.js
  • src/web/helpers.rs
  • src/web/assets/style.css
  • src/web/routes/index.rs
  • src/web/assets/logs.js
  • src/web/routes/daemons.rs
  • src/web/routes/mod.rs
  • src/web/mod.rs

gaojunran and others added 2 commits June 8, 2026 12:57
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@gaojunran gaojunran closed this Jun 8, 2026
@gaojunran gaojunran reopened this Jun 8, 2026
@gaojunran gaojunran closed this Jun 8, 2026
@gaojunran gaojunran reopened this Jun 8, 2026
@gaojunran gaojunran closed this Jun 8, 2026
@gaojunran gaojunran reopened this Jun 8, 2026
@jdx jdx merged commit 0566047 into jdx:main Jun 8, 2026
8 of 12 checks passed
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