Conversation
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
The load method is not robust against corrupted or malformed checkpoint files. If path.read_text() succeeds but the content is not valid JSON, json.loads will raise a JSONDecodeError. Similarly, if the JSON is valid but missing keys, CheckpointRecord(**data) will raise a TypeError. This would crash the pipeline. It's safer to wrap this logic in a try...except block to handle potential parsing errors, log a warning, and return None, treating the checkpoint as invalid. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Replacing the Unicode checkbox '🔲' with just spaces makes the list item for current objectives look incomplete. To maintain visual structure and adhere to the ASCII-only guidelines, it would be better to use an ASCII representation for an unchecked box, such as [ ]. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
The word 'swords' appears to be a typo or an incomplete replacement for the '⚔️' emoji. For consistency with other ASCII tags being introduced (like [COMBAT] in the action_icons dictionary on line 375), this header should use a similar tag format. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
While the '🎒' emoji was removed, the header for 'Inventory' now lacks the ASCII tag format that has been applied to most other headers in this file (e.g., [BOOK], [TARGET]). For consistency, consider adding a descriptive tag like [BAG] or [INVENTORY]. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
The _get_env_as_bool method handles None but not empty strings. If the environment variable is set to an empty string (e.g., MY_VAR=), it will evaluate to False because "" is not in the set of true values. This is inconsistent with _get_env_as_int, which correctly treats an empty string as a reason to use the default value. For consistency, an empty string should also cause _get_env_as_bool to fall back to its default. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Implements P1-FEATURE-003: Batch Processing feature with: - BatchProcessor class for sequential multi-file processing - Automatic checkpoint resumption for interrupted sessions - Progress reporting with rich progress bars - Batch summary reports in markdown format - CLI integration via `batch` command - Comprehensive test suite (17 tests) Also includes initial pipeline.py test coverage (12 tests). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Implements additional pipeline tests (10 new passing tests): - Checkpoint/Resume functionality (4 tests) - Checkpoint saving after stages - Resume from checkpoint skips completed stages - Resume disabled runs from beginning - Graceful handling of corrupted checkpoints - Output generation (3 tests) - All output files created - Output directory structure validation - Statistics included in output - Status tracking (3 tests) - Status JSON creation - Status updates per stage - Progress percentage calculation Also includes 7 existing passing tests: - Session directory management (3 tests) - Initialization (4 tests) Total: 26/36 tests implemented (10 skipped for future work) Note: Some tests require CheckpointManager mocking fix for Mock serialization. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
… tests and documentation
Contributor
Summary of ChangesHello @Gambitnl, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces significant enhancements to the application's robustness, usability, and internal architecture. The primary focus is on enabling checkpointing and resume capabilities for the audio processing pipeline, ensuring that long-running tasks can recover from interruptions without losing progress. Alongside this, a new batch processing feature has been added to the CLI, and the Gradio UI has undergone substantial modularization for better maintainability. New functionality for generating story narratives from processed sessions has also been integrated, complemented by extensive updates to documentation and the test suite to reflect these changes and improve overall code quality. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This is an excellent and comprehensive pull request that successfully implements a robust checkpoint/resume system for the processing pipeline. The addition of serialization methods to the data classes and the careful integration into pipeline.py are well-executed. The major refactoring of app.py into modular UI components and the extraction of story generation logic into StoryNotebookManager significantly improve the codebase's maintainability and structure. The new batch and generate-story CLI commands are valuable additions. Furthermore, the extensive updates to documentation, including new test plans and archiving of old files, demonstrate a strong commitment to project quality and clarity. I have one suggestion for improving error handling during checkpoint resumption, but overall, this is outstanding work.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
… Insights, Speaker Management, and Story Notebook tabs - Implemented Document Viewer tab for secure access to Google Docs via OAuth. - Created Help tab with setup instructions and usage guidelines. - Developed Import Notes tab for backfilling campaign sessions with extracted data. - Introduced LLM Chat tab for interactive conversations with configured character profiles. - Added Logs tab for viewing application logs and clearing old entries. - Implemented Social Insights tab for analyzing out-of-character banter and generating keyword clouds. - Created Speaker Management tab for mapping speaker IDs to actual names. - Developed Story Notebook tab for generating narratives from processed session transcripts.
…ction This commit introduces the automatic character profile extraction feature (P1-FEATURE-001).
Gambitnl
pushed a commit
that referenced
this pull request
Nov 12, 2025
…hods This refactoring addresses technical debt by breaking down the 98-line diarize() method into three focused, single-responsibility methods: ## Extracted Methods 1. **_load_audio_for_diarization()** (16 lines) - Loads audio using torchaudio with fallback to file path - Handles in-memory vs file-based loading - Clear error handling with debug logging 2. **_perform_diarization()** (19 lines) - Executes the pyannote pipeline - Converts results to SpeakerSegment objects - Returns both raw diarization and formatted segments - Improved logging with speaker count 3. **_extract_speaker_embeddings()** (66 lines) - Extracts voice embeddings for each speaker - Handles pydub import and audio loading errors gracefully - Processes all segments per speaker - Returns dict of speaker embeddings with detailed logging ## Refactored Main Method The main diarize() method (18 lines) now: - Clearly orchestrates the three-step pipeline - Has improved documentation with numbered steps - Maintains backward compatibility (same API) - Better separation of concerns ## Testing Added comprehensive unit tests (262 lines) covering: - Audio loading with torchaudio success and fallback - Diarization execution with various input types - Embedding extraction success and error cases - Empty segment handling - Import error handling - Partial failure scenarios (one speaker fails) ## Metrics - **Before**: 1 method with 98 lines - **After**: 4 methods averaging ~30 lines each - **Added Tests**: 13 new test cases for extracted methods - **Code Quality**: Improved maintainability and testability - **API**: No breaking changes - fully backward compatible ## Benefits - Each method has a single, clear responsibility - Easier to test individual steps independently - Better error handling and logging throughout - Simpler to debug and extend - Reduced cognitive load when reading code Fixes technical debt identified in refactoring candidate #6.
Gambitnl
added a commit
that referenced
this pull request
Nov 12, 2025
…21) * refactor(diarizer): extract complex diarize() method into focused methods This refactoring addresses technical debt by breaking down the 98-line diarize() method into three focused, single-responsibility methods: ## Extracted Methods 1. **_load_audio_for_diarization()** (16 lines) - Loads audio using torchaudio with fallback to file path - Handles in-memory vs file-based loading - Clear error handling with debug logging 2. **_perform_diarization()** (19 lines) - Executes the pyannote pipeline - Converts results to SpeakerSegment objects - Returns both raw diarization and formatted segments - Improved logging with speaker count 3. **_extract_speaker_embeddings()** (66 lines) - Extracts voice embeddings for each speaker - Handles pydub import and audio loading errors gracefully - Processes all segments per speaker - Returns dict of speaker embeddings with detailed logging ## Refactored Main Method The main diarize() method (18 lines) now: - Clearly orchestrates the three-step pipeline - Has improved documentation with numbered steps - Maintains backward compatibility (same API) - Better separation of concerns ## Testing Added comprehensive unit tests (262 lines) covering: - Audio loading with torchaudio success and fallback - Diarization execution with various input types - Embedding extraction success and error cases - Empty segment handling - Import error handling - Partial failure scenarios (one speaker fails) ## Metrics - **Before**: 1 method with 98 lines - **After**: 4 methods averaging ~30 lines each - **Added Tests**: 13 new test cases for extracted methods - **Code Quality**: Improved maintainability and testability - **API**: No breaking changes - fully backward compatible ## Benefits - Each method has a single, clear responsibility - Easier to test individual steps independently - Better error handling and logging throughout - Simpler to debug and extend - Reduced cognitive load when reading code Fixes technical debt identified in refactoring candidate #6. * refactor: Address code review feedback - improve type hints and test maintainability Based on code review from gemini-code-assist bot, this commit makes the following improvements: ## Type Hints Enhancement 1. **Added Union to typing imports** - Enables proper type annotation for methods accepting multiple types 2. **_load_audio_for_diarization return type** - Added `-> Union[Dict, str]` return type hint - Clearly indicates the method can return either a dict or string 3. **_perform_diarization parameter type** - Added `diarization_input: Union[Dict, str]` type hint - Makes the dual-mode input explicit in the signature 4. **_extract_speaker_embeddings parameter type** - Added `diarization: Any` type hint - Documents that this accepts pyannote pipeline results ## Exception Handling Improvement 5. **More specific exception catching** - Changed `except Exception` to `except ImportError` for pydub import - Prevents masking of other unexpected errors - Makes error handling intent clearer ## Test Code Quality 6. **Extracted common mock setup to fixture** - Created `mock_audio_segment_setup` pytest fixture - Reduces ~60 lines of duplicated mock configuration - Refactored `test_extract_speaker_embeddings_successful` - Refactored `test_extract_speaker_embeddings_handles_inference_error` - Improves test maintainability and readability ## Benefits - Better static type checking support - Clearer API contracts through type hints - More maintainable and DRY test code - More robust exception handling - Improved code readability All changes maintain backward compatibility. --------- Co-authored-by: Claude <noreply@anthropic.com>
Gambitnl
pushed a commit
that referenced
this pull request
Nov 14, 2025
Add completion summary to the diarizer refactoring task document and update the refactoring README to track progress. Changes: - Add detailed implementation summary to 06-diarizer-complex-method.md - Document what was implemented (2 extracted methods) - Record actual effort (2 hours vs 10-12 estimated) - Note deviations from original plan - Include metrics showing 30% code reduction - List 6 new unit tests added - Update refactoring README.md - Mark task #6 as completed with ✅ indicator - Add completion summary with key metrics - Update summary statistics to show 1/10 completed - Update last modified date to 2025-11-14 No architecture documentation updates needed - the refactoring was internal to the SpeakerDiarizer class and doesn't change the public API or high-level design documented in DEVELOPMENT.md.
Gambitnl
added a commit
that referenced
this pull request
Nov 14, 2025
* refactor: Extract methods from SpeakerDiarizer.diarize() for better maintainability Extract complex logic from _extract_speaker_embeddings() method into smaller, focused methods to improve code readability and testability. Changes: - Extract _load_audio_for_embeddings(): Handles pydub audio loading with proper error handling for embedding extraction - Extract _extract_single_speaker_embedding(): Extracts embedding for a single speaker, including audio concatenation and inference - Simplify _extract_speaker_embeddings(): Now orchestrates the extraction process by calling the new helper methods The refactoring reduces the complexity of _extract_speaker_embeddings() from 81 lines to 57 lines, making it easier to understand and maintain. Each extracted method has a single, well-defined responsibility. Testing: - Add 6 new unit tests for the extracted methods - Tests cover success cases, error handling, and edge cases - Syntax validation passed for all modified files This completes task #2 from the refactoring plan (06-diarizer-complex-method.md). * docs: Update refactoring documentation for completed task #6 Add completion summary to the diarizer refactoring task document and update the refactoring README to track progress. Changes: - Add detailed implementation summary to 06-diarizer-complex-method.md - Document what was implemented (2 extracted methods) - Record actual effort (2 hours vs 10-12 estimated) - Note deviations from original plan - Include metrics showing 30% code reduction - List 6 new unit tests added - Update refactoring README.md - Mark task #6 as completed with ✅ indicator - Add completion summary with key metrics - Update summary statistics to show 1/10 completed - Update last modified date to 2025-11-14 No architecture documentation updates needed - the refactoring was internal to the SpeakerDiarizer class and doesn't change the public API or high-level design documented in DEVELOPMENT.md. * refactor: Improve type hints and optimize imports in diarizer Address code review feedback from gemini-code-assist to improve code quality, type safety, and performance. Changes: 1. Type Safety Improvements - Add TYPE_CHECKING imports for AudioSegment and Annotation - Replace Optional[Any] with Optional['AudioSegment'] in _load_audio_for_embeddings() - Replace Any with 'Annotation' and 'AudioSegment' type hints in _extract_single_speaker_embedding() - Update _perform_diarization() return type to use 'Annotation' - Update _extract_speaker_embeddings() parameter type 2. Documentation Accuracy - Fix Raises clause in _extract_single_speaker_embedding() to specify RuntimeError instead of generic Exception 3. Performance Optimization - Replace `from pydub import AudioSegment` import inside loop with `type(audio).empty()` to avoid repeated module lookups - This removes import overhead from the per-speaker iteration These improvements enhance static analysis capabilities, provide better IDE autocomplete support, and reduce unnecessary import overhead in the embedding extraction loop. --------- Co-authored-by: Claude <noreply@anthropic.com>
Gambitnl
pushed a commit
that referenced
this pull request
Nov 14, 2025
Resolved merge conflict by acknowledging that both refactorings #6 and #8 have been completed: - Task #6 (Diarizer Complex Method) - completed on main branch - Task #8 (Campaign Artifact Counting) - completed on this branch Changes: - Marked task #6 as completed with status and completion notes - Updated Summary Statistics: 2 of 10 completed (was 1 of 10) - Added actual effort tracking: ~4 hours total - Updated status line to list both completed tasks - Added "Next Priority" guidance for Phase 1 tasks This resolves the upcoming merge conflict when this branch is merged to main or when a PR is created.
Gambitnl
added a commit
that referenced
this pull request
Nov 14, 2025
* feat: Enhance CampaignArtifactCounter with additional features Enhanced the CampaignArtifactCounter class to align with refactoring documentation specifications. The core extraction was already complete, but added additional useful features for better usability. Changes to src/artifact_counter.py: - Enhanced ArtifactCounts dataclass with session_ids and narrative_paths lists - Added session_count and narrative_count properties for backward compatibility - Added total_artifacts computed property - Enhanced to_dict() to include new fields - Updated _count_session_artifacts to track session IDs and narrative paths - Added count_sessions() convenience method - Added count_narratives() convenience method - Added get_all_campaigns() to list all campaigns with artifacts - Added get_campaign_summary() for detailed campaign information Changes to tests/test_artifact_counter.py: - Added tests for new properties (session_count, narrative_count, total_artifacts) - Added test for session_ids and narrative_paths tracking - Added tests for count_sessions() convenience method - Added tests for count_narratives() convenience method - Added tests for get_all_campaigns() method - Added tests for get_campaign_summary() method - Added tests to verify session IDs and narrative paths are tracked correctly All changes maintain backward compatibility with existing code. The original implementation already had proper error handling, caching, and testability as required by the refactoring task. Related: docs/refactoring/08-campaign-artifact-counting.md * docs: Update refactoring documentation for completed task #8 Updated documentation to reflect completion of campaign artifact counter extraction and enhancement: Changes to docs/refactoring/08-campaign-artifact-counting.md: - Added completion status banner at the top - Added comprehensive Implementation Notes section documenting: - What was already complete (core extraction) - Enhancements added (convenience methods, query methods, tracking) - Usage examples for all new features - Actual vs estimated effort (2 hours vs 9-13 hours) - Success criteria validation - Benefits realized - Integration details - Files modified - Lessons learned Changes to docs/refactoring/README.md: - Marked task #8 as completed with checkmark - Added completion date and status - Added completion notes summarizing enhancements - Updated summary statistics to show 1 of 10 completed - Updated "Last Updated" date to 2025-11-14 The documentation now provides: - Clear completion status for tracking progress - Detailed implementation notes for future reference - Usage examples for developers - Lessons learned for future refactorings * refactor: Address code review feedback for CampaignArtifactCounter Implemented three improvements based on Gemini code review: 1. Performance: Added caching to get_all_campaigns() - Previously performed filesystem scan on every call - Now uses double-checked locking pattern with TTL-based cache - Added use_cache parameter (default: True) for cache control - Prevents performance bottlenecks when called frequently (e.g., UI dropdowns) 2. Code duplication: Refactored get_campaign_summary() to use to_dict() - Eliminated duplicate dictionary construction logic - Now leverages ArtifactCounts.to_dict() as base - Augments with campaign_id and errors fields - Renames keys for API consistency (sessions -> session_count, etc.) - Follows DRY principle for easier maintenance 3. Code quality: Removed redundant import in tests - Removed inline 'from pathlib import Path' in test_narrative_paths_tracked - Path already imported at module level (line 6) - Improves readability per PEP 8 guidelines Additional improvements: - Updated clear_cache() to also clear campaigns list cache - Enhanced get_cache_stats() to include campaigns_list_cached status - Added comprehensive tests for new caching behavior: - test_get_all_campaigns_caching: Verifies cache works correctly - test_clear_cache_clears_campaigns_list: Verifies cache clearing - test_get_campaign_summary_uses_to_dict: Verifies refactored logic All changes tested and verified with manual integration tests. Related review: gemini-code-assist bot feedback * docs: Update refactoring README to acknowledge both completions Resolved merge conflict by acknowledging that both refactorings #6 and #8 have been completed: - Task #6 (Diarizer Complex Method) - completed on main branch - Task #8 (Campaign Artifact Counting) - completed on this branch Changes: - Marked task #6 as completed with status and completion notes - Updated Summary Statistics: 2 of 10 completed (was 1 of 10) - Added actual effort tracking: ~4 hours total - Updated status line to list both completed tasks - Added "Next Priority" guidance for Phase 1 tasks This resolves the upcoming merge conflict when this branch is merged to main or when a PR is created. --------- Co-authored-by: Claude <noreply@anthropic.com>
Gambitnl
pushed a commit
that referenced
this pull request
Nov 14, 2025
Summary of Changes: - Updated MASTER_PLAN.md: Moved 6 completed bugs to Done section - Updated TEST_PLANS.md: Marked P0-2, P2-1, P2-2 as completed with summaries - Updated refactoring/README.md: Marked #5, #6, #8, #9 as completed - Created CONCURRENT_ORCHESTRATION_SPRINT_2025-11-14.md: Comprehensive sprint report Sprint Results: - 15 concurrent tasks completed across 3 batches - 220+ tests added, 99% coverage for party_config.py - ~5,000 lines of production code - Zero merge conflicts maintained - 100% task independence validated Documentation Updates: - 6 bugs moved from "To Do" to "Done" in MASTER_PLAN.md - 3 test plans marked complete with implementation summaries - 4 refactoring tasks marked complete with actual effort tracking - Added comprehensive sprint report with metrics and lessons learned Files Modified: - docs/MASTER_PLAN.md - docs/TEST_PLANS.md - docs/refactoring/README.md - docs/CONCURRENT_ORCHESTRATION_SPRINT_2025-11-14.md (new)
Gambitnl
added a commit
that referenced
this pull request
Nov 14, 2025
Summary of Changes: - Updated MASTER_PLAN.md: Moved 6 completed bugs to Done section - Updated TEST_PLANS.md: Marked P0-2, P2-1, P2-2 as completed with summaries - Updated refactoring/README.md: Marked #5, #6, #8, #9 as completed - Created CONCURRENT_ORCHESTRATION_SPRINT_2025-11-14.md: Comprehensive sprint report Sprint Results: - 15 concurrent tasks completed across 3 batches - 220+ tests added, 99% coverage for party_config.py - ~5,000 lines of production code - Zero merge conflicts maintained - 100% task independence validated Documentation Updates: - 6 bugs moved from "To Do" to "Done" in MASTER_PLAN.md - 3 test plans marked complete with implementation summaries - 4 refactoring tasks marked complete with actual effort tracking - Added comprehensive sprint report with metrics and lessons learned Files Modified: - docs/MASTER_PLAN.md - docs/TEST_PLANS.md - docs/refactoring/README.md - docs/CONCURRENT_ORCHESTRATION_SPRINT_2025-11-14.md (new) Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.