code/cl8y-research
2
0
Fork 0
Code Issues 10 Pull requests Projects Releases Packages Wiki Activity

Code quality: DRY, readability, and Rust/sqlx practice (gap analysis §10) #11

New issue
Open
opened 2026-08-31 05:33:57 +00:00 by PlasticDigits · 1 comment
PlasticDigits commented 2026-08-31 05:33:57 +00:00 (Migrated from gitlab.com)
Copy link

Summary

Improve DRY, readability, and Rust/library practice from gap analysis §10. Not a feature dump — keep behavior unless a listed item is already superseded by another issue (then hook that issue instead of conflicting).

Source: gaps/GAP_1788152435.md §10 (and §9 cost notes that are code-quality).

Current codebase

DRY

  • FixtureBundle constructors copied in collect.rs tests, pipeline.rs tests, tests/issue_plan.rs.
  • long_body / sample_body paragraph padders duplicated; fixtures/happy-week/replicate.json repeats one paragraph ~30 times for word count.
  • SQL schema in migrations/001_init.sql and store.rs migrate().
  • Skills duplicated under skills/ and .cursor/skills/.
  • Address lists: invariants::CANONICAL_ADDRESSES, CL8Y-web tokenDirectory.ts, DEX tokenlist.json.
  • search.rs is a 14-line wrapper.
  • write_publish_template vs unused DraftMr JSON.
  • Dead config: telegram_bot_token, gitlab_token, github_token, Config::default_repos(), secrets::redact, TELEGRAM_RETENTION_DAYS unused, SourceKind::onchain_authoritative unused by search.
  • tests/issue_plan.rs run_with uses mem::forget on TempDir.

Readability

  • pipeline.rs ~621 lines (stage machine + plan + IO + tests).
  • plan_week serde_json::from_str.unwrap_or_else first-line fallback hides bad model JSON.
  • --skip-image inverted (Replicate/image issue should replace with --image).
  • onchain_authoritative includes Repo (READMEs are not onchain).

Rust / library

  • Blocking reqwest + tokio only for Postgres; future API should be async.
  • sqlx string queries, no sqlx::migrate!.
  • Predictor: Send + Sync with &mut self create.
  • serde_yaml 0.9 unmaintained.
  • Error::Http(String) everywhere.
  • clap env feature unused; config is manual std::env::var.
  • Regex::new on every lint/sanitize/redact call.
  • Duplicate cargo test in CI / Makefile retest / AGENTS.md (intentional flake hunt — document or drop one).
  • MSRV 1.80 vs CI rust:1.86.

Why this is needed

Dead fields and duplicated fixtures slow every live-collector MR. pipeline.rs size makes editor/grounding work risky. Regex-per-call and duplicated SQL already caused drift.

Constraints / guardrails

  1. Do not change blog contract, allowlist semantics, or create-once rules “while cleaning.”
  2. If Postgres/Replicate/Telegram/publish issues already delete a dead field by using it, this issue skips that item (checklist in MR).
  3. Keep AGPL-3.0-only.
  4. No new secrets. No include of CL8Y-web as a crate.
  5. Cursor skills may remain copies of skills/ if that is the repo convention (CL8Y-web does this) — then add a one-line “keep in sync” note instead of inventing a generator, or make .cursor/skills symlinks if GitLab supports them.

Relevant files

src/pipeline.rs, src/store.rs, src/lib.rs, src/config.rs, src/invariants.rs, src/lint.rs, src/mdx.rs, src/telegram.rs, src/search.rs, src/publish.rs, tests/issue_plan.rs, migrations/001_init.sql, Cargo.toml, .gitlab-ci.yml, AGENTS.md, Makefile, skills/**, .cursor/skills/**

Recommended direction

  1. Test helpers module: tests/common/mod.rs for FixtureBundle, long_body, run_week temp dirs without mem::forget.
  2. Compact fixture draft: generate padding in the test predictor, don’t commit a 100KB repeated JSON if avoidable.
  3. include_str!("../migrations/001_init.sql") or sqlx migrate (coordinate with Postgres issue).
  4. Split pipeline into plan.rs / emit.rs / stage.rs.
  5. OnceLock<Regex> for lint/mdx/telegram.
  6. onchain_authoritative: only indexer kinds (and not Repo, not telegram, not web).
  7. plan_week: fail if plan JSON invalid (no silent first-line topic) or log a structured fallback flag in plan.json.
  8. Document MSRV vs CI; bump MSRV to 1.86 or pin CI to 1.80.
  9. Replace serde_yaml if a small change; otherwise note defer.
  10. Use dead tokens in live issues; this MR only removes them if still unused at the end.

Acceptance criteria

  • Shared test fixture helpers; mem::forget gone.
  • Schema defined in one place.
  • pipeline.rs split or reduced with a documented module map in docs/architecture.md.
  • Regex compiled once per pattern.
  • onchain_authoritative() false for Repo, Telegram, and web-search (when added).
  • Invalid plan JSON is explicit in tests (fail or flagged fallback).
  • MSRV/CI skew documented or resolved.
  • AGENTS.md duplicate cargo test explained or single invocation.
  • Dead code: each unused item either wired (other issues) or removed.

Test plan — functional paths

  1. Full cargo test --all-targets twice (or once if retest dropped) green.
  2. cargo clippy --all-targets -- -D warnings green.
  3. Fixture week still writes the artifact set.
  4. Diversity / injection / dry-run tests still pass.

Test plan — attack, hack, and abuse

Refactor-only, but regressions:

  1. Allowlist tests still reject SSRF and lookalike hrefs.
  2. Secret marker still fails emit.
  3. Stage skip still fail-closed.
  4. Create-once still refuses second create.
  5. Sanitize still strips javascript: / unofficial addresses.

Verification criteria

  1. Reviewer diffs are mechanical (move/extract) plus the explicit behavior fixes above.
  2. Architecture doc matches modules.
  3. Gap §10 items checked off or linked to the issue that consumed them.
## Summary Improve **DRY, readability, and Rust/library practice** from gap analysis §10. Not a feature dump — keep behavior unless a listed item is already superseded by another issue (then hook that issue instead of conflicting). Source: [`gaps/GAP_1788152435.md`](https://gitlab.com/PlasticDigits/cl8y-research/-/blob/main/gaps/GAP_1788152435.md) §10 (and §9 cost notes that are code-quality). --- ## Current codebase ### DRY - `FixtureBundle` constructors copied in `collect.rs` tests, `pipeline.rs` tests, `tests/issue_plan.rs`. - `long_body` / `sample_body` paragraph padders duplicated; `fixtures/happy-week/replicate.json` repeats one paragraph ~30 times for word count. - SQL schema in `migrations/001_init.sql` **and** `store.rs` `migrate()`. - Skills duplicated under `skills/` and `.cursor/skills/`. - Address lists: `invariants::CANONICAL_ADDRESSES`, CL8Y-web `tokenDirectory.ts`, DEX `tokenlist.json`. - `search.rs` is a 14-line wrapper. - `write_publish_template` vs unused `DraftMr` JSON. - Dead config: `telegram_bot_token`, `gitlab_token`, `github_token`, `Config::default_repos()`, `secrets::redact`, `TELEGRAM_RETENTION_DAYS` unused, `SourceKind::onchain_authoritative` unused by search. - `tests/issue_plan.rs` `run_with` uses `mem::forget` on `TempDir`. ### Readability - `pipeline.rs` ~621 lines (stage machine + plan + IO + tests). - `plan_week` `serde_json::from_str.unwrap_or_else` first-line fallback hides bad model JSON. - `--skip-image` inverted (Replicate/image issue should replace with `--image`). - `onchain_authoritative` includes `Repo` (READMEs are not onchain). ### Rust / library - Blocking `reqwest` + tokio only for Postgres; future API should be async. - sqlx string queries, no `sqlx::migrate!`. - `Predictor: Send + Sync` with `&mut self` create. - `serde_yaml` 0.9 unmaintained. - `Error::Http(String)` everywhere. - clap `env` feature unused; config is manual `std::env::var`. - `Regex::new` on every lint/sanitize/redact call. - Duplicate `cargo test` in CI / Makefile `retest` / `AGENTS.md` (intentional flake hunt — document or drop one). - MSRV `1.80` vs CI `rust:1.86`. --- ## Why this is needed Dead fields and duplicated fixtures slow every live-collector MR. `pipeline.rs` size makes editor/grounding work risky. Regex-per-call and duplicated SQL already caused drift. --- ## Constraints / guardrails 1. **Do not** change blog contract, allowlist semantics, or create-once rules “while cleaning.” 2. If Postgres/Replicate/Telegram/publish issues already delete a dead field by using it, this issue **skips** that item (checklist in MR). 3. Keep AGPL-3.0-only. 4. No new secrets. No `include` of CL8Y-web as a crate. 5. Cursor skills may remain copies of `skills/` if that is the repo convention (CL8Y-web does this) — then add a one-line “keep in sync” note instead of inventing a generator, **or** make `.cursor/skills` symlinks if GitLab supports them. --- ## Relevant files `src/pipeline.rs`, `src/store.rs`, `src/lib.rs`, `src/config.rs`, `src/invariants.rs`, `src/lint.rs`, `src/mdx.rs`, `src/telegram.rs`, `src/search.rs`, `src/publish.rs`, `tests/issue_plan.rs`, `migrations/001_init.sql`, `Cargo.toml`, `.gitlab-ci.yml`, `AGENTS.md`, `Makefile`, `skills/**`, `.cursor/skills/**` --- ## Recommended direction 1. Test helpers module: `tests/common/mod.rs` for `FixtureBundle`, `long_body`, `run_week` temp dirs **without** `mem::forget`. 2. Compact fixture draft: generate padding in the test predictor, don’t commit a 100KB repeated JSON if avoidable. 3. `include_str!("../migrations/001_init.sql")` or sqlx migrate (coordinate with Postgres issue). 4. Split `pipeline` into `plan.rs` / `emit.rs` / `stage.rs`. 5. `OnceLock<Regex>` for lint/mdx/telegram. 6. `onchain_authoritative`: only indexer kinds (and not Repo, not telegram, not web). 7. `plan_week`: fail if plan JSON invalid (no silent first-line topic) **or** log a structured fallback flag in `plan.json`. 8. Document MSRV vs CI; bump MSRV to 1.86 **or** pin CI to 1.80. 9. Replace `serde_yaml` if a small change; otherwise note defer. 10. Use dead tokens in live issues; this MR only removes them if still unused at the end. --- ## Acceptance criteria - [ ] Shared test fixture helpers; `mem::forget` gone. - [ ] Schema defined in one place. - [ ] `pipeline.rs` split or reduced with a documented module map in `docs/architecture.md`. - [ ] Regex compiled once per pattern. - [ ] `onchain_authoritative()` false for `Repo`, `Telegram`, and web-search (when added). - [ ] Invalid plan JSON is explicit in tests (fail or flagged fallback). - [ ] MSRV/CI skew documented or resolved. - [ ] `AGENTS.md` duplicate `cargo test` explained or single invocation. - [ ] Dead code: each unused item either wired (other issues) or removed. --- ## Test plan — functional paths 1. Full `cargo test --all-targets` twice (or once if retest dropped) green. 2. `cargo clippy --all-targets -- -D warnings` green. 3. Fixture week still writes the artifact set. 4. Diversity / injection / dry-run tests still pass. --- ## Test plan — attack, hack, and abuse Refactor-only, but regressions: 1. Allowlist tests still reject SSRF and lookalike hrefs. 2. Secret marker still fails emit. 3. Stage skip still fail-closed. 4. Create-once still refuses second create. 5. Sanitize still strips `javascript:` / unofficial addresses. --- ## Verification criteria 1. Reviewer diffs are mechanical (move/extract) plus the explicit behavior fixes above. 2. Architecture doc matches modules. 3. Gap §10 items checked off or linked to the issue that consumed them.
PlasticDigits commented 2026-08-31 05:35:10 +00:00 (Migrated from gitlab.com)
Copy link

marked as related to #7

marked as related to #7
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
No results found.
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
code/cl8y-research#11
No description provided.