fix: resolve issue #299 and Phase 1 cleanup
- 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 <noreply@anthropic.com>
This commit is contained in:
@@ -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",
|
||||
},
|
||||
},
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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."""
|
||||
|
||||
Reference in New Issue
Block a user