Skip to content

Review: tests, CI gaps, docs drift, god-file maintainability #45

Description

@awksedgreep

Summary

Test/CI/docs/maintainability findings. No single item is critical, but together they explain the repeated source-only-tag history (v0.4.0 macOS link, v0.6.3 lockfile, v0.7.0 authctl --version, v0.7.1 fix-forward) and make the next planner bug harder to review.

Findings

  • Dirty tree is fmt-only but release-blocking: 4 files, 34+/12- (engine.rs:146,1292,4634, catalog_publication.rs:148, p2_delta_refresh.rs:18,58,103,272, shared.rs:590). TESTING.md:252,388 and docs/RELEASING.md:12 require a clean tree — evidence/tags taken now verify nothing. cargo fmt --all + commit.
  • God files: query_tvf.rs 5385 lines / 217 fns; engine.rs 5272 / 163 (single impl Engine from txn → 6 query families query_range:2435, query_latest:2724, query_aggregate:2975, query_window:3528+3791, query_rollup:3972+4085 + PromQL helpers :5063-5248). Triplicated vtab layer (metrics 1320 + logs 1440 + traces 1580 + shadow stores) enforced by convention, not types. Global-registry concurrency concentrated in shared.rs (1116 lines, WriterGate:218, SharedEngine:452, static mutexes) with only inline gate tests (:772-889).
  • Test gaps: no crates/timeless-ext/tests/ and no crates/timeless-codec/tests/ (only inline #[cfg(test)]); TVF planner (decode_args, best_index_args*, metric_candidates) has matcher unit tests but no malformed-argv/bitmask red-team; codec-8 CLP fallback has no dedicated test; tools/demogen (4 of last 8 commits) has no test dir; --include-ignored real-extension contracts are easy to skip (green-but-vacuous run); 5 allow(dead_code) suppressions hide dead-code signal.
  • CI hole: no auto-triggers (manual dispatch only, TESTING.md:424-429); query-contracts.yml runs only harness tests, not workspace/servers/fmt/clippy/shell suites; production-gate.yml misses root fmt/clippy/doc, oracle/sql-equivalents, cli/correctness/crash/dbhealth, libsql-check, embedded example. Split workspaces (root exclude=["servers"], default-members excludes dbhealth-ext, detached tools/*) mean bare cargo test --workspace covers ~3 of ~15 testable crates. No clippy.toml/rustfmt.toml/MSRV pin (toolchain drift already visible as fmt churn).
  • Docs drift: QUERY_RELEASE_REPORT.md:3-32 still says v0.5.0 is latest (stale since 0.7.7/aa258fa); ARTIFACTS.md:11-22 and COMPATIBILITY.md:143-146 stuck at v0.7.6; TESTING.md:52-64 Python inventory omits demogen/screencast.py + 7 bench/*.py so its own test -z "$(git ls-files '*.py')" gate can't pass; dist/ holds only 0.3.0 tarballs. HEAD is 8 past v0.7.7 with growing Unreleased (WAL profile) while AGENTS.md pre-tag checklist is unsatisfied. (WAL docs themselves — GUIDE.md:674-687 vs CHANGELOG.md:13 — are in sync, good.)

Suggested fix

  1. Commit/revert fmt churn; keep fmt --check green.
  2. Refresh ARTIFACTS/COMPATIBILITY to v0.7.7/unreleased; label QUERY_RELEASE_REPORT archived or refresh; fix TESTING.md Python inventory.
  3. Extend production-gate.yml with root fmt + clippy + harness contracts + oracle validate + one correctness.sh smoke (no auto-triggers needed).
  4. Extract promql.rs from engine.rs:5063-5248 and query_tvf::planning from :638-870 with malformed-input regression tests — gives timeless-ext its first real tests/ candidate.

Source

Full code review @ bacdf6f, tests/CI/docs track.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions