feat(skill): make aso_skill a real, tested, installable Python package

[alirezarezvani]

Summary

Foundation-only refactor. No agent-contract changes — the four slash commands (/aso-full-audit, /aso-optimize, /aso-prelaunch, /aso-competitor), the outputs/[app-name]/ folder layout, and every legacy path inside app-store-optimization/ and .claude/skills/aso/ continue to work unchanged via symlinks.

In one sentence: rename the canonical source from the un-importable hyphenated app-store-optimization/ to a proper aso_skill/ Python package, add a pytest suite (50 tests), expose an aso CLI, fix the hardcoded 2025-11-07 reference date in load-bearing agent prompts, end the drift between the duplicate skill copies, and add a 24h iTunes cache.

Why this is needed

Three issues surfaced during the planning audit:

• Packaging was broken. app-store-optimization/ is not a valid Python module identifier. No __init__.py, no tests/. pyproject.toml declared a package that could not actually be pip installd.
• The agent contract was broken in main. aso-strategist.md:932 invokes python3 aso_scorer.py < /tmp/aso_input.json against a module with no __main__ block. The call was a silent no-op.
• The duplicate skill copy at .claude/skills/aso/ had drifted. Every analytic .py differed from its twin in app-store-optimization/. The drift had no enforcement mechanism.
• aso-strategist.md hardcoded Today's date: November 7, 2025 (lines 27, 237, 1097) — every timeline generated after Nov 2025 referenced past dates.

What changed

Area	Before	After
Canonical source	app-store-optimization/*.py (hyphenated, un-importable)	aso_skill/*.py (real package, pip install-able)
Legacy path	regular files	symlinks back to aso_skill/
.claude/skills/aso/	hand-maintained copy (drifted)	directory symlink to app-store-optimization/
__main__ blocks	absent (broken agent contract)	present on all 8 analytic modules
Tests	none	50 pytest tests, runs in <3s
CLI	none	aso score / optimize / validate / keywords / ab-test / checklist / itunes
iTunes API	uncached, cold every call	file-based 24h TTL cache with stale-fallback
Reference date in prompts	hardcoded 2025-11-07	dynamic via date -u +%Y-%m-%d

Backwards compatibility

• ZIP users who copy app-store-optimization/ to ~/.claude/skills/ keep working (symlinks resolve on cp -r).
• Agents that cd app-store-optimization && python3 <module>.py keep working — and now actually succeed end-to-end, because the underlying file finally has a __main__ block.
• scripts/materialize_compat.py replaces symlinks with real copies for Windows / archive-extraction users whose filesystem doesn't support them.
• Core install stays zero-dependency. pytest is the lone optional extra (pip install '.[test]').

Test plan

• pip install -e ".[test]" succeeds locally on Python 3.11
• pytest tests/ — 50 passed in 2.72s
• aso --help lists the seven subcommands
• aso validate --platform apple --field title --value "X"*50 exits 1
• aso validate --platform apple --field title --value "TaskFlow" exits 0
• aso score < tests/fixtures/score_input.json → overall_score: 91.5
• python3 app-store-optimization/aso_scorer.py < tests/fixtures/score_input.json works via symlink → identical score
• python3 .claude/skills/aso/aso_scorer.py < tests/fixtures/score_input.json works via two-hop symlink chain → identical score
• readlink -f app-store-optimization/aso_scorer.py → aso_skill/scorer.py
• No hardcoded Today's date: November 7, 2025 in load-bearing agent prompts (enforced by tests/test_dates.py)
• CI matrix (3.8–3.13) green on push
• Slash command smoke test: /aso-full-audit TestApp in a real Claude Code session produces the expected outputs/TestApp/ folder structure (manual)

https://claude.ai/code/session_01CbA3Mp4rD4vW9Yjd2aPEXw

---

Generated by Claude Code

[github-actions]

Invalid PR source

PRs to main must come from the dev branch only.

Current source: claude/github-actions-setup-YDKxe

Please:

1. Close this PR
2. Merge your changes into dev first
3. Use the Auto-Create Release PR (dev to main) workflow

[github-actions]

Large PR alert: this PR has 10642 changes. Consider splitting it into smaller PRs (<200 changes) for faster review and fewer conflicts.

[github-actions]

Large PR alert: this PR has 10645 changes. Consider splitting it into smaller PRs (<200 changes) for faster review and fewer conflicts.

[github-actions]

Merge conflict detected

This branch has conflicts with dev. Resolve them locally:

git fetch origin
git rebase origin/dev
# Resolve conflicts, then:
git rebase --continue
git push --force-with-lease

[claude]

Code Review — PR #10: feat(skill): make aso_skill a real, tested, installable Python package

Note: PR is currently in DRAFT state. Review is based on the diff as-is.

---

Overview

This is a solid foundation refactor that fixes three real bugs that were blocking correct agent operation:

1. app-store-optimization/ was not a valid Python module identifier — the package couldn't be pip install-ed
2. All 8 analytic modules lacked __main__ blocks — python3 aso_scorer.py < input.json (called by aso-strategist.md:932) silently did nothing
3. .claude/skills/aso/ had drifted from app-store-optimization/ with no enforcement mechanism
4. aso-strategist.md hardcoded Today's date: November 7, 2025 — every generated timeline after that date referenced past dates

The approach (symlinks for backwards compatibility, proper package in aso_skill/, 50-test pytest suite) is well-reasoned. No agent-contract changes is the right call.

---

Bugs / Must-Fix

1. beta variable removed but z_beta still referenced in aso_skill/ab_test.py

In calculate_sample_size, this line was removed:

# app-store-optimization/ab_test_planner.py (old)
alpha = 1 - self.CONFIDENCE_LEVELS[confidence_level]
beta = 1 - power  # ← DELETED in the PR

But z_beta still appears in the formula:

n_per_variant = math.ceil(
    ((z_alpha + z_beta) ** 2 * sd_pooled**2) / ...
)

If z_beta is still computed from beta = 1 - power somewhere in the un-diffed context this is fine, but if beta was the only source, z_beta is now a NameError at runtime. The test suite passes according to the PR, but calculate_sample_size may not be exercised with the A/B statistical path in the current tests. Please add an explicit test that calls calculate_sample_size and verify z_beta is defined.

2. aso itunes without subcommand gives a confusing error

it_sub = p_it.add_subparsers(dest="itunes_command")
# missing: required=True

Running aso itunes (without search or app) falls through to:

else:
    sys.stderr.write("Error: itunes requires a subcommand (search | app)\n")
    return 2

But only after checking --clear-cache. Add required=True to it_sub so argparse shows a proper help/error message.

---

ASO-Specific Concerns

Character limit validation — ✅ Correct and tested

test_metadata.py directly exercises every platform limit:

• Apple: title 30, subtitle 30, keywords 100, description 4000 ✓
• Google: title 50, short description 80, full description 4000 ✓
• Over-limit values return is_valid: False with "exceeds limit" in errors ✓
• The CI character-limit grep was updated from constant names (APPLE_TITLE_LIMIT = 30) to dict-form values ('title': 30), which correctly matches the new metadata.py structure ✓

Date regression guard — narrow regex

test_dates.py FORBIDDEN_LITERALS only catches November 7, 2025. A future contributor hardcoding a different date (e.g., January 15, 2026) won't be caught. Consider a broader pattern:

# Catches any hardcoded "Today's date: <Month> <day>, <year>"
re.compile(r"\bToday'?s\s+[Dd]ate:\s*\w+ \d{1,2}, 20\d{2}\b"),
re.compile(r"\bCurrent\s+Date:\s*\*?\*?\w+ \d{1,2}, 20\d{2}\b"),

aso-optimizer.md not in LOAD_BEARING_PROMPTS

test_dates.py checks strategist, master, research, and prelaunch — but not optimizer. Add it if it contains any date references.

---

Code Quality

iTunes cache — well-implemented

• XDG-compliant path resolution with Windows LOCALAPPDATA fallback ✓
• Graceful degradation: write failures, unreadable files, and permission errors all fall back silently ✓
• Stale-fallback on network failure is exactly right for this use case ✓
• SHA-1 truncated to 16 hex chars (64 bits) for URL keying: collision risk is negligible at this scale ✓
• Tests are network-free (urllib.request.urlopen patched) ✓

Minor: write-probe race condition in _ensure_writable

probe = self.cache_dir / ".write_probe"
probe.write_text("ok", encoding="utf-8")
probe.unlink()

Two concurrent processes can create the same probe file. Not a correctness issue (worst case: one process gets an OSError and marks the cache non-writable), but using tempfile.NamedTemporaryFile(dir=self.cache_dir) would be cleaner.

from __future__ import annotations usage

Present in cli.py, itunes_cache.py, materialize_compat.py but absent in competitors.py, keywords.py, ab_test.py. Inconsistent — pick one approach and apply uniformly.

Optional import removed inconsistently

Optional was removed from imports in ab_test.py and competitors.py but the itunes_cache.py still imports it. Fine since it's used there, just noting the pattern.

---

Security

• No hardcoded credentials or API keys ✓
• iTunes API is public, no auth required ✓
• urllib.request.urlopen with a 10s timeout prevents indefinite hangs ✓
• Input validation happens at system boundary (CLI) before dispatching to modules ✓
• json.loads on API responses — no shell eval or exec anywhere ✓

---

Documentation

CHANGELOG.md — follows Keep a Changelog format, clearly describes what changed and why ✓

CLAUDE.md needs updating — still references the old dual-copy architecture:

• Line: "See .claude/skills/aso/AGENT-INTEGRATION.md for detailed explanation" — that file was deleted in this PR
• The "Dual Structure Architecture" section still describes "Two Copies?" which no longer applies (it's now symlinks)
• The cp -r app-store-optimization/* .claude/skills/aso/ sync command is now incorrect

---

Test Coverage Assessment

Area	Tests	Quality
Character limits (Apple + Google)	test_metadata.py (12 tests)	✅ Exhaustive boundary tests
Scorer math	test_scorer.py (9 tests)	✅ Zero-input, high-input, weight sum
A/B significance	test_ab_test.py	⚠️ Verify z_beta path
CLI subprocess	test_cli.py	✅ Exit codes, stdin/stdout
iTunes cache	test_itunes_cache.py (6 tests)	✅ Cache hit, bypass, TTL, stale fallback
Legacy path compat	test_golden.py	✅ python3 app-store-optimization/aso_scorer.py
Date regression	test_dates.py	⚠️ Regex too narrow

time.sleep(1.1) in test_expired_entry_refetches and test_network_failure_falls_back_to_stale_cache is acceptable since the overall suite runs in <3s, but consider monkeypatch on time.time() if CI flakiness emerges.

---

Summary

Merge blockers:

1. Verify z_beta is defined after removing beta = 1 - power; add a direct test for calculate_sample_size if not covered
2. Add required=True to itunes subparser

Should fix before merge:
3. Broaden test_dates.py regex to catch any hardcoded date, not just Nov 7 2025
4. Update CLAUDE.md to remove stale dual-copy references

Nice to have:
5. Add aso-optimizer.md to test_dates.py load-bearing prompts list
6. Replace write-probe with tempfile approach in ITunesCache._ensure_writable
7. Consistent from __future__ import annotations across new modules

The core refactor is well-structured and the backwards-compatibility story is solid. Once the z_beta question is resolved and CLAUDE.md is updated, this is ready for a non-draft review.

[github-actions]

Large PR alert: this PR has 10658 changes. Consider splitting it into smaller PRs (<200 changes) for faster review and fewer conflicts.

[claude]

Code Review — PR #10: Make aso_skill a real, tested, installable Python package

Note: PR is in DRAFT state. Review is pre-merge; flagging blockers and nice-to-haves separately.

---

Overview

This is a well-motivated foundation refactor. It fixes three real bugs (broken __main__ contract, hardcoded 2025 dates, drift between two copies of the skill), and adds 50 tests and a proper CLI. The agent-contract backward-compatibility story via symlinks is sound. No agent-facing behaviour changes.

---

Blockers

1. CLAUDE.md is stale after this change

The ## ASO Agent System section still documents the old two-copy architecture:

.claude/skills/aso/ — hand-maintained copy (drifted)

That section now conflicts with the new symlink reality. Any contributor reading CLAUDE.md will follow the wrong mental model. The "Dual Structure Architecture" and "Keeping Synchronized" sub-sections should be updated to describe the symlink structure.

2. ab_test.py — silent removal of beta may mask a calculation error

In calculate_sample_size, the diff removes beta = 1 - power but z_beta is still referenced in the formula (z_alpha + z_beta) ** 2. If z_beta is defined in unchanged code above the diff window, this is fine — but if it was previously derived from beta, the sample size math now uses an undefined or stale value. Please confirm z_beta is independently computed and that the existing test test_sample_size_increases_as_effect_size_decreases actually exercises the power parameter.

3. Character limit CI check pattern mismatch

The CI validation step now checks for "'title': 30" in aso_skill/metadata.py. If the constants are stored as LIMITS = {"apple": {"title": 30, ...}}, the grep could match the wrong platform's "title": 30 key, or fail entirely if the dict uses a slightly different format. Make the grep target platform-specific, e.g. "apple.*title.*30" or validate both platforms separately.

---

ASO-Specific Concerns

Character limit test coverage gap

test_metadata.py tests Apple title (30), subtitle (30), keywords (100), description (4000), and both Google fields — but Apple Promotional Text (170 chars) has no test. Add:

def test_apple_promotional_text_170_chars():
    optimizer = MetadataOptimizer("apple")
    result = optimizer.validate_character_limits({"promotional_text": "X" * 170})
    assert result["is_valid"] is True

def test_apple_promotional_text_over_limit():
    optimizer = MetadataOptimizer("apple")
    result = optimizer.validate_character_limits({"promotional_text": "X" * 171})
    assert result["is_valid"] is False

{{ TODAY }} placeholders in aso-strategist.md

Replacing November 7, 2025 with {{TODAY}} and {{WEEK_1_MONDAY}} is the right fix, but if an agent renders the template without substituting the placeholders (e.g., due to a tool call failure), the output will literally contain {{TODAY}}. Consider adding a guard in test_dates.py that also checks the output files don't contain unresolved {{...}} after a test run — or at minimum document the expected agent substitution flow clearly.

---

Code Quality

time.sleep(1.1) in cache tests

test_expired_entry_refetches and test_network_failure_falls_back_to_stale_cache both do time.sleep(1.1) with TTL_SECONDS=1. This works but is inherently flaky on slow CI runners. Consider making TTL=0 and using monkeypatch.setattr(itunes_cache.time, "time", lambda: 9999999) to simulate time passing without actual sleep.

_ensure_writable probe has a TOCTOU race

probe = self.cache_dir / ".write_probe"
probe.write_text("ok", encoding="utf-8")
probe.unlink()

Two concurrent processes both probing writability will clobber each other's probe file. Low risk in practice, but tempfile.NamedTemporaryFile in the cache dir would be cleaner.

scripts/materialize_compat.py comment is misleading

Line 12443: # os.readlink works on Python 3.8; Path.readlink is 3.9+.

Since requires-python = ">=3.9" after this PR, the workaround is unnecessary. Either use Path.readlink(path) directly or drop the comment.

test_analyze_keyword_set_has_ranked_results is too loose

The assertion checks for any key containing "keyword", "rank", "top", or "summary" — this would pass even if the return structure was completely wrong. Tighten it to assert specific expected keys that the function is documented to return.

__main__ blocks on 7 of 8 modules are untested

test_cli.py:test_legacy_script_path_still_works only exercises aso_scorer.py. The other 7 modules' __main__ paths are never invoked in the test suite. Consider adding at least a smoke test for one more (e.g., metadata.py) to ensure the pattern works across the board.

---

Security

• No hardcoded credentials or API keys. ✅
• iTunes API URLs are constructed from user-controlled search terms but only used for cache file key hashing (SHA-1, non-cryptographic use) and URL construction — no shell execution risk. ✅
• cli.py reads files and JSON; no eval or shell injection paths. ✅
• Cache files are stored under user-controlled directories with graceful failure — acceptable. ✅

---

Documentation

• CHANGELOG follows Keep a Changelog format with clear Added/Changed/Fixed sections. ✅
• CLAUDE.md needs updating (see blocker Add Claude Code GitHub Workflow #1). ❌
• AGENT-INTEGRATION.md was correctly deleted since the symlink makes the "why two copies" explanation obsolete. ✅

---

Summary

Category	Status
Packaging fix (importable package)	✅
Backward compat via symlinks	✅
__main__ blocks added	✅
Hardcoded dates fixed + regression test	✅
iTunes 24h TTL cache	✅
50 tests, runs in <3s	✅
CLI with correct exit codes	✅
CLAUDE.md updated	❌ Blocker
beta removal in ab_test.py	⚠️ Needs confirmation
CI char-limit grep pattern	⚠️ Fragile
Apple promo text test coverage	⚠️ Missing

Three issues to resolve before merge, then this is solid foundation work.

[github-actions]

Branch claude/github-actions-setup-YDKxe was automatically deleted after merge.