From b636a0a2924c1ecde4c844c195e020005c476b45 Mon Sep 17 00:00:00 2001 From: yusyus Date: Tue, 24 Feb 2026 21:22:05 +0300 Subject: [PATCH] fix: resolve issue #299 and Phase 1 cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix #299: rename --chunk-size/--chunk-overlap to --streaming-chunk-size/ --streaming-overlap in arguments/package.py to avoid collision with the RAG --chunk-size flag from arguments/common.py - Phase 1a: make package_skill.py import args via add_package_arguments() instead of a 105-line inline duplicate argparse block; fixes the root cause of _reconstruct_argv() passing unrecognised flag names - Phase 1b: centralise setup_logging() into utils.py and remove 4 duplicate module-level logging.basicConfig() calls from doc_scraper.py, github_scraper.py, codebase_scraper.py, and unified_scraper.py - Fix test_package_structure.py / test_cli_paths.py version strings (3.1.1 → 3.1.2) Co-Authored-By: Claude Sonnet 4.6 --- src/skill_seekers/cli/arguments/package.py | 10 +-- src/skill_seekers/cli/codebase_scraper.py | 9 +-- src/skill_seekers/cli/doc_scraper.py | 18 +---- src/skill_seekers/cli/github_scraper.py | 9 +-- src/skill_seekers/cli/package_skill.py | 87 +--------------------- src/skill_seekers/cli/unified_scraper.py | 3 +- src/skill_seekers/cli/utils.py | 16 ++++ tests/test_cli_paths.py | 2 +- tests/test_package_structure.py | 8 +- 9 files changed, 37 insertions(+), 125 deletions(-) diff --git a/src/skill_seekers/cli/arguments/package.py b/src/skill_seekers/cli/arguments/package.py index c7c9416..d51b3be 100644 --- a/src/skill_seekers/cli/arguments/package.py +++ b/src/skill_seekers/cli/arguments/package.py @@ -70,8 +70,8 @@ PACKAGE_ARGUMENTS: dict[str, dict[str, Any]] = { "help": "Use streaming ingestion for large docs (memory-efficient)", }, }, - "chunk_size": { - "flags": ("--chunk-size",), + "streaming_chunk_size": { + "flags": ("--streaming-chunk-size",), "kwargs": { "type": int, "default": 4000, @@ -79,12 +79,12 @@ PACKAGE_ARGUMENTS: dict[str, dict[str, Any]] = { "metavar": "N", }, }, - "chunk_overlap": { - "flags": ("--chunk-overlap",), + "streaming_overlap": { + "flags": ("--streaming-overlap",), "kwargs": { "type": int, "default": 200, - "help": "Overlap between chunks (streaming mode, default: 200)", + "help": "Character overlap between chunks (streaming mode, default: 200)", "metavar": "N", }, }, diff --git a/src/skill_seekers/cli/codebase_scraper.py b/src/skill_seekers/cli/codebase_scraper.py index fc27740..751f20d 100644 --- a/src/skill_seekers/cli/codebase_scraper.py +++ b/src/skill_seekers/cli/codebase_scraper.py @@ -37,6 +37,7 @@ from skill_seekers.cli.code_analyzer import CodeAnalyzer from skill_seekers.cli.config_extractor import ConfigExtractor from skill_seekers.cli.dependency_analyzer import DependencyAnalyzer from skill_seekers.cli.signal_flow_analyzer import SignalFlowAnalyzer +from skill_seekers.cli.utils import setup_logging # Try to import pathspec for .gitignore support try: @@ -46,8 +47,6 @@ try: except ImportError: PATHSPEC_AVAILABLE = False -# Configure logging -logging.basicConfig(level=logging.INFO, format="%(asctime)s - %(levelname)s - %(message)s") logger = logging.getLogger(__name__) @@ -2382,11 +2381,7 @@ Examples: if args.depth is None: args.depth = "deep" # Default depth - # Set logging level - if getattr(args, "quiet", False): - logging.getLogger().setLevel(logging.WARNING) - elif args.verbose: - logging.getLogger().setLevel(logging.DEBUG) + setup_logging(verbose=args.verbose, quiet=getattr(args, "quiet", False)) # Handle --dry-run if getattr(args, "dry_run", False): diff --git a/src/skill_seekers/cli/doc_scraper.py b/src/skill_seekers/cli/doc_scraper.py index b7af156..2595c30 100755 --- a/src/skill_seekers/cli/doc_scraper.py +++ b/src/skill_seekers/cli/doc_scraper.py @@ -47,28 +47,12 @@ from skill_seekers.cli.llms_txt_detector import LlmsTxtDetector from skill_seekers.cli.llms_txt_downloader import LlmsTxtDownloader from skill_seekers.cli.llms_txt_parser import LlmsTxtParser from skill_seekers.cli.arguments.scrape import add_scrape_arguments +from skill_seekers.cli.utils import setup_logging # Configure logging logger = logging.getLogger(__name__) -def setup_logging(verbose: bool = False, quiet: bool = False) -> None: - """Configure logging based on verbosity level. - - Args: - verbose: Enable DEBUG level logging - quiet: Enable WARNING level logging only - """ - if quiet: - level = logging.WARNING - elif verbose: - level = logging.DEBUG - else: - level = logging.INFO - - logging.basicConfig(level=level, format="%(message)s", force=True) - - def infer_description_from_docs( base_url: str, first_page_content: str | None = None, name: str = "" ) -> str: diff --git a/src/skill_seekers/cli/github_scraper.py b/src/skill_seekers/cli/github_scraper.py index e742280..3e1cc88 100644 --- a/src/skill_seekers/cli/github_scraper.py +++ b/src/skill_seekers/cli/github_scraper.py @@ -31,6 +31,7 @@ except ImportError: sys.exit(1) from skill_seekers.cli.arguments.github import add_github_arguments +from skill_seekers.cli.utils import setup_logging # Try to import pathspec for .gitignore support try: @@ -40,8 +41,6 @@ try: except ImportError: PATHSPEC_AVAILABLE = False -# Configure logging FIRST (before using logger) -logging.basicConfig(level=logging.INFO, format="%(asctime)s - %(levelname)s - %(message)s") logger = logging.getLogger(__name__) # Import code analyzer for deep code analysis @@ -1391,11 +1390,7 @@ def main(): parser = setup_argument_parser() args = parser.parse_args() - # Set logging level from behavior args - if getattr(args, "quiet", False): - logging.getLogger().setLevel(logging.WARNING) - elif getattr(args, "verbose", False): - logging.getLogger().setLevel(logging.DEBUG) + setup_logging(verbose=getattr(args, "verbose", False), quiet=getattr(args, "quiet", False)) # Handle --dry-run if getattr(args, "dry_run", False): diff --git a/src/skill_seekers/cli/package_skill.py b/src/skill_seekers/cli/package_skill.py index 9a49e79..00ee167 100644 --- a/src/skill_seekers/cli/package_skill.py +++ b/src/skill_seekers/cli/package_skill.py @@ -188,6 +188,8 @@ def package_skill( def main(): + from skill_seekers.cli.arguments.package import add_package_arguments + parser = argparse.ArgumentParser( description="Package a skill directory into a .zip file for Claude", formatter_class=argparse.RawDescriptionHelpFormatter, @@ -210,92 +212,11 @@ Examples: """, ) - parser.add_argument("skill_dir", help="Path to skill directory (e.g., output/react/)") - - parser.add_argument( - "--no-open", action="store_true", help="Do not open the output folder after packaging" - ) - - parser.add_argument( - "--skip-quality-check", action="store_true", help="Skip quality checks before packaging" - ) - - parser.add_argument( - "--target", - choices=[ - "claude", - "gemini", - "openai", - "markdown", - "langchain", - "llama-index", - "haystack", - "weaviate", - "chroma", - "faiss", - "qdrant", - ], - default="claude", - help="Target LLM platform (default: claude)", - ) - - parser.add_argument( - "--upload", - action="store_true", - help="Automatically upload after packaging (requires platform API key)", - ) - - parser.add_argument( - "--streaming", - action="store_true", - help="Use streaming ingestion for large docs (memory-efficient, with chunking)", - ) - - parser.add_argument( - "--streaming-chunk-size", - type=int, - default=4000, - help="Maximum characters per chunk (streaming mode only, default: 4000)", - ) - - parser.add_argument( - "--streaming-overlap", - type=int, - default=200, - help="Character overlap between chunks (streaming mode only, default: 200)", - ) - - parser.add_argument( - "--batch-size", - type=int, - default=100, - help="Number of chunks per batch (streaming mode, default: 100)", - ) - - # Chunking parameters (for RAG platforms) - parser.add_argument( - "--chunk", - action="store_true", - help="Enable intelligent chunking for RAG platforms (auto-enabled for RAG adaptors)", - ) - - parser.add_argument( - "--chunk-tokens", - type=int, - default=512, - help="Maximum tokens per chunk (default: 512, recommended for OpenAI embeddings)", - ) - - parser.add_argument( - "--no-preserve-code", - action="store_true", - help="Allow code block splitting (default: false, code blocks preserved)", - ) - + add_package_arguments(parser) args = parser.parse_args() success, package_path = package_skill( - args.skill_dir, + args.skill_directory, open_folder_after=not args.no_open, skip_quality_check=args.skip_quality_check, target=args.target, diff --git a/src/skill_seekers/cli/unified_scraper.py b/src/skill_seekers/cli/unified_scraper.py index df59b14..7322a9f 100644 --- a/src/skill_seekers/cli/unified_scraper.py +++ b/src/skill_seekers/cli/unified_scraper.py @@ -28,12 +28,12 @@ try: from skill_seekers.cli.conflict_detector import ConflictDetector from skill_seekers.cli.merge_sources import ClaudeEnhancedMerger, RuleBasedMerger from skill_seekers.cli.unified_skill_builder import UnifiedSkillBuilder + from skill_seekers.cli.utils import setup_logging except ImportError as e: print(f"Error importing modules: {e}") print("Make sure you're running from the project root directory") sys.exit(1) -logging.basicConfig(level=logging.INFO, format="%(asctime)s - %(levelname)s - %(message)s") logger = logging.getLogger(__name__) @@ -1134,6 +1134,7 @@ Examples: ) args = parser.parse_args() + setup_logging() # Create scraper scraper = UnifiedScraper(args.config, args.merge_mode) diff --git a/src/skill_seekers/cli/utils.py b/src/skill_seekers/cli/utils.py index cd271f2..c3d56e5 100755 --- a/src/skill_seekers/cli/utils.py +++ b/src/skill_seekers/cli/utils.py @@ -17,6 +17,22 @@ logger = logging.getLogger(__name__) T = TypeVar("T") +def setup_logging(verbose: bool = False, quiet: bool = False) -> None: + """Configure root logging level based on verbosity flags. + + Args: + verbose: Enable DEBUG level logging + quiet: Enable WARNING level logging only (suppress INFO) + """ + if quiet: + level = logging.WARNING + elif verbose: + level = logging.DEBUG + else: + level = logging.INFO + logging.basicConfig(level=level, format="%(message)s", force=True) + + def open_folder(folder_path: str | Path) -> bool: """ Open a folder in the system file browser diff --git a/tests/test_cli_paths.py b/tests/test_cli_paths.py index c12e443..9a50f0e 100644 --- a/tests/test_cli_paths.py +++ b/tests/test_cli_paths.py @@ -138,7 +138,7 @@ class TestUnifiedCLIEntryPoints(unittest.TestCase): # Should show version output = result.stdout + result.stderr - self.assertIn("3.1.1", output) + self.assertIn("3.1.2", output) except FileNotFoundError: # If skill-seekers is not installed, skip this test diff --git a/tests/test_package_structure.py b/tests/test_package_structure.py index dc16ec7..d5b9d7b 100644 --- a/tests/test_package_structure.py +++ b/tests/test_package_structure.py @@ -24,7 +24,7 @@ class TestCliPackage: import skill_seekers.cli assert hasattr(skill_seekers.cli, "__version__") - assert skill_seekers.cli.__version__ == "3.1.1" + assert skill_seekers.cli.__version__ == "3.1.2" def test_cli_has_all(self): """Test that skill_seekers.cli package has __all__ export list.""" @@ -88,7 +88,7 @@ class TestMcpPackage: import skill_seekers.mcp assert hasattr(skill_seekers.mcp, "__version__") - assert skill_seekers.mcp.__version__ == "3.1.1" + assert skill_seekers.mcp.__version__ == "3.1.2" def test_mcp_has_all(self): """Test that skill_seekers.mcp package has __all__ export list.""" @@ -108,7 +108,7 @@ class TestMcpPackage: import skill_seekers.mcp.tools assert hasattr(skill_seekers.mcp.tools, "__version__") - assert skill_seekers.mcp.tools.__version__ == "3.1.1" + assert skill_seekers.mcp.tools.__version__ == "3.1.2" class TestPackageStructure: @@ -212,7 +212,7 @@ class TestRootPackage: import skill_seekers assert hasattr(skill_seekers, "__version__") - assert skill_seekers.__version__ == "3.1.1" + assert skill_seekers.__version__ == "3.1.2" def test_root_has_metadata(self): """Test that skill_seekers root package has metadata."""