Skip to content

fix: remove duplicate countIssuesInJSONLFile function#1

Merged
maphew merged 1 commit into
mainfrom
fix-ci
Nov 29, 2025
Merged

fix: remove duplicate countIssuesInJSONLFile function#1
maphew merged 1 commit into
mainfrom
fix-ci

Conversation

@maphew

@maphew maphew commented Nov 29, 2025

Copy link
Copy Markdown
Owner

Fixes build failure in Test, Lint, Test (Windows), and Test Nix Flake jobs.

The function was defined in both init.go and doctor.go. Removed the init.go version which is now unused. The doctor.go version (which calls countJSONLIssues) is the canonical implementation.

Closes run #19780439467

Summary by CodeRabbit

  • Bug Fixes

    • Resolved hook installation failures when working with git worktrees. The system now correctly detects and follows the gitdir pointer to install hooks in the proper location.
  • Chores

    • Removed duplicate internal code helper function.

✏️ Tip: You can customize this high-level summary in your review settings.

Fixes build failure in Test, Lint, Test (Windows), and Test Nix Flake jobs.
The function was defined in both init.go and doctor.go. Removed the init.go
version which is now unused. The doctor.go version (which calls
countJSONLIssues) is the canonical implementation.

Fixes #19780439467
@maphew maphew merged commit 9381a20 into main Nov 29, 2025
2 of 4 checks passed
@coderabbitai

coderabbitai Bot commented Nov 29, 2025

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

Walkthrough

Added two issue entries documenting known problems: duplicate function elimination (bd-0e3) and git worktree hook installation failure (bd-63l). Removed the duplicate countIssuesInJSONLFile function from init.go, consolidating implementation and eliminating redundancy.

Changes

Cohort / File(s) Change Summary
Issue Documentation
.beads/issues.jsonl
Added two new issue entries: bd-0e3 tracking duplicate function removal and bd-63l documenting git worktree hook installation bug
Code Cleanup
cmd/bd/init.go
Removed duplicate countIssuesInJSONLFile helper function and its usage; canonical implementation retained elsewhere

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Straightforward function removal with no complex logic dependencies
  • Documentation-only changes to issue tracking file
  • Homogeneous pattern with no conflicting cross-file interactions

Poem

🐰 Hop, hop, away goes duplicate code,
One function remains on the well-trodden road,
Two issues now logged in the beads JSONL store,
Cleaner and simpler than before!

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-ci

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4ef5a28 and f134a3d.

📒 Files selected for processing (2)
  • .beads/issues.jsonl (2 hunks)
  • cmd/bd/init.go (0 hunks)

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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 mentioned this pull request Nov 29, 2025
maphew pushed a commit that referenced this pull request Feb 2, 2026
P0 #1: Conflict detection now happens BEFORE push to prevent data loss.
Previously: Pull -> Push -> Detect conflicts (wrong - conflicts overwritten)
Now: Pull -> Detect conflicts -> Push (skip conflicting issues)

P0 #2: Added SyncContext struct for thread-safe sync operations.
- SyncContext holds store, actor, dbPath, issueIDCounter
- WithContext variants of all sync functions
- globalContextIDCounter for cross-context uniqueness
- Enables concurrent sync operations without race conditions

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Executed-By: beads/crew/emma
Rig: beads
Role: crew
maphew pushed a commit that referenced this pull request Feb 6, 2026
Add flock-based shared/exclusive locking for JSONL reads (auto-import)
and writes (export/flush) to prevent corruption when sync and daemon
flush race.

**No new dependencies** - uses existing gofrs/flock v0.13.0 (already in go.mod)

Security fixes from SECURITY_AUDIT.md:
- Issue #1: Sync export and daemon auto-flush race causing data loss
- Issue #3: Auto-import reading partially-written JSONL during export

Changes:
- Add jsonl_lock.go with JSONLLock type using gofrs/flock
- Update exportToJSONLDeferred to acquire exclusive lock during export
- Update autoImportIfNewer to acquire shared lock during import
- Add comprehensive race condition tests
- Add generateUniqueTestID helper for test isolation

The JSONL lock ensures that:
- Export operations have exclusive access (no concurrent reads/writes)
- Import operations share access with other readers, block during writes
- Lock is held for entire export+commit+finalize sequence

Co-Authored-By: SageOx <ox@sageox.ai>
maphew pushed a commit that referenced this pull request Feb 15, 2026
…townhall#1706)

* perf(doctor): fix O(n) full-table scans causing 130s doctor runs

Three performance fixes for bd doctor on large databases (23k+ issues):

1. CheckDuplicateIssues (66s → 10ms): Replace SearchIssues() that loaded
   ALL issues into memory with SQL GROUP BY aggregation. The old code
   transferred 23k full issue rows (50+ columns) over MySQL wire protocol
   just to count duplicates.

2. CheckStaleClosedIssues (57s → 4ms): Replace SearchIssues() that loaded
   ALL closed issues with SELECT COUNT(*) SQL query. Same root cause as #1.

3. ResolvePartialID (60s+ → <1s for missing IDs): The substring search
   fallback loaded ALL issues when exact match failed. Now passes the hash
   as a search query to leverage SQL-level id LIKE filtering instead of
   transferring the entire database to Go for in-memory matching.

Total bd doctor runtime: 130s → 6s (22x speedup).
Total gt doctor runtime: infinite hang → 15s.

Root cause: These checks used store.SearchIssues() which does SELECT id
then GetIssuesByIDs() (full row fetch). On Dolt server mode with 23k+
issues, transferring all rows over MySQL wire protocol is catastrophically
slow. SQL aggregation and filtering avoid the data transfer entirely.

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

* test(doctor): add 7 missing tests for performance PR

CheckStaleClosedIssues (6 tests):
- DisabledSmallCount: threshold=0, <10k closed → OK
- DisabledLargeCount: threshold=0, ≥10k closed → warning
- EnabledWithCleanable: threshold=30d, old issues → correct count
- EnabledNoneCleanable: threshold=30d, recent issues → OK
- PinnedExcluded: all pinned → 0 cleanable
- MixedPinnedAndStale: 5 stale + 3 pinned → reports 5

CheckDuplicateIssues (2 tests):
- MultipleDuplicateGroups: 2+ groups → correct groupCount/dupCount
- ZeroDuplicatesNullHandling: SUM() NULL → defaults to 0

ResolvePartialID (1 test):
- TitleFalsePositive: hash in title but different ID → rejected

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

---------

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