Skip to content

Add optional automated pipeline: collect / generate / eval / prune#8

Open
Abukhadeejah wants to merge 2 commits into
bennewell35:mainfrom
Abukhadeejah:skillmill
Open

Add optional automated pipeline: collect / generate / eval / prune#8
Abukhadeejah wants to merge 2 commits into
bennewell35:mainfrom
Abukhadeejah:skillmill

Conversation

@Abukhadeejah

Copy link
Copy Markdown

Proposal issue: #7

Summary

Optional, fully automated implementation of the six framework steps for people who already collect signals into Postgres+pgvector. The manual workflow in framework.md is unchanged — this is purely additive, as a bookmark_maxxing.pipeline subpackage with its own console script bookmark-maxxing-pipeline.

Framework step Command
Collect collect "<theme>" — ranked semantic matches from your own signal store (HTTP or Supabase RPC), read-only
Extract + Cluster strong-cluster gates inside generate: recurring (>=3 sources), platform diversity, distinct authors
Convert generate "<theme>" — LLM writes a card per templates/skill-card.md, primary sources only
Publish SKILL.md to skills/<domain>/<name>/ + upsert to skill_cards table
Compound eval (faithfulness judge + link liveness) + prune (active → probation → retired, archived never deleted, dry-run by default)

Anti-hallucination: a citation allowlist guard de-links any URL the model didn't receive in the signals (text kept, link dropped).

Zero new required dependencies

Everything network-facing (httpx) is imported lazily inside the functions that use it, so the core package and the dependency-free CI (unittest discover on bare 3.10/3.11/3.12) are untouched. Pipeline users opt in with pip install "bookmark-maxxing[pipeline]".

Files

  • src/bookmark_maxxing/pipeline/ — config, llm, collect, gates, generate, evaluate, prune, adapters, cli
  • schemas/skills-table.sqlskill_cards table + commented match_signals RPC contract
  • docs/automated-pipeline.md — setup + usage
  • tests/test_pipeline.py — pure-logic unittest coverage: gates, citation guard, frontmatter, prune decisions (no network/DB)
  • .env.example — new optional "Automated pipeline" section
  • pyproject.toml[pipeline] extra + second console script

Test plan

  • PYTHONPATH=src python3 -m unittest discover -s tests — 43 tests pass (existing + 18 new)
  • python3 -m compileall -q src tests clean
  • git diff --check clean
  • All pipeline modules import with stdlib only (no httpx/supabase installed)
  • Verified E2E against a live Supabase + OpenRouter setup: collect → generate (faithfulness-judged, citation-guarded) → eval → prune through the full active → probation → retired lifecycle

… cards out

Implements the six framework steps as four schedulable commands
(collect / generate / eval / prune) for people who already collect
signals into Postgres+pgvector. The manual workflow is unchanged.

- pgvector semantic clustering (HTTP endpoint or Supabase match_signals RPC)
- strong-cluster gates: recurring (>=3 sources), platform diversity,
  distinct authors (one person repeating themselves is not a trend)
- citation allowlist guard: URLs the model never received get de-linked
- faithfulness LLM-judge (0-100 groundedness) + link liveness checks
- conservative prune lifecycle: active -> probation -> retired,
  archived never deleted, dry-run by default

Zero new required dependencies: everything network-facing imports httpx
lazily, so the core package and CI stay dependency-free. Pipeline extras
install via pip install "bookmark-maxxing[pipeline]".
@Abukhadeejah
Abukhadeejah requested a review from bennewell35 as a code owner July 6, 2026 16:52
@bennewell35

Copy link
Copy Markdown
Owner

Arsalan, this is a strong direction and very aligned with where Bookmark Maxxing should go. I reviewed it locally and the core test suite passes:

  • PYTHONPATH=src python3 -m unittest discover -s tests -> 43 tests passed
  • python3 -m compileall -q src tests -> clean
  • git diff --check -> clean
  • bookmark-maxxing-pipeline collect "agent loops" with no signal source exits cleanly with the expected config error

Before we merge, can you patch two things?

  1. Sanitize generated skill names and domains before writing files.

Right now write_skill_file() trusts the LLM-generated name: frontmatter and joins it directly into the path:

cfg.output_dir / domain / skill_name / "SKILL.md"

A generated name like ../../escaped-skill can write outside PIPELINE_OUTPUT_DIR. Please add a small slug validator/sanitizer for both skill_name and domain, reject or normalize anything outside a safe pattern like [a-z0-9][a-z0-9-]{0,62}, and assert the resolved output path remains under cfg.output_dir.resolve() before writing or archiving.

Please add tests for:

  • ../../escaped-skill cannot write outside PIPELINE_OUTPUT_DIR
  • absolute paths are rejected
  • unsafe domains are rejected or normalized
  • a valid kebab-case skill name still writes normally
  1. Make the strong-cluster gates match the docs.

The docs and PR body say the pipeline uses strong gates: recurring sources, platform diversity, and distinct authors. In code, cmd_generate() only hard-stops on passes_recurring; diversity and distinct authors are passed to the LLM as soft-fails.

Please either:

  • enforce report.strong before generation, or
  • explicitly rename diversity/authors as advisory gates in the docs, PR copy, and CLI output.

My preference is to enforce report.strong for the automated pipeline. That keeps the automation opinionated and protects Bookmark Maxxing from turning one person's repeated posts or one vendor launch into a generated skill.

Suggested tests:

  • generate skips when recurring passes but distinct authors fail
  • generate skips when recurring passes but platform diversity fails
  • collect output makes it obvious which gate failed

Once those two are fixed, I think this becomes a very mergeable contribution. The architecture is promising: optional extras, lazy network imports, read-only collection, citation allowlist, eval history, and conservative pruning are all the right shape for this repo.

… gates

- sanitize_slug() normalizes LLM-generated name/domain to kebab-case and
  rejects path-like input (separators, '..', absolute paths); resolved
  output path is asserted to stay under PIPELINE_OUTPUT_DIR before any
  write or archive move
- all three gates (recurring, diversity, distinct-authors) are now hard:
  cmd_generate skips with the named failing gate(s) before any LLM call;
  collect output gains "strong" + "failed_gates" fields
- prune guards the archive move against unsafe DB-sourced names
- docs updated; 8 new tests (traversal/absolute/domain rejection, valid
  name writes, per-gate skip reporting)
@Abukhadeejah

Copy link
Copy Markdown
Author

Both addressed in 4c51d3e — thanks for the sharp review.

1. Path sanitization — added sanitize_slug() in adapters.py: rejects anything path-like outright (/, \, .., absolute paths), normalizes benign formatting (case, spaces) to kebab-case, and validates against [a-z0-9][a-z0-9-]{0,62}. Both write_skill_file() and archive_skill_file() now go through a shared _skill_dir() that also asserts the resolved path is_relative_to(cfg.output_dir.resolve()) before touching the filesystem. prune guards the archive move too, since DB-stored names are ultimately LLM-sourced. Tests cover: ../../escaped-skill rejected (and nothing written outside the tree), absolute paths rejected, unsafe domains rejected, valid kebab-case writes normally.

2. Gates now hard, per your preferencecmd_generate calls a new failed_gates(cfg, report) and skips before any LLM call, naming exactly which gate(s) failed (e.g. SKIP: gate(s) failed — distinct-authors (1/2 authors)). collect output gains "strong" and "failed_gates" fields so it's obvious pre-generation which gate blocks a theme. Docs and the (now unreachable) soft-fail prompt wording updated; the in-prompt SKIP instruction stays as defense-in-depth. Tests cover: recurring passes but distinct-authors fails → named skip; recurring passes but diversity fails → named skip; strong cluster → empty failure list.

CI re-verified locally: 51 tests pass under PYTHONPATH=src python3 -m unittest discover -s tests, compileall clean, git diff --check clean.

@bennewell35 bennewell35 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. Thanks for the quick, precise follow-up.

I re-reviewed commit 4c51d3e locally and verified the two blockers are addressed:

  • generated skill names/domains are sanitized before filesystem writes
  • path traversal and absolute paths are rejected
  • archive/prune paths use the same containment guard
  • strong-cluster gates are now enforced before generation
  • collect reports strong and failed_gates

Local verification on the updated branch:

  • PYTHONPATH=src python3 -m unittest discover -s tests -> 51 tests passed
  • python3 -m compileall -q src tests -> clean
  • git diff --check -> clean
  • manual path check rejected ../../escaped-skill and /tmp/escaped, while normalizing a valid name safely under PIPELINE_OUTPUT_DIR

This is a strong contribution and a meaningful step toward making Bookmark Maxxing a real signal-to-skill pipeline.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants