Skip to content

chore: remove redundant inline comments across all crates#13

Merged
tlongwell-block merged 1 commit into
mainfrom
tyler/remove-llm-comments
Mar 10, 2026
Merged

chore: remove redundant inline comments across all crates#13
tlongwell-block merged 1 commit into
mainfrom
tyler/remove-llm-comments

Conversation

@tlongwell-block
Copy link
Copy Markdown
Collaborator

Summary

Remove ~120 inline comments that restate what self-evident code already says, without explaining why. Zero code or functionality changes — only comment lines deleted.

34 files changed across 12 crates · 164 lines removed

What was removed

Comments that narrate the next line of code:

// Create the collection.          ← before create_collection()
// Parse optional type filter.     ← before a match on query params
// Clean up.                       ← before a delete call
// 1. Evaluate condition.          ← numbered step labels
// Send the request.               ← test narration
// Fetch the feed.                 ← before client.get(&feed_url)

What was kept

  • All /// and //! doc comments — untouched
  • Section separators (// ── Helpers ───────)
  • WHY explanations (SSRF rationale, backoff reset logic, race condition notes, TOCTOU warnings)
  • Safety & security notes (⚠️ SECURITY: Do NOT log auth_token)
  • Non-obvious behavior (JSON_CONTAINS semantics, NIP protocol references, type cast explanations)
  • TODO / FIXME / HACK / NOTE markers
  • Test scenario descriptions that add real clarity

Verification

  • cargo check
  • Diff is comment-only — the only two non-pure-comment-line changes are trailing inline comments removed from the end of existing code lines (the code on those lines is identical before and after)

Breakdown by crate

Crate Files Comments Removed
sprout-test-client 4 ~27
sprout-relay 13 ~25
sprout-workflow 3 24
sprout-db 4 ~19
sprout-huddle 2 8
sprout-mcp 2 5
sprout-core 1 4
sprout-auth 2 3
sprout-search 1 3
sprout-proxy 1 2
sprout-audit 1 1
sprout-pubsub 0 0 (already clean)

Strip ~160 inline comments that restate what self-evident code does
without explaining WHY. No code or functionality changes.

Kept: doc comments, section separators, safety/security notes,
WHY explanations, edge case warnings, non-obvious behavior notes,
TODO/FIXME markers, and test scenario descriptions.

34 files changed across 12 crates.
@tlongwell-block tlongwell-block force-pushed the tyler/remove-llm-comments branch from a395a8c to 5980257 Compare March 10, 2026 15:40
@tlongwell-block tlongwell-block merged commit 431c9f2 into main Mar 10, 2026
8 checks passed
@tlongwell-block tlongwell-block deleted the tyler/remove-llm-comments branch March 10, 2026 16:06
tlongwell-block added a commit that referenced this pull request Mar 11, 2026
* origin/main:
  feat: soft-delete for events/channels, enriched API responses, NIP-29 group management (#17)
  feat: Channel management, messaging, threads, DMs, reactions, and NIP-29 support (#16)
  Improve chat scrolling and multiline composer (#14)
  chore: remove redundant inline comments across all crates (#13)
  Initial backend revisions, workflow expansion (#5)
  Add desktop Home feed (#12)
  Add desktop Playwright e2e harness (#11)
  Update desktop icon and persist window state (#9)
  feat: add channel creation flow (#8)
wpfleger96 added a commit that referenced this pull request May 22, 2026
…iew findings

The original implementation created a second parallel Tauri command
(discover_all_acp_providers) alongside the existing one to avoid
changing the return type. This produced two commands, two hooks, two
query keys, and two raw type converters. Consolidates into a single
command returning the full catalog, with a useAvailableAcpProviders
hook that type-narrows for callers needing non-null command/binaryPath.

Also fixes: pipe deadlock in install command (#1), UTF-8 truncation
panic (#2/#4), adds install concurrency guard (#11), exact provider ID
match (#15), error display stdout fallback (#5), success banner
suppression when already available (#12), misleading re-run text (#13),
IIFE refactor in PersonaDialog (#14), hidden internal query lift (#7),
configurable e2e mocks (#9), shared raw type exports (#8), and
classify_provider unit tests (#10).
wpfleger96 added a commit that referenced this pull request May 22, 2026
…iew findings

The original implementation created a second parallel Tauri command
(discover_all_acp_providers) alongside the existing one to avoid
changing the return type. This produced two commands, two hooks, two
query keys, and two raw type converters. Consolidates into a single
command returning the full catalog, with a useAvailableAcpProviders
hook that type-narrows for callers needing non-null command/binaryPath.

Also fixes: pipe deadlock in install command (#1), UTF-8 truncation
panic (#2/#4), adds install concurrency guard (#11), exact provider ID
match (#15), error display stdout fallback (#5), success banner
suppression when already available (#12), misleading re-run text (#13),
IIFE refactor in PersonaDialog (#14), hidden internal query lift (#7),
configurable e2e mocks (#9), shared raw type exports (#8), and
classify_provider unit tests (#10).
wpfleger96 added a commit that referenced this pull request May 22, 2026
…iew findings

The original implementation created a second parallel Tauri command
(discover_all_acp_providers) alongside the existing one to avoid
changing the return type. This produced two commands, two hooks, two
query keys, and two raw type converters. Consolidates into a single
command returning the full catalog, with a useAvailableAcpProviders
hook that type-narrows for callers needing non-null command/binaryPath.

Also fixes: pipe deadlock in install command (#1), UTF-8 truncation
panic (#2/#4), adds install concurrency guard (#11), exact provider ID
match (#15), error display stdout fallback (#5), success banner
suppression when already available (#12), misleading re-run text (#13),
IIFE refactor in PersonaDialog (#14), hidden internal query lift (#7),
configurable e2e mocks (#9), shared raw type exports (#8), and
classify_provider unit tests (#10).
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.

1 participant