Skip to content

chore: remove practice and pulse prototype feeds#65

Merged
vishnujayvel merged 1 commit into
mainfrom
chore/remove-practice-pulse-feeds
Mar 9, 2026
Merged

chore: remove practice and pulse prototype feeds#65
vishnujayvel merged 1 commit into
mainfrom
chore/remove-practice-pulse-feeds

Conversation

@vishnujayvel

@vishnujayvel vishnujayvel commented Mar 9, 2026

Copy link
Copy Markdown
Owner

Summary

Remove practice and pulse feed producers — these were prototypes that are no longer part of the product.

  • Remove PracticeFeedConfig, PulseFeedConfig, PulseThresholds structs from types
  • Remove PracticeProducer and PulseProducer from builtin feeds (8 producers → 6)
  • Remove default config values and polling logic for both feeds
  • Remove renderPulseSegment() and pulse case from status-line rendering
  • Update all affected tests (config defaults, feed registration count, placeholder lists)

Net result: -155 lines across 8 files.

Test plan

  • go build ./... compiles cleanly
  • go test -race ./... passes all 12 packages
  • make install + hookwise doctor runs clean
  • No references to practice/pulse remain in production code

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Changes
    • The pulse status indicator, which displayed session count information, has been removed from the application status line
    • Support for pulse and practice feed configurations has been discontinued; available feeds now focus on core integrations including project, calendar, weather, news, memories, and insights

These were placeholder feed producers that are no longer part of the
product. Removes PulseProducer, PracticeProducer, PulseFeedConfig,
PracticeFeedConfig, PulseThresholds types, and all associated config
defaults, CLI rendering code, and test references.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Mar 9, 2026

Copy link
Copy Markdown
Warnings
⚠️ Architecture-critical files changed. Consider updating arch tests.
⚠️

No issue reference found in PR description. Add Closes #XX or No issue.

Generated by 🚫 dangerJS against aa4af53

@coderabbitai

coderabbitai Bot commented Mar 9, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This pull request removes Pulse and Practice feed support across the codebase, including configuration types, default settings, builtin producers, rendering logic, and related tests. The changes eliminate these feed sources from the CLI status-line display and configuration management system.

Changes

Cohort / File(s) Summary
Configuration & Type Definitions
internal/core/types.go, internal/core/config.go, internal/core/config_test.go
Removed PulseFeedConfig, PulseThresholds, and PracticeFeedConfig types; deleted Pulse and Practice fields from FeedsConfig struct; removed default configurations for both feeds; updated test assertions to no longer validate Pulse feed defaults or parsing.
Producer Implementation
internal/feeds/builtin.go
Removed PulseProducer and PracticeProducer types with their Name() and Produce() method implementations; updated comments to reflect 6 instead of 8 builtin producers.
Feed Polling & Dispatch
internal/feeds/polling.go
Removed intervalFor and isEnabled case handlers for "pulse" and "practice" feeds, causing them to use default interval and treat as unknown/enabled feeds.
Rendering Logic
cmd/hookwise/main.go
Deleted case "pulse" from renderBuiltinSegment switch; removed entire renderPulseSegment function that extracted session count and returned colored output.
Test Updates
cmd/hookwise/cli_test.go, internal/feeds/feeds_test.go
Removed pulse entries from no-data and placeholder test scenarios; replaced all pulse references with "project" in feed-related tests; updated expected builtin count from 8 to 6 producers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 76.92% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'chore: remove practice and pulse prototype feeds' clearly and concisely summarizes the main change of removing two prototype feed producers.
Description check ✅ Passed The description includes all required template sections: Summary with clear bullet points, Motivation explaining why changes were made, comprehensive Test plan with checked items, and Checklist items completed.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/remove-practice-pulse-feeds

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
internal/feeds/feeds_test.go (1)

352-365: 🧹 Nitpick | 🔵 Trivial

Update stale comment in test function.

The comment on line 352 still says "Test 10: RegisterBuiltins registers all 8 producers" but the test now expects 6 producers. The comment should be updated for consistency.

📝 Proposed comment fix
 // ---------------------------------------------------------------------------
-// Test 10: RegisterBuiltins registers all 8 producers
+// Test 10: RegisterBuiltins registers all 6 producers
 // ---------------------------------------------------------------------------
 
 func TestRegisterBuiltins_All8(t *testing.T) {

Note: The function name TestRegisterBuiltins_All8 could also be renamed to TestRegisterBuiltins_All6 for full consistency, though this is optional as Go test names don't affect functionality.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/feeds/feeds_test.go` around lines 352 - 365, Update the stale
comment and optionally the test name to match the current expected count: change
the comment that reads "Test 10: RegisterBuiltins registers all 8 producers" to
reflect 6 producers, and optionally rename the test function
TestRegisterBuiltins_All8 to TestRegisterBuiltins_All6 for clarity; ensure the
test still calls NewRegistry(), RegisterBuiltins(r) and asserts len(r.All()) ==
6 and that expectedNames remain the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@internal/feeds/feeds_test.go`:
- Around line 352-365: Update the stale comment and optionally the test name to
match the current expected count: change the comment that reads "Test 10:
RegisterBuiltins registers all 8 producers" to reflect 6 producers, and
optionally rename the test function TestRegisterBuiltins_All8 to
TestRegisterBuiltins_All6 for clarity; ensure the test still calls
NewRegistry(), RegisterBuiltins(r) and asserts len(r.All()) == 6 and that
expectedNames remain the same.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 33f5d2fa-3018-44fc-aca7-4ee7ffc97363

📥 Commits

Reviewing files that changed from the base of the PR and between b1d6255 and aa4af53.

📒 Files selected for processing (8)
  • cmd/hookwise/cli_test.go
  • cmd/hookwise/main.go
  • internal/core/config.go
  • internal/core/config_test.go
  • internal/core/types.go
  • internal/feeds/builtin.go
  • internal/feeds/feeds_test.go
  • internal/feeds/polling.go
💤 Files with no reviewable changes (4)
  • cmd/hookwise/main.go
  • internal/core/types.go
  • internal/core/config.go
  • internal/feeds/polling.go

@vishnujayvel vishnujayvel merged commit e0ac95d into main Mar 9, 2026
4 checks passed
vishnujayvel pushed a commit that referenced this pull request Mar 9, 2026
Accept Go-based content from main for CONTRIBUTING.md, SECURITY.md, and
docs/cli.md (replacing stale TypeScript references). Accept deletion of
src/cli/commands/init.tsx, src/core/tui-launcher.ts, and
tests/core/tui-launcher.test.ts (removed by PR #65).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
vishnujayvel pushed a commit that referenced this pull request Mar 9, 2026
Resolve conflicts after PR #65 removed practice and pulse feeds.
- Remove pulse/practice cases from getFeedInterval switch
- Keep custom feeds iteration and insights_ prefix logic from PR #64
- Update TestDoctorFeedHealthPlaceholder to use project feed instead of practice
- Keep testify assert/require style from PR #64

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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