Skip to content

Follow-up audit after 0324168d: order-sensitive CALLS_API and remaining integration gaps #1

Description

@yshishenya

Summary

Thank you for the quick follow-up fixes. I repeated the audit against 0324168d using the same repository snapshots and the same independent oracles as the previous pass.

The new revision does fix the two main TypeScript findings from the previous report:

  • project-scoped tsconfig.json aliases no longer collapse across independent projects;
  • the documented TypeScript dependency contract now covers TS/TSX, emitted .js/.jsx specifiers, type-only and side-effect imports, named/star/type re-exports, literal import(), literal require(), multiple/exact/suffix aliases, and conventional package self-imports.

The update is focused and has near-neutral performance on the two unchanged Python-heavy repository snapshots. I would now consider the skill suitable for an explicit advisory pilot, but I still would not make it a mandatory/default stage of speckit-bootstrap.

The most important remaining correctness issue is that CALLS_API is still semantically dependent on ProcessPoolExecutor completion order when two modules export the same interface/base name.

Audit scope

Item Value
Previous audited revision 443ad954
Current audited revision 0324168d
Source comparison 5 files, +472 / -84
Upstream tests 21/21 passing
Documented commands 48/48 exiting successfully across two real repositories
Independent TypeScript cases 28 cases × 3 revisions
Robustness/fault groups 23 total, including a separate completion-order probe
Determinism stress runs 42
Independent report checks 51 preparation assertions + 30 separate cross-checks
Snapshot date 2026-07-14

All destructive/fault-injection work was run in disposable clones and synthetic fixtures. The original repositories were checked before and after; HEAD and pre-existing dirty state were unchanged.

Was → became

Metric Previous 443ad954 Current 0324168d Interpretation
Upstream unit tests 16/16 21/21 Improved
Documented TypeScript contract 6/17 17/17 Fixed
Extended real-world TypeScript cases 0/8 1/8 Improved, still limited
Negative controls 3/3 3/3 Retained
Overall independent TS matrix 9/28 21/28 Large improvement
Limitations in the original 22-group matrix 15/22 13/22 Two prior groups closed
Combined matrix after adding completion-order probe not measured 14/23 limitations reproduced One previous determinism conclusion reopened/narrowed
Weighted production-import recall on the unchanged Python snapshots 97.0% 97.0% Unchanged; the update is TypeScript-focused
Weighted test-import recall 26.5% 26.5% Unchanged
Product-scenario boundary recall 66.7% 66.7% Unchanged
Hand-labelled layer accuracy 70.6% 70.6% Unchanged

Controlled previous → current performance

Seven full builds and fifteen incremental builds were measured for each revision/repository after one warm-up, with counterbalanced order.

Snapshot Mode Previous median Current median Current / previous
Repository A full 1,136.35 ms 1,139.53 ms 1.003×
Repository A incremental 851.93 ms 867.22 ms 1.018×
Repository B full 2,361.22 ms 2,330.55 ms 0.987×
Repository B incremental 1,910.57 ms 1,942.30 ms 1.017×

This is effectively performance-neutral for the tested Python-heavy snapshots. Both repositories contained zero tracked .ts/.tsx files, so the new TypeScript correctness does not create a measurable navigation gain there yet.

Confirmed fixes

The following current-revision cases passed independently of the upstream tests:

  • static extensionless TS import;
  • .js specifier resolving to .ts;
  • .jsx specifier resolving to .tsx;
  • import type;
  • side-effect import;
  • named, star, and type re-export;
  • literal dynamic import();
  • literal require();
  • TSX parity;
  • two independent projects using the same alias without cross-project leakage;
  • exact alias, multiple targets, and suffix alias;
  • conventional workspace self-import.

The previous fixes for nested/conditional Python imports, content-hash invalidation, add/delete/rename, corrupt-JSON cache recovery, symlink escape prevention, explicit unsupported-language reporting, and timestamp-free output also remained intact.

Finding 1 — CALLS_API depends on completion order (high)

Minimal fixture

// provider_a.ts
export interface Contract { run(): void; }

// provider_b.ts
export interface Contract { run(): void; }

// consumer.ts
import { Contract } from "./provider_a";
export class Impl implements Contract { run() {} }

The same parsed FileResult objects were passed to build_graph() in the two orders that the process pool may produce:

Result order consumer IMPORTS consumer CALLS_API
provider_a, provider_b, consumer provider_a provider_b
provider_b, provider_a, consumer provider_a provider_a

The serialized maps also had different SHA-256 values.

Cause

  • parse results are appended in as_completed() order;
  • id_to_result preserves that order;
  • abstract_providers[ab] = nid stores one global provider per bare symbol name and overwrites earlier providers;
  • CALLS_API resolves Contract through that global map instead of the already-resolved import.

A 42-run repeated-build stress sample produced one hash per group, which is a useful operational signal, but it does not disprove the constructed order case. One of seven additional full builds also produced a different map hash with the same node count, edge count, and byte size.

Suggested acceptance criteria

  • Sort parsed results by normalized relative path before building cache/graph structures.
  • Do not model abstract_providers as bare_name -> one node globally.
  • Prefer the provider resolved by the source module's explicit import/reference.
  • Preserve ambiguity explicitly or fail closed when multiple unqualified providers remain.
  • Add a regression test with duplicate Contract providers and both result permutations.
  • Assert both byte stability and semantic edge stability.

Finding 2 — module-ID collisions silently discard files (high)

make_node_id() removes .py/.ts/.tsx and collapses /index.

Two synthetic pairs reproduced silent collisions:

  • dual.py + dual.ts → one dual node;
  • pkg.py + pkg/index.py → one pkg node.

Four supported files therefore became two nodes through id_to_result[node_id] = r.

Suggested acceptance criteria:

  • detect duplicate node IDs before insertion;
  • either produce collision-free IDs or emit a deterministic hard error listing every conflicting path;
  • add cross-language and file-vs-index regression tests.

Finding 3 — cache schema/provenance is not versioned (high)

The cache now validates the shape of individual entries and checks the complete source hash, which is good. Remaining cases:

  1. A syntactically valid JSON cache with the wrong top-level shape can still fail later because load_cache() accepts any JSON value.
  2. A semantically poisoned entry with the correct content_hash is trusted because there is no schema version, parser version, builder revision, or configuration fingerprint.

Suggested acceptance criteria:

  • top-level cache schema/version validation;
  • parser/dependency/builder/configuration fingerprint;
  • rebuild incompatible caches with a clear warning;
  • tests for valid-JSON wrong-shape caches and same-hash semantically stale entries.

Finding 4 — documented target root remains ambiguous (high integration impact)

The skill says to run scripts from the skill base directory, while the documented build command omits a target root:

python3 scripts/build_belief_map.py
python3 scripts/build_belief_map.py --full

Following those instructions literally builds a six-file map of the skill repository instead of the user's project. The builder supports a positional root internally, but the workflow does not show it and there is no --output option; .belief_map.sexp and .belief_map_cache.json are always written inside the scanned root.

Suggested acceptance criteria:

  • document an absolute skill-script path plus an explicit absolute target root;
  • add explicit --root/positional-root and --output examples;
  • fail clearly when the effective root appears to be the skill bundle itself unintentionally;
  • add an end-to-end docs test that invokes the exact README/SKILL command from a separate target repository.

Finding 5 — output/cache writes are not atomic or locked (medium)

Both the cache and map are written in place:

Injected interruption truncated prior-good files. Eight concurrent same-input builders completed in one sample, but the implementation still has no file lock or atomic commit, so that sample is not a safety guarantee.

Suggested acceptance criteria:

  • write to a same-filesystem temporary file;
  • flush/fsync as appropriate and os.replace() atomically;
  • serialize writers with a lock or reject concurrent writes;
  • retain the last known-good map/cache after interruption tests.

Finding 6 — extended TypeScript resolution remains 1/8 (medium)

The documented contract is now 17/17. The following extended real-world cases still fail:

  • TypeScript import = require(...);
  • template-literal dynamic import;
  • inherited tsconfig aliases via extends;
  • baseUrl-only resolution;
  • package entry through package.json#source;
  • package entry through package.json#exports;
  • self-package subpath through exports.

Only the type-query case passed. These do not all need to become supported, but the supported contract should be explicit and covered by tests so callers do not infer Node/TypeScript resolver parity.

Finding 7 — CLI/query error handling (medium)

Malformed user regexes reach unguarded re.compile(...), for example in search_modules(). Invalid numeric query arguments also produce tracebacks, and a catastrophic regex exceeded an external one-second deadline.

Suggested acceptance criteria:

  • catch re.error and return a short actionable diagnostic;
  • validate numeric/depth arguments before execution;
  • bound or structurally restrict user-supplied regex work;
  • add malformed and adversarial CLI cases.

Finding 8 — distribution/reproducibility gaps (medium)

At the audit snapshot the repository had no tag/release, dependency lock, or tracked LICENSE file. A clean system-Python run failed immediately with:

ModuleNotFoundError: No module named 'tree_sitter_typescript'

The 99.899% recall / 99.832% precision Upgraide claim in LOG.md could not be independently reproduced because the oracle, dataset, exact revision, and runnable harness were not committed. This is not evidence that the claim is wrong; it means it is currently an author-provided measurement rather than a reproducible repository result.

Suggested acceptance criteria:

  • pinned dependency manifest/lock and a friendly preflight;
  • immutable tag/release with checksums;
  • tracked license file;
  • committed benchmark oracle/dataset or a precise public reproduction recipe.

Lower-severity robustness cases

  • A valid source path containing spaces becomes difficult/unreachable through the emitted unquoted path trie.
  • A missing map fails clearly, but a structurally malformed map can be accepted as an empty graph.
  • Shell/Swift and other languages are explicitly unsupported. This is acceptable product scope, but it matters for a shell-centric bootstrap repository: scanning speckit-bootstrap currently returns zero nodes.

Integration decision for speckit-bootstrap

Recommended now

An explicit, advisory opt-in pilot is reasonable if it:

  • pins 0324168d (or a later immutable release and checksum);
  • runs after the primary Spec Kit bootstrap rather than becoming a required bootstrap stage;
  • supplies an absolute target root;
  • uses an isolated/pinned Python environment;
  • wraps writes with an external lock/atomic publication until native support exists;
  • keeps rg/native repository inspection as the fallback and source of truth;
  • never treats graph output as a CI correctness gate by itself.

This pilot passed 7/8 evaluated gates; semantic order stability is the remaining failed gate.

Not recommended yet

Mandatory/default bootstrap integration passed only 1/15 stricter gates (the current update's incremental-cost gate). The blockers are semantic order sensitivity, collision handling, cache/write guarantees, target-root UX, incomplete test-impact recall, unsupported shell surface, distribution immutability, and the lack of a large real TypeScript repository in this evaluation.

Proposed completion checklist

  • Completion-order regression is fixed semantically and byte-for-byte.
  • Duplicate node IDs cannot silently overwrite files.
  • Cache has schema and provenance versioning.
  • Map/cache publication is atomic and writer-safe.
  • Documentation names the target root unambiguously.
  • CLI reports malformed regex/numeric input without a traceback or unbounded hang.
  • Supported TypeScript resolution semantics are explicitly documented and tested.
  • Dependencies and release artifacts are pinned/reproducible.
  • Benchmark claims have a runnable evidence package or clearly state their external provenance.

I am happy to rerun the same independent matrix after the next fixes. The largest improvement in this revision is real: the advertised TypeScript contract moved from 6/17 to 17/17 without a material performance regression.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions