Skip to content

feat: Rich-based diff renderer with intra-line highlighting - #1

Closed
KUSH42 wants to merge 2 commits into
mainfrom
feat/rich-diff-renderer
Closed

feat: Rich-based diff renderer with intra-line highlighting#1
KUSH42 wants to merge 2 commits into
mainfrom
feat/rich-diff-renderer

Conversation

@KUSH42

@KUSH42 KUSH42 commented Apr 1, 2026

Copy link
Copy Markdown
Owner

feat: Rich-based diff renderer with intra-line highlighting

Introduces a Rich/Pygments-based rendering engine in agent/rich_output.py and replaces the inline diff renderer with a new DiffRenderer class.

Changes

agent/rich_output.py (new module, no project-specific imports)

Core rendering toolkit — consumed here and by the follow-up highlighting PR:

  • LanguageDetector — extension map + content-pattern heuristics
  • SyntaxHighlighter — Pygments → Rich markup → ANSI string
  • FilePathFormatter — per-filetype icons, compact relative paths
  • DiffRenderer — unified diff → Rich Text objects with line numbers
  • _intra_diff — character-level segment lists via SequenceMatcher (exposed for testing)
  • _parse_diff_filename — strips a/ / b/ prefixes, handles /dev/null (exposed for testing)

DiffRenderer over the old ANSI-string path

  • Intra-line character-level diff: changed chars highlighted bright-red/green bold, unchanged chars use the base diff background. Similarity threshold 0.5 — below it the lines are too dissimilar and flat highlighting is used instead
  • Per-run del/add pairing: consecutive -/+ runs are zipped for intra-line comparison then emitted in order, avoiding cross-hunk false pairings
  • Summary header per file: ● filename.py Added N lines, removed M lines
  • DiffRenderer.to_lines() replaces _render_inline_unified_diff in the display layer; truncation (max lines / max files) is handled by the existing _summarize_rendered_diff_sections wrapper in display.py

Bug fixes

  • Error token red background — _PygmentsToRich mapped Error tokens to "bold red on red" (red background), matching the diff deletion colour. Pygments emits Error tokens for content its markdown lexer cannot tokenize (e.g. emoji inside headings), producing spurious red highlights on unrelated text. Changed to "bold red" (text colour only), consistent with Generic.Error.

Tests

51 passing in tests/test_rich_output.py. tests/test_display.py assertions updated for the new header format.

KUSH42 added 2 commits April 1, 2026 19:52
@KUSH42
feat: Rich-based rendering engine with intra-line diff highlighting
80dab43 
Introduces agent/rich_output.py — a self-contained Rich/Pygments rendering
toolkit with no project-specific imports.

Public API:
- LanguageDetector: extension map + content-pattern heuristics
- SyntaxHighlighter: Pygments → Rich markup → ANSI string
- FilePathFormatter: per-filetype icons, compact relative paths
- DiffRenderer: unified diff → Rich Text with line numbers
- clean_command_output: strip venv/stacktrace noise from command output

DiffRenderer replaces _render_inline_unified_diff in display.py:
- Intra-line character-level highlighting via SequenceMatcher (threshold 0.5)
- Per-run del/add pairing to avoid cross-hunk false matches
- Summary header: ● filename.py   Added N lines, removed M lines
- Console width from shutil.get_terminal_size, not hardcoded

Tests: 51 passing in tests/test_rich_output.py
@KUSH42
fix(rich_output): Error token uses text colour not background
4f42b5b 
Pygments emits Error tokens for content its markdown lexer cannot
tokenize (emoji in headings, unknown syntax, etc.). Mapping Error to
"bold red on red" matched the diff-deletion colour, causing spurious
red backgrounds on unrelated text. Changed to "bold red" (text colour
only), consistent with Generic.Error.
@KUSH42
KUSH42 force-pushed the feat/rich-diff-renderer branch from 00e8cf7 to 4f42b5b Compare April 1, 2026 21:49
@KUSH42

KUSH42 commented Apr 1, 2026

Copy link
Copy Markdown
Owner Author

Superseded by NousResearch#4470

@KUSH42 KUSH42 closed this Apr 1, 2026
KUSH42 added a commit that referenced this pull request Apr 11, 2026
@KUSH42
fix(tui): UX audit quick-wins — focus restore, bindings, slash hint, …
510bfe4 
…flash

- GAP-17: restore input focus after every agent turn (call_after_refresh)
- GAP-7: flash hint bar with "Unknown command: /X" when slash lookup is empty
- GAP-4: ctrl+p inserts @ and opens path/file completion overlay
- GAP-5: ctrl+r aliased to ctrl+f for history search (muscle-memory parity)
- #1: dim input chevron (--busy class) while agent_running instead of hiding
- #2: brief $success background flash on ToolHeader when streaming completes

460 TUI tests passing.
KUSH42 pushed a commit that referenced this pull request May 1, 2026
@erosika @kshitijk4poor
fix(honcho): keep legacy schemeless baseUrl configs working
5883df5 
The scheme-validation commit (e77a3f2c) was too strict: a user with
legacy ''baseUrl: localhost:8000'' (no ''http://'' prefix) in their
''~/.honcho/config.json'' would get ''No API key configured'' from the
CLI after that change, even though their setup worked before.

urlparse on a schemeless host:port treats the host segment as the
scheme and leaves netloc empty, so the http/https check rejected it.

Falls back to a lenient check for schemeless strings that look like
hosts: contain '.' or ':', aren't a boolean/null literal, aren't pure
digits. The SDK still rejects truly malformed URLs at connect time
with a clearer error than ours.

Three new tests: legacy schemeless hosts accepted; obvious garbage
literals (''true'', ''null'', ''12345'') still rejected.  Reviewer
noted concern #1: schemeless regression for self-hosters with old
configs.
KUSH42 pushed a commit that referenced this pull request May 1, 2026
@alt-glitch
ci(nix): auto-fix stale npm hashes on push to main (NousResearch#16285)
18f585f 
* ci(nix): auto-fix stale npm hashes on push to main

When a PR merges to main with updated package-lock.json or package.json
in ui-tui/ or web/, the new auto-fix-main job detects stale npmDepsHash
values and pushes a fix commit directly to main.

This eliminates the recurring manual hash-bump PRs (NousResearch#15420, NousResearch#15314,
NousResearch#15272, NousResearch#15244) by reusing the existing fix-lockfiles --apply pipeline.

The fix commit only touches nix/*.nix files, which are outside the push
path filter (package-lock.json / package.json), so it cannot re-trigger
itself.

Closes NousResearch#15314

* fix(ci): use GitHub App token for auto-fix-main push

GITHUB_TOKEN commits are invisible to workflow triggers (GitHub's
infinite-loop prevention). The auto-fix-main job pushes directly to
main, so the fix commit never triggered downstream nix.yml verification.

Mint a short-lived token via the repo's GitHub App (daimon-nous, APP_ID
+ APP_PRIVATE_KEY secrets) so the push is treated as a real event and
nix.yml fires to verify the corrected hashes.

Tested via workflow_dispatch dry-run: app token minted successfully,
checkout with app token succeeded, fix job correctly gated.

Resolves review feedback from Bugbot (r3144569551).

* ci(nix): rename lockfile check job for required status check

Rename 'check' → 'nix-lockfile-check' so the status check name is
unambiguous when added as a required check on main.

* fix(ci): harden auto-fix-main against races, loops, and silent failures

Address adversarial review findings:

1. Race condition (#1): Job-level concurrency with cancel-in-progress
   collapses back-to-back pushes; ref: main checkout always gets latest
   branch state; explicit push target (origin HEAD:main).

2. Loop prevention (#2): File-whitelist check before commit aborts if
   any file outside nix/{tui,web}.nix was modified, preventing
   accidental self-triggering.

3. Silent infra failures (#8): nix-lockfile-check now fails explicitly
   when fix-lockfiles exits without reporting stale status (catches nix
   setup failures, network errors, script bugs that bypass continue-on-error).

4. Commit traceability (NousResearch#11): Auto-fix commits include source SHA and
   workflow run URL in the commit body.

5. Explicit push target (NousResearch#12): git push origin HEAD:main instead of
   bare git push.

---------

Co-authored-by: alt-glitch <alt-glitch@users.noreply.github.com>
KUSH42 pushed a commit that referenced this pull request May 1, 2026
@OutThisLife
fix(tui): drop stale stream events after ctrl-c interrupt (NousResear…
e42065b 
…ch#16706)

* fix(tui): drop stale stream events after ctrl-c interrupt

Once interruptTurn() flips this.interrupted, only recordMessageDelta
short-circuited.  recordReasoningDelta/Available, recordToolStart/
Progress/Complete, and recordInlineDiffToolComplete kept populating
turnState until the python loop reached its next _interrupt_requested
check (~1s on busy turns), making it look like ctrl-c was ignored
while late "thinking" + tool calls kept landing in the UI.

Add the same interrupted guard to every stream-side recorder, and
clear the flag at startMessage() so the next turn isn't suppressed
if the previous turn never delivered message.complete.

* fix(tui): guard recordTodos against post-interrupt mutation; fake-timers in test

Copilot review on PR NousResearch#16706:

1. `recordToolStart` is interruption-guarded, but `tool.start`
   handler also calls `recordTodos(payload.todos)` first — so a
   late tool.start carrying todos could still mutate `turnState.todos`
   after Ctrl-C, leaving ghost rows in the panel.  Adds the same
   `if (this.interrupted) return` early-exit to `recordTodos` so
   *all* tool.start side-effects are dropped post-interrupt.

2. The interrupt test was leaking a real `setTimeout` (interrupt
   cooldown) across test files, which could fire later and mutate
   uiStore from the wrong test context.  Wraps the test in
   `vi.useFakeTimers()` + `vi.runAllTimers()` and restores real
   timers in finally.

3. Extends the same test with a todos payload on the post-interrupt
   tool.start so we have explicit regression coverage for #1.

* fix(tui): guard pushTrail post-interrupt; harden interrupt-test cleanup

Round 2 Copilot review on PR NousResearch#16706:

1. `tool.generating` events route through `pushTrail`, which was not
   interruption-guarded — late events could still write 'drafting …'
   into `turnTrail` after Ctrl-C, leaving a stale shimmer in the UI.
   Adds the same `if (this.interrupted) return` early-exit.

2. Test cleanup moved `vi.runAllTimers()` into `finally` (before
   `vi.useRealTimers()`) so a mid-test assertion failure can't leak
   the interrupt-cooldown setTimeout across other test files.

3. Replaced the misleading 'pre-interrupt todos … expected to be
   cleared by the interrupt cycle' comment with an accurate one
   reflecting current behaviour (interrupt does NOT clear todos).

4. Added an explicit assertion that a post-interrupt `tool.generating`
   event does not extend `turnTrail` — regression coverage for #1.
KUSH42 pushed a commit that referenced this pull request May 10, 2026
@teknium1
docs(teams-pipeline): cron renewal recipe, sidebar wiring, skill rewrite
242da9d 
Fifth and final slice polish on top of @dlkakbs's docs + skill. Three
things ship here:

1. Subscription renewal cron recipe (the #1 operational footgun).

   Microsoft Graph webhook subscriptions expire at 72 hours max and
   don't auto-renew. The shipped operator runbook mentioned
   `maintain-subscriptions --dry-run` as a "daily or periodic check"
   but never told operators how to actually automate it. Without a
   scheduled job, any production deployment silently stops ingesting
   meetings three days after go-live.

   Adds an "Automating subscription renewal (REQUIRED for production)"
   section to website/docs/guides/operate-teams-meeting-pipeline.md
   with three concrete options and copy-pasteable configs:

   - Option 1: Hermes cron (`hermes cron add --schedule "0 */12 * * *"
     --script-only --command "hermes teams-pipeline maintain-subscriptions"`)
   - Option 2: systemd service + timer (12h cadence, Persistent=true
     so missed runs catch up after reboots)
   - Option 3: plain crontab with a wrapper that sources .env for
     credentials

   Go-Live Checklist gains a bolded mandatory item for the schedule
   being in place, with a cross-link to the section.

   website/docs/user-guide/messaging/teams-meetings.md adds a
   `:::warning:::` admonition right after the manual `subscribe`
   examples so anyone who creates a subscription manually is told
   the same day that it will silently expire in 72 hours.

2. Sidebar wiring. Shela's new docs pages (teams-meetings.md and
   operate-teams-meeting-pipeline.md) weren't in website/sidebars.ts,
   so they were orphaned URLs — reachable only if someone knew the
   path. Wired teams-meetings into Messaging Platforms next to the
   existing teams entry, and operate-teams-meeting-pipeline into
   Guides & Tutorials next to microsoft-graph-app-registration from
   PR NousResearch#21922. Adjacent placement keeps the related pages discoverable
   from each other.

3. SKILL.md rewrite (v1.0.0 → v1.1.0).

   The original skill had five Turkish-only trigger phrases, which
   works in a Turkish-speaking session but doesn't match English
   triggers. Rewrote the skill to:

   - Describe triggers by intent instead of exact phrases, with
     explicit "works in any language" framing and example phrases
     in both English and Turkish.
   - Add a Decision Tree section covering the three most common user
     asks (missing summary, setup verification, re-run request) and
     the specific CLI command sequence for each.
   - Add a dedicated "Critical pitfall: Graph subscriptions expire
     in 72 hours" section that tells the agent exactly what to do
     when a user reports "worked yesterday, nothing today" — the
     most common operational failure mode.
   - Expand the command reference into three labeled groups (Status
     and inspection / Re-running and debugging / Subscription
     management) so the agent can reach for the right command
     without scanning.
   - Add cross-links to all four related docs pages (Azure app
     registration, webhook listener setup, full pipeline setup,
     operator runbook).

Validation:
- npm run build: all new pages route, anchor to
  #automating-subscription-renewal-required-for-production resolves
  from both the runbook TOC and the teams-meetings.md admonition.
- scripts/run_tests.sh on the relevant test suites (607 tests): all
  pass.
KUSH42 pushed a commit that referenced this pull request May 10, 2026
@teknium1
fix(windows): quote cache paths in bash + augment PATH so rg/bash res…
fc91886 
…olve on first launch

Three interrelated bugs from teknium1's first interactive chat on Windows:

1. **Snapshot/cwd file paths unquoted in bash command strings.**  The session
   bootstrap and per-command wrapper interpolated
   ``self._snapshot_path`` / ``self._cwd_file`` unquoted into bash commands
   like ``export -p > C:/Users/ryanc/.../hermes-snap-xxx.sh``.  Git Bash's
   MSYS2 layer handles ``C:/...`` paths correctly ONLY when quoted; unquoted,
   the colon and forward-slash get glob-parsed and the redirect targets a
   bogus path.  Symptom: every terminal command emitted two
   ``C:/Users/.../hermes-snap-*.sh (No such file or directory)`` lines that
   bled into stdout (``stderr=STDOUT`` on the local backend) and corrupted
   file contents when the agent wrote to scratch paths via the terminal
   tool.  Fix: ``shlex.quote()`` every interpolation of ``_snapshot_path``
   and ``_cwd_file`` in base.py — no-op on POSIX (the paths contain no
   shell-metachars), critical on Windows.

2. **Stale PATH on first hermes launch after install.**  ``install.ps1``
   adds the PortableGit ``cmd`` / ``bin`` / ``usr\bin`` directories to the
   Windows **User** PATH via ``SetEnvironmentVariable(..., "User")``.  That
   write propagates to newly *spawned* processes only — already-running
   shells (including the one the user types ``hermes`` into immediately
   after install) retain their old PATH.  So hermes starts with a PATH that
   doesn't include bash, rg, grep, ssh — and ``search_files`` reports
   "rg/find not available" when the user clearly just installed them.

   Fix: new ``_augment_path_with_known_tools()`` helper called from
   ``configure_windows_stdio()`` on startup.  Prepends the Hermes-managed
   Git directories + the WinGet Links directory (where ripgrep lands) to
   ``os.environ['PATH']`` if they exist on disk but aren't already in
   PATH.  Subsequent subprocess calls (including bash spawns via
   ``_find_bash()``) inherit the augmented PATH and find everything.
   No-op on POSIX and when the directories don't exist.

3. **Root cause of "file content corruption".**  #1 was the proximate cause.
   Errors like ``C:/Users/.../hermes-snap-xxx.sh: No such file or directory``
   were emitted on stderr by the failed redirect, captured into stdout via
   ``stderr=subprocess.STDOUT``, and if the agent used terminal commands
   like ``cat > file`` the leaked error bytes became part of the file.
   Fixing #1 eliminates this entirely.

## Tests

All 77 Windows-compat tests still pass on Linux (POSIX path is
shlex.quote('/tmp/foo.sh') → '/tmp/foo.sh' — unchanged).

## Not addressed here (would need a bigger design)

- Python file tools (``write_file``, ``read_file``) and the bash-backed
  terminal tool see DIFFERENT views of ``/tmp`` on Windows.  Python treats
  ``/tmp`` as ``C:\tmp`` (drive-relative), Git Bash's MSYS2 treats it as
  a virtual mount to the PortableGit install's ``tmp\``.  Would need a
  translation shim in the Python tools to resolve bash-virtual paths to
  their native-Windows equivalents.  Workaround for users today: use
  absolute native paths (``C:\Users\you\...``) instead of ``/tmp/...``
  when crossing between terminal and Python file tools.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@KUSH42