From 2beade67f20ae4e76de670a2a31287ab13c076bf Mon Sep 17 00:00:00 2001 From: inliver233 Date: Tue, 22 Sep 2026 01:37:51 +0800 Subject: [PATCH] Reconstruction pass: make the gates tell the truth, make the package self-contained, keep the one-hop symptom index MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An external audit of this repository landed on four structural findings, and this pass is the response to them. Everything below was re-measured on this tree before being acted on; two of the audit's claims needed a correction of my own, and both corrections are noted in place. **The gates were optimistic, and one of them did not read content at all.** doctor.py the capability table was hard-coded (`repack + sign = bool(java) and python`), so a machine with a JDK and no zipalign/apksigner was told it could re-sign; the script inventory was a hand-written list of 28 against 51 files on disk, omitting the nine most important scripts. It now scans the directory (`registered/checked = 52 / 52`, unjudgeable entries counted as `unknown`, never skipped) and resolves capability as a real requires-closure, reporting BLOCKED with the missing piece and an installable next step. Measured: the old doctor reports `[OK] repack + sign` on a JDK-only PATH where keytool, jarsigner, zipalign, apksigner and all eight smali jars are absent. The report is allowed to look bad; that is the point. check_commands.py (new) anchor checks prove a section exists, never that a command runs. This parses each script's argparse table with `ast` and checks every documented invocation against it. 146 commands / 41 interfaces, 0 drift. It caught `sig_probe.py --live --pkg`, which does not exist -- the reference had been wrong since it was written. check_routing.py (new) the symptom index now has one home (`SKILL.md`, because symptom -> file must stay one hop) and the inventory has another (`references/routing.md`). This gate proves the mirror agrees, that every reference file is named, and that every script is named. **so_constpatch.py was corrupting APKs it rebuilt.** The writer copied filename and timestamp onto a fresh ZipInfo and recompressed the whole archive: 435-entry real package, `extra` lost on 205 entries, `external_attr` rewritten on all 435, 132 untouched entries recompressed, and `resources.arsc` pushed to an unaligned offset. Android's own `zipalign -c -v 4` fails the old output and passes the new one; a 26-case fixture suite pins it, and `extractNativeLibs="false"` is now refused by default with a pointer at `repack.py`. Fixing it exposed the same defect in `repack.py`, where `^lib/[^/]+\.so$` cannot match `lib/arm64-v8a/*.so` -- so its alignment handling had never once fired on a real APK. **The package was not self-contained.** 59 references pointed at the repository root, which `npx skills add` does not install. They now converge on `references/evidence-summary.md` (capability, conclusion, strength, evidence that ships) plus `evidence/` with the machine-readable companions: 8 references remain and each is explicitly labelled "repository root, not shipped". A drill in which the new files were copied to an isolated directory and asked three questions answered all three. **SKILL.md: 572 lines / 15,733 tokens -> 398 lines / ~10,900 tokens**, without losing the two things that had to stay. The symptom index remains inline; the reference and script indexes moved to the on-demand routing file. The freed budget went into making the rules *stronger*, not shorter: the constraints section is now a table of thirteen non-negotiable constraints each paired with the check that catches a violation, and the tooling obligations are instructions whose violation is a defect (hedging language in SKILL.md: 1 line, down from a paragraph of hedged advice). Long references gained a navigation head so a reader can decide from the top whether to load one. **New: an executable test layer.** `tests/` (unit, CLI contract, no-device integration) plus `pytest.ini`, `tests/run_tests.py`, a minimal CI job, and `evals/evals.json` written but never run -- stated as such in the file. 608 passed / 158 skipped / 25 xfailed. The 25 xfails are a work order, not a pass: thirteen scripts report a missing input by exiting 1 instead of 2, seven let a traceback escape, four treat `--help` as an error, and `preflight --json` lacks a `status` field. A regression row for `dex_patch_bytes.py` reproduces a previous artifact byte-for-byte (`67ae1e58e7a44777e5ac8f9f823ab21e…`) and additionally pins the four bytes that may move. **Two real defects the new tests found in the tooling itself.** `check_repo.py` decoded subprocess output with the locale encoding, so on a GBK console a script printing an em dash in its usage line was reported as crashed; it now decodes UTF-8 explicitly. And a CLI contract test drove a *real attached device* through `coldstart.py` -- logcat, screenshots, five orphan `adb logcat` processes -- which is fixed twice over: child processes run in a private temporary cwd, and the fifteen device-bound scripts are never executed beyond `--help`/argument parsing. **Quoting on the device side is now one implementation.** `devsh.su` escaped only double quotes, so `;`, `$( )` and backticks in a caller-supplied value still reached the device shell while `coldstart.Device.shell` quoted correctly. Both use `scripts/device_shell.py` now: POSIX single-quote escaping, argv arrays by default, and identifier validation that refuses a malformed package, path or component with exit 2 instead of a traceback. The integration test that used to pin the *defect* (it asserted `su -c "…"` with `\"` escapes) now asserts the property, across five metacharacter payloads. Gates: check_repo 0 problems (including the content-reading identity scan over git ls-files), check_refs 0 dangling / 0 warning, check_routing coherent (53 symptom / 45 reference / 54 script rows), check_budget 0 failure / 0 warning with the always-loaded body now measured in tokens rather than in "narrative lines with the index excluded", check_commands 146 checked / 0 drift, pytest 608 passed. Not done, and stated rather than implied: CI has never run on a runner; the evals have never been executed; the 25 xfails are open; no device-side install/launch was performed in this pass. --- .github/workflows/ci.yml | 86 ++ README.md | 40 +- check_budget.py | 197 ++- check_commands.py | 1020 ++++++++++++++ check_repo.py | 14 +- check_routing.py | 176 +++ .../EXTENSION-reconstruction.md | 521 +++++++ .../EXTENSION-test-harness.md | 361 +++++ docs/tool-verification/README.md | 2 + pytest.ini | 27 + skills/apk-reverse/SKILL.md | 413 ++---- skills/apk-reverse/evals/evals.json | 56 + .../evidence/capability-matrix.json | 420 ++++++ .../apk-reverse/evidence/known-limitations.md | 131 ++ .../evidence/tested-tool-versions.json | 103 ++ skills/apk-reverse/references/ad-removal.md | 3 + .../references/advanced-unpacking.md | 18 +- .../references/byte-level-patching.md | 10 +- .../references/coverage-and-limits.md | 30 +- .../references/coverage/route-inventory.md | 70 + skills/apk-reverse/references/dart-aot.md | 12 +- .../desensitization-and-leak-scans.md | 9 +- .../references/detection-and-anti-analysis.md | 2 +- skills/apk-reverse/references/dex-patching.md | 3 + .../apk-reverse/references/dynamic-frida.md | 3 + .../references/emulation-and-rpc.md | 9 +- skills/apk-reverse/references/environment.md | 3 + .../references/evidence-summary.md | 102 ++ .../references/handoff-boundaries.md | 4 +- .../references/java2c-and-jni-sinking.md | 2 +- .../kernel-and-environment-hardening.md | 12 +- .../native-dbi-and-deobfuscation.md | 20 +- .../references/on-device-tooling.md | 7 +- skills/apk-reverse/references/patch-audit.md | 3 + skills/apk-reverse/references/pitfalls.md | 3 + .../references/precedents/README.md | 2 +- .../precedents/flutter-plus-shell-case-1.md | 6 +- .../frida-spawn-hangs-zero-events-case-2.md | 8 +- .../l1-equal-length-patch-case-4.md | 2 +- .../logd-broken-module-never-ran-case-3.md | 4 +- ...l-grep-finds-what-nobody-grepped-case-5.md | 16 +- .../references/protocol-reverse.md | 12 +- skills/apk-reverse/references/recon.md | 5 +- .../apk-reverse/references/repack-and-sign.md | 3 + skills/apk-reverse/references/routing.md | 192 +++ skills/apk-reverse/references/runtime-data.md | 3 + .../references/server-config-and-updates.md | 7 +- .../references/signature-derived-keys.md | 9 +- skills/apk-reverse/references/split-apk.md | 21 +- .../references/third-party-builds.md | 3 + skills/apk-reverse/references/tls-and-cert.md | 3 + skills/apk-reverse/references/toolchain.md | 2 +- skills/apk-reverse/references/verification.md | 3 + .../references/vmp-differential-analysis.md | 14 +- skills/apk-reverse/scripts/capabilities.py | 1198 +++++++++++++++++ skills/apk-reverse/scripts/device_shell.py | 302 +++++ skills/apk-reverse/scripts/devsh.py | 54 +- skills/apk-reverse/scripts/doctor.py | 613 ++++++--- skills/apk-reverse/scripts/repack.py | 5 +- skills/apk-reverse/scripts/so_constpatch.py | 1062 ++++++++++++++- tests/README.md | 75 ++ tests/cli/contract.py | 181 +++ tests/cli/test_cli_contract.py | 262 ++++ tests/conftest.py | 174 +++ tests/fixtures/apk/.gitignore | 9 + tests/fixtures/apk/arsc_aligned.apk | Bin 0 -> 1830 bytes tests/fixtures/apk/noextract.apk | Bin 0 -> 1233 bytes tests/fixtures/apk/plain.apk | Bin 0 -> 1144 bytes tests/fixtures/apk/services.apk | Bin 0 -> 1307 bytes tests/fixtures/logcat_sample.txt | 11 + tests/fixtures/make_fixture_apk.py | 369 +++++ tests/fixtures/tombstone_sample.txt | 20 + .../test_adb_command_construction.py | 334 +++++ tests/integration/test_no_device_flows.py | 252 ++++ tests/run_tests.py | 41 + tests/test_so_constpatch.py | 686 ++++++++++ tests/unit/test_blob_decode.py | 186 +++ tests/unit/test_dex_patch_bytes_regression.py | 216 +++ tests/unit/test_dexutil.py | 393 ++++++ tests/unit/test_protobuf_decode_raw.py | 192 +++ tests/unit/test_scan_leaks.py | 198 +++ 81 files changed, 10332 insertions(+), 708 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 check_commands.py create mode 100644 check_routing.py create mode 100644 docs/tool-verification/EXTENSION-reconstruction.md create mode 100644 docs/tool-verification/EXTENSION-test-harness.md create mode 100644 pytest.ini create mode 100644 skills/apk-reverse/evals/evals.json create mode 100644 skills/apk-reverse/evidence/capability-matrix.json create mode 100644 skills/apk-reverse/evidence/known-limitations.md create mode 100644 skills/apk-reverse/evidence/tested-tool-versions.json create mode 100644 skills/apk-reverse/references/coverage/route-inventory.md create mode 100644 skills/apk-reverse/references/evidence-summary.md create mode 100644 skills/apk-reverse/references/routing.md create mode 100644 skills/apk-reverse/scripts/capabilities.py create mode 100644 skills/apk-reverse/scripts/device_shell.py create mode 100644 tests/README.md create mode 100644 tests/cli/contract.py create mode 100644 tests/cli/test_cli_contract.py create mode 100644 tests/conftest.py create mode 100644 tests/fixtures/apk/.gitignore create mode 100644 tests/fixtures/apk/arsc_aligned.apk create mode 100644 tests/fixtures/apk/noextract.apk create mode 100644 tests/fixtures/apk/plain.apk create mode 100644 tests/fixtures/apk/services.apk create mode 100644 tests/fixtures/logcat_sample.txt create mode 100644 tests/fixtures/make_fixture_apk.py create mode 100644 tests/fixtures/tombstone_sample.txt create mode 100644 tests/integration/test_adb_command_construction.py create mode 100644 tests/integration/test_no_device_flows.py create mode 100644 tests/run_tests.py create mode 100644 tests/test_so_constpatch.py create mode 100644 tests/unit/test_blob_decode.py create mode 100644 tests/unit/test_dex_patch_bytes_regression.py create mode 100644 tests/unit/test_dexutil.py create mode 100644 tests/unit/test_protobuf_decode_raw.py create mode 100644 tests/unit/test_scan_leaks.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..42eeb76 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,86 @@ +# CI for the apk-reverse skill repository. +# +# WHAT THIS RUNS +# * the repository's own structural gates: check_repo.py, check_refs.py, +# check_budget.py, check_routing.py, and check_commands.py when it is present +# * the pytest suite: tests/unit, tests/cli, tests/integration +# +# WHAT THIS DOES NOT RUN, ON PURPOSE +# * anything needing a device, adb, frida, a rooted phone, a network peer or a +# real sample APK. Those routes are recorded experiments in tests/benchmark.md +# and their status is documented in +# docs/tool-verification/EXTENSION-test-harness.md. +# * anything about agent behaviour. Whether a model routes, classifies or +# refuses correctly is not observable from a unit test; skills/apk-reverse/ +# evals/evals.json is the skeleton for that, and it has never been executed. +# * the dex_patch_bytes regression hash. Its input lives in git-ignored +# tools/_work/bench/, so that test self-skips here with a printed reason +# rather than failing. Run it locally after downloading the sample. +# +# Coverage is therefore: static structure, documentation anchors, budget, command +# references, pure parsers, CLI contracts and no-device flows. Nothing more is +# claimed. +name: ci + +on: + push: + pull_request: + +permissions: + contents: read + +jobs: + test: + name: python ${{ matrix.python-version }} + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: ["3.11", "3.13"] + + env: + PYTHONIOENCODING: utf-8 + PYTHONDONTWRITEBYTECODE: "1" + # The gates spawn sibling scripts and decode their output. On a Windows + # runner the default codec is the console's (GBK here), and one script's + # --help text is not GBK-decodable, which is reported as a crash in that + # script rather than as a harness decoding problem. Pinning UTF-8 removes + # the whole class of failure on every platform. + PYTHONUTF8: "1" + + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + + - name: Install test dependencies + run: python -m pip install --upgrade pip pytest + + - name: Structural gate - repository layout + run: python check_repo.py + + - name: Structural gate - documentation anchors + run: python check_refs.py + + - name: Structural gate - context budget + run: python check_budget.py + + - name: Structural gate - documented commands (if present) + run: | + if [ -f check_commands.py ]; then + python check_commands.py + else + echo "check_commands.py is not present in this revision; skipping." + fi + + - name: Structural gate - routing inventory matches the entry point + run: python check_routing.py + + - name: Regression suite + run: python -m pytest -q + + - name: Summarise skipped and expected-failure coverage + if: always() + run: python -m pytest -q -ra --no-header -p no:cacheprovider 2>&1 | tail -n 40 diff --git a/README.md b/README.md index e8d7d28..9a7ab22 100644 --- a/README.md +++ b/README.md @@ -206,6 +206,22 @@ references/ loaded on demand, one topic each the entry-point file as a prompt surface precedents/ the positive case library: route including dead ends, a grade per assertion, measured pit-falls, and the write-back checklist + routing.md the on-demand inventory: every reference with when to load it, every + script with what it does, and a mirror of the symptom index + evidence-summary.md the condensation that ships with the skill: capability, one-line + conclusion, strength, and the evidence you can actually open in an + installed copy + + ../evals/ NOT a spec directory either, but the location the Agent + Skills guidance recommends: `evals.json` holds the + with-skill / without-skill cases this skill has **not** run, + with the method for running them written into the file + ../evidence/ NOT a spec directory: the machine-readable companions to the evidence + summary reference above -- `capability-matrix.json` (the same rows with + more fields), `tested-tool-versions.json` (versions and the probe behind + each), `known-limitations.md` (the installer-facing limit list). Shipped + inside the skill so an installed copy can answer "was this verified, and + how strongly" without the repository pitfalls.md the failure catalogue -- read before building advanced-unpacking.md the dump landed but the bodies are empty: extraction-shell diagnosis by trivial-body ratio, FART-style active invocation and why its classic hooks @@ -364,6 +380,11 @@ scripts/ parameterized, path-agnostic self-destructing target still yields evidence ``` +The repository also carries an **executable** test layer, which is a different thing from the +evidence record: `tests/` asserts what the scripts do (unit, CLI contract, no-device +integration) and `tests/benchmark.md` records what a route did on a real target. `tests/README.md` +states the split, and `.github/workflows/ci.yml` runs the gates plus the suite. + ## Install This repository is a **skills repository**: the skill lives at `skills/apk-reverse/`, which is the @@ -488,14 +509,21 @@ Four tools live at the root and are not part of the installed skill: check_repo.py every skill discovered, frontmatter valid, scripts runnable, documented paths resolve, README paths explicit and existing, and -- on the tracked surface only -- no target identity - (delegates the rules to skills/apk-reverse/scripts/scan_leaks.py so - there is one place to argue with the exemption list) + (delegates the rules to skills/apk-reverse/scripts/scan_leaks.py + so there is one place to argue with the exemption list) check_refs.py every cross-reference that names a section of another document reaches a real heading in that document -check_budget.py keep the always-loaded part from creeping: narrative lines in - SKILL.md (index lines counted separately, because one line per - bundled file is the price of discoverability), index-row length, - and conclusions restated outside their legitimate homes +check_routing.py the on-demand inventory still matches the entry point: the + symptom mirror agrees with SKILL.md, every reference file is + named in skills/apk-reverse/references/routing.md, and every + script is too +check_commands.py every command a document tells you to run is checked against + the script's own argparse table -- a documented flag that does + not exist is a drift the anchor checks cannot see +check_budget.py keep the always-loaded part from creeping: SKILL.md's whole + body (index lines included, because they load too) measured in + lines and tokens, index-row length, long files with no + navigable head, and hedged rules reported as a trend build_scripts.py audit for machine-specific leftovers (absolute paths, credentials) ``` diff --git a/check_budget.py b/check_budget.py index f2ca940..be3b519 100644 --- a/check_budget.py +++ b/check_budget.py @@ -1,31 +1,44 @@ #!/usr/bin/env python3 -"""Budget gate for the apk-reverse skill: keep the always-loaded part from creeping. +"""Budget and shape gate for the apk-reverse skill: keep the always-loaded part from creeping, +and keep the long parts navigable. `check_repo.py` proves the repository is *consistent* (paths resolve, indexes are complete). -This proves it is not *bloating*. Those are different failures, and the second has no natural -counter-pressure: every pass adds a reference, an index row and a coverage claim, and nothing in -the repository notices. +This proves it is not *bloating*, and that a reader can find their way into a long file. Those are +different failures, and the second has no natural counter-pressure: every pass adds a reference, an +index row and a coverage claim, and nothing in the repository notices. What is measured, and why each measure is shaped the way it is: - 1. **Content lines vs index lines.** An index row is the price of discoverability: one line per - bundled file, and this skill ships 38 references and 48 scripts, so ~100 of its lines are - structural. Counting them against a prose budget would push the skill to drop files it needs, - so the budget applies to the *narrative* lines — rules, workflow, gates, classification — - which is the part that actually competes for attention. Anthropic's Agent Skills guidance - says "keep SKILL.md under 500 lines"; its own `skill-creator` ships 480 and `mcp-builder` 237. - Here the narrative budget is 460, and the total is reported for context, not as a verdict. - 2. **Index row length.** The index tables are the largest part of SKILL.md and the easiest to + 1. **The always-loaded part, in tokens rather than lines.** A skill's whole `SKILL.md` body loads + into context when the skill activates (agentskills.io/specification, "Progressive disclosure"), + so every line of it competes for attention -- **including the index tables**. An earlier version + of this gate subtracted the index lines from the budget, which made the number look better than + the context actually is: 150 index lines are still 150 lines the model reads. The budget is now + measured on the whole file, reported in lines *and* estimated tokens, with the index share + reported for context rather than credited against the budget. + Line counts are kept because they are the number a human can check by hand; the token estimate + is `characters / 4`, which is crude and stated as crude -- it is a trend indicator, not a + measurement of a specific tokenizer. + 2. **Index row length.** The index tables are the largest part of `SKILL.md` and the easiest to grow by accident: each new entry is written by someone who knows the detail, and the detail lands in the row. An index row says *when to load the file*; the file says what is in it. 3. **Restated conclusions** (reported as notes, never as failures). A conclusion legitimately - appears in the benchmark matrix, in the evidence record, and in the coverage statement — + appears in the benchmark matrix, in the evidence record, and in the coverage statement -- those three exist to state results. The check flags a concept that has spread *beyond* those - homes, because that is when a future correction has to hunt for every copy. It is a note - rather than a failure precisely because the three legitimate homes make a high count normal. + homes, because that is when a future correction has to hunt for every copy. 4. **Reference discoverability.** Every reference must be reachable from SKILL.md or nothing will ever load it. (`check_repo.py` also enforces this; the duplication is deliberate, because a dead reference is invisible in a way a broken path is not.) + 5. **Long-file navigability.** A file over `NAV_LINES` that a reader cannot skim is a file they + will not load. Each one must open with a navigable head -- a "what this answers" line and + either a table of contents or a "when to load" line -- so the decision to read further can be + made from the top. This is the check that keeps "file-level progressive disclosure" from + becoming a claim the repository makes but does not implement. + 6. **Instruction strength** (a note, never a failure). The repository's rules are meant to be + followed, not weighed: a rule phrased as "consider X" reads as optional and is skipped under + load. This reports the ratio of hedging phrases to imperative ones as a *trend*, and lists the + hedged lines it found, so a human can decide -- the gate cannot judge whether a given sentence + should be optional. Usage: python check_budget.py # report; exit 1 if a hard limit is exceeded @@ -44,11 +57,25 @@ SKILL_DIR = os.path.join(ROOT, 'skills', 'apk-reverse') SKILL = os.path.join(SKILL_DIR, 'SKILL.md') REFS = os.path.join(SKILL_DIR, 'references') -CONTENT_LINES_OK = 460 # narrative budget +CONTENT_LINES_OK = 460 # always-loaded lines: soft ceiling, warns above this CONTENT_LINES_HARD = 560 # past this, something is structurally wrong +# Token budget for the always-loaded body. Calibrated here rather than copied: the Agent Skills +# spec recommends roughly 5,000 tokens of *instructions*, and this skill deliberately exceeds it, +# because the two things it will not move out of the always-loaded body are the symptom index +# (~54 rows; "symptom -> file" is the one lookup that must stay one hop) and the four gates with +# their pass criteria. The budget therefore tracks whether the body is *growing past what the +# design requires*, and the number is reviewed against a real tokenizer (see token_estimate). +TOKEN_SOFT = 13000 +TOKEN_HARD = 16000 INDEX_ROW_SOFT = 250 # chars of description per row INDEX_ROW_HARD = 400 # a row longer than this is a paragraph, not an index entry INDEXES = ('Symptom index', 'Reference index', 'Script index') +NAV_LINES = 100 # above this, a reference must open navigably +# A navigable head: one of these must appear in the first NAV_HEAD_LINES lines. +NAV_HEAD_LINES = 40 +HEDGE_RE = re.compile( + r'\b(consider|considering|you may|you might|might want|optionally|prefer(?:ably)?|' + r'it is worth|feel free|if you like|tends to be better)\b', re.I) # Concepts that have been restated once already. Reported, not enforced -- see the docstring. RESTATED = { @@ -59,16 +86,21 @@ RESTATED = { 'ezAndroid is JNI sinking, not a VMP': [r'JNI sinking'], 'KernelSU userspace cannot change a syscall': [r'userspace module.{0,60}cannot change'], 'protobuf proto3 explicit zero': [r'proto3.{0,40}(?:explicit )?zero'], + 'armv7 AOT strings are UTF-8, not UTF-16': [r'armv7.{0,80}UTF-8', r'UTF-8.{0,80}armv7'], } # Documents whose job is to state results; a concept appearing here is not drift. LEGITIMATE_HOMES = ('tests/benchmark.md', 'docs/tool-verification/') -def skill_lines(): - with open(SKILL, encoding='utf-8') as fh: +def read_lines(path): + with open(path, encoding='utf-8') as fh: return fh.read().splitlines() +def skill_lines(): + return read_lines(SKILL) + + def index_rows(lines): """(index line count, [(section, first-cell, desc-length)] for over-long rows).""" n = 0 @@ -94,20 +126,50 @@ def index_rows(lines): return n, long_rows +def token_estimate(text): + """Token count for the always-loaded body. + + Uses `tiktoken` (cl100k_base) when it happens to be installed, because a real measurement beats + an estimate everywhere in this repository -- including in this file. Otherwise falls back to + `characters / 3.6`, which was *calibrated against that measurement* on this file (69,143 chars + -> 17,144 tokens) rather than guessed; markdown-heavy prose tokenizes worse per character than + the usual /4 rule of thumb. + """ + try: + import tiktoken + return len(tiktoken.get_encoding('cl100k_base').encode(text)), 'tiktoken/cl100k_base' + except Exception: + return int(len(text) / 3.6), 'estimate chars/3.6' + + def check_size(lines): n_idx, _ = index_rows(lines) - content = len(lines) - n_idx - if content <= CONTENT_LINES_OK: - lv, msg = 'ok', 'SKILL.md %d narrative lines + %d index lines = %d total (budget %d narrative)' % ( - content, n_idx, len(lines), CONTENT_LINES_OK) - elif content <= CONTENT_LINES_HARD: - lv, msg = 'warn', ('SKILL.md narrative is %d lines, over the %d budget by %d (%d index lines ' - 'excluded). Move detail into references/ and leave a pointer.' % - (content, CONTENT_LINES_OK, content - CONTENT_LINES_OK, n_idx)) + content = len(lines) + text = '\n'.join(lines) + toks, how = token_estimate(text) + pct_idx = (100.0 * n_idx / content) if content else 0.0 + + findings = [] + if content <= CONTENT_LINES_OK and toks <= TOKEN_SOFT: + msg = ('SKILL.md %d lines (%d index = %.0f%% of the file, and it is still loaded: the budget ' + 'is measured on the whole body), ~%d tokens (%s; soft %d)' + % (content, n_idx, pct_idx, toks, how, TOKEN_SOFT)) + findings.append(('ok', msg)) + elif content <= CONTENT_LINES_HARD and toks <= TOKEN_HARD: + findings.append(( + 'warn', + 'SKILL.md is %d lines / ~%d tokens (%s), over the soft budget (%d lines / %d tokens) -- ' + 'the always-loaded body is what the model reads on activation. Move detail into ' + 'references/ and leave a pointer; %d of those lines are the index tables, which are ' + 'loaded too. Total references: %d files.' + % (content, toks, how, CONTENT_LINES_OK, TOKEN_SOFT, n_idx, + len(glob.glob(os.path.join(REFS, '*.md')))))) else: - lv, msg = 'fail', 'SKILL.md narrative is %d lines, past the hard limit of %d' % ( - content, CONTENT_LINES_HARD) - return lv, msg, n_idx, content + findings.append(( + 'fail', + 'SKILL.md is %d lines / ~%d tokens (%s), past the hard limit (%d lines / %d tokens)' + % (content, toks, how, CONTENT_LINES_HARD, TOKEN_HARD))) + return findings, n_idx, content, toks def check_index_rows(lines): @@ -122,9 +184,57 @@ def check_index_rows(lines): return out +def check_navigable(): + """A long reference must be skimmable from its first screen. + + The head is what lets a reader decide *not* to read the rest, which is the only thing that makes + a 400-line reference cheaper than a 40-line one. Two shapes are accepted, because both are in + use here: an explicit table of contents, or an explicit "when to load / what this answers" line. + """ + out = [] + for path in sorted(glob.glob(os.path.join(REFS, '*.md'))): + lines = read_lines(path) + if len(lines) < NAV_LINES: + continue + head = '\n'.join(lines[:NAV_HEAD_LINES]).lower() + has_toc = bool(re.search(r'(?m)^#+ *(table of contents|contents)\s*$', head)) + has_when = ('when to load' in head or 'what this answers' in head + or 'load this when' in head or 'when this applies' in head) + if not (has_toc or has_when): + out.append(('warn', 'references/%s is %d lines with neither a table of contents nor a ' + '"when to load"/"what this answers" line in its first %d lines -- ' + 'a reader cannot decide from the top whether to load it' + % (os.path.basename(path), len(lines), NAV_HEAD_LINES))) + return out + + +def check_hedging(): + """Report hedged instruction lines; never fail on them. + + A rule that reads as optional gets skipped when the context is under load. This cannot be + decided by regex -- some sentences *should* be optional -- so it reports the lines and a ratio, + and a human decides. The ratio is the trend: if it climbs, the rules are getting softer. + """ + lines = skill_lines() + body = [ln for ln in lines + if ln.strip() and not ln.startswith('|') and not ln.startswith('#') + and not ln.startswith('---')] + hedged = [(i + 1, ln.strip()) for i, ln in enumerate(lines) if HEDGE_RE.search(ln)] + imperative = [ln for ln in body if re.match(r'^\s*[-*]? *(\*\*)?[A-Z]', ln)] + ratio = (len(hedged) / len(body)) if body else 0.0 + notes = [] + if hedged: + notes.append('hedged instruction lines: %d of %d prose lines (%.1f%%; report-only -- a ' + 'hedged rule reads as optional). First few: %s' + % (len(hedged), len(body), 100.0 * ratio, + '; '.join('L%d %s' % (n, t[:52]) for n, t in hedged[:3]))) + return notes + + def check_restated(): notes = [] files = (glob.glob(os.path.join(REFS, '*.md')) + + glob.glob(os.path.join(REFS, '*', '*.md')) + glob.glob(os.path.join(ROOT, 'docs', 'tool-verification', '*.md')) + [SKILL, os.path.join(ROOT, 'README.md'), os.path.join(ROOT, 'tests', 'benchmark.md')]) corpus = {} @@ -146,13 +256,23 @@ def check_restated(): def check_discoverable(): + """Every reference must be reachable from the entry point or from the routing file it points to. + + `SKILL.md` is the entry point; since this pass it carries the symptom index and delegates the + inventory to `references/routing.md` (which `check_routing.py` in turn proves names every + reference and every script). Counting the routing file here is what keeps the delegation honest + instead of letting a file become unreachable the moment its index row moves. + """ body = open(SKILL, encoding='utf-8').read() + routing = os.path.join(REFS, 'routing.md') + if os.path.isfile(routing): + body += open(routing, encoding='utf-8').read() out = [] for f in sorted(glob.glob(os.path.join(REFS, '*.md'))): name = os.path.basename(f) if name not in body: - out.append(('fail', 'references/%s is never mentioned in SKILL.md -- nothing will ' - 'load it' % name)) + out.append(('fail', 'references/%s is named neither in SKILL.md nor in ' + 'references/routing.md -- nothing will load it' % name)) return out @@ -164,15 +284,20 @@ def main(): args = ap.parse_args() lines = skill_lines() - lv, msg, n_idx, content = check_size(lines) - findings = [(lv, msg)] + findings, n_idx, content, toks = check_size(lines) findings += check_index_rows(lines) findings += check_discoverable() - notes = check_restated() + findings += check_navigable() + notes = check_restated() + check_hedging() if args.json: - print(json.dumps({'findings': [{'level': l, 'message': m} for l, m in findings], - 'notes': notes, 'index_lines': n_idx, 'content_lines': content}, indent=2)) + print(json.dumps({ + 'findings': [{'level': l, 'message': m} for l, m in findings], + 'notes': notes, + 'index_lines': n_idx, + 'total_lines': content, + 'token_estimate': toks, + }, indent=2)) else: for l, m in findings: print('%s %s' % ({'ok': ' ok ', 'warn': ' WARN ', 'fail': ' FAIL '}[l], m)) diff --git a/check_commands.py b/check_commands.py new file mode 100644 index 0000000..68b68ef --- /dev/null +++ b/check_commands.py @@ -0,0 +1,1020 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- +"""Validate every `python