Anki Factory: add GitHub ops gates - #10
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10c1d5f0f5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| rejected_config_hashes: dict[str, str] = {} | ||
| for key, spec in specs.items(): | ||
| for rejected_id in spec.get("rejected_duplicate_source_notetype_ids", []): | ||
| rejected_config_hashes[str(rejected_id)] = key |
There was a problem hiding this comment.
The contract-driven rejected_duplicate_source_notetype_ids map is constructed but never enforced, and the gate instead relies on a single hard-coded config_sha256 check. That means decks lacking config_sha256 metadata (for example legacy collection.anki2 profiles) can still match standard_cloze by name+fields and bypass the intended rejection of known bad duplicate note types, so the APKG gate may accept a format that the contract explicitly bans.
Useful? React with 👍 / 👎.
| if "collection.anki21b" in names and shutil.which("zstd"): | ||
| archive.extract("collection.anki21b", workdir) |
There was a problem hiding this comment.
collection.anki21b is only processed when an external zstd binary is present; otherwise the function falls through and reports the APKG as unsupported. In a clean environment without zstd, APKGs that contain only collection.anki21b (a format this tool is meant to support) cannot be profiled, which breaks downstream apkg_profiler/quality_gate usage unless users discover and install an undeclared system dependency.
Useful? React with 👍 / 👎.
| parser.add_argument("--contract", required=True, help="standardized-anki-contract.json") | ||
| parser.add_argument("--cards", help="Candidate card JSON to validate") | ||
| parser.add_argument("--apkg", help="APKG to validate") | ||
| parser.add_argument("--strict-template-hash", action="store_true", help="Require canonical anki21b template/config hashes when available") |
There was a problem hiding this comment.
The APKG validator only enforces note-type config fingerprint equality when --strict-template-hash is explicitly passed, but the default path accepts name+field matches even if template/config hashes drift. As a result, a deck with modified templates/CSS can pass quality_gate.py --apkg under default invocation, despite the contract’s non-negotiable rule that templates must not be altered.
Useful? React with 👍 / 👎.
| <article class="panel"> | ||
| <h2>Fixture Gate</h2> | ||
| <div class="metric">{status_label(bool(fixture_gate.get('ok')))}</div> | ||
| <p>Good fixture warnings: {html.escape(str(len(as_list(good_fixture.get('warnings')))))}</p> |
There was a problem hiding this comment.
The dashboard treats fixture_gate.good.warnings as a list, but validate_public_fixtures.py emits it as a numeric count. Because as_list() converts non-lists to [], the UI will always display Good fixture warnings: 0 even when the smoke report records warnings, which makes the published quality summary inaccurate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2867ff6d39
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not isinstance(cards, list): | ||
| raise SystemExit("--cards must be a JSON array") |
There was a problem hiding this comment.
run_anki_factory.py runs quality_gate.py and then human_review_pack.py on the same --cards file, but quality_gate.load_cards() accepts wrapped payloads like { "cards": [...] } while human_review_pack.py hard-fails unless the root JSON is a list. This creates a real pipeline break where a card file can pass the quality gate and then immediately fail the review-pack step (--cards must be a JSON array), so the one-command runtime flow rejects an input format it already claims to support.
Useful? React with 👍 / 👎.
| require("$GITHUB_STEP_SUMMARY" in workflow, "CI workflow must publish the smoke report summary") | ||
| require("anki-factory-ci-artifacts/smoke-report.json" in workflow, "CI workflow must persist the smoke report JSON") | ||
| for glob in EXPECTED_PATH_GLOBS: | ||
| require(workflow.count(glob) >= 2, f"CI path filter must include {glob} for pull_request and push") |
There was a problem hiding this comment.
validate_hook_and_ci() enforces CI path coverage with workflow.count(glob) >= 2, which only counts raw occurrences in the YAML text. Because it does not verify one occurrence under pull_request.paths and one under push.paths, a path can be removed from push and duplicated under pull_request while this check still passes, allowing main-branch pushes for that path to skip the smoke workflow without the integration gate detecting the regression.
Useful? React with 👍 / 👎.
| "available": bool(paths), | ||
| "files": [source_file(path, note) for path in paths], |
There was a problem hiding this comment.
source_group() sets available from bool(paths) instead of checking that listed files exist, so a typoed or deleted input path is still reported as available in source-bundle.json. This causes downstream planning to treat required lecture/jokbo inputs as present even when the files are missing on disk, which can let runs proceed with invalid source coverage and misleading intake reports.
Useful? React with 👍 / 👎.
| if quality_report and (quality_report.get("errors") or quality_report.get("warnings")): | ||
| status = "needs_rewrite" | ||
| manifest = { | ||
| "ok": status in {"preview_ready", "needs_card_candidates"}, |
There was a problem hiding this comment.
run_anki_factory.py marks a run as successful whenever status is needs_card_candidates, but that status is also used when source intake is incomplete (for example, lecture_only_preview without any --lecture). In that case the manifest still returns ok: true and suggests drafting cards next, which masks a blocked prerequisite and can allow automation to treat an invalid run as healthy.
Useful? React with 👍 / 👎.
1 participant
Summary
Validation
.github/skills/anki-factory-quality/scripts/run-smoke.shpython3 tools/anki-factory/scripts/validate_copilot_integration.py --repo ..github/workflows/anki-factory-ci.ymland.github/workflows/anki-factory-pages.ymlPrivacy Boundary
Gate Checklist
anki-factory-smoke-reportartifact.fixture_gate,copilot_integration_gate, andagent_eval_gateare validated by smoke.validate_copilot_integration.py.