Skip to content

feat: register approved cheat-sheet suggestions into the graph (Workstream F, F5) - #1107

Open
skypank-coder wants to merge 2 commits into
OWASP:mainfrom
skypank-coder:feat/cheatsheets-workstream-f5
Open

skypank-coder wants to merge 2 commits into
OWASP:mainfrom
skypank-coder:feat/cheatsheets-workstream-f5

Conversation

@skypank-coder

Copy link
Copy Markdown
Contributor

Workstream F, checkpoint F5 — the final F slice

Wires the approved-suggestion conversion into the live import/register path.
Builds directly on F1–F3 (#1024) and F4 (#1033), which are already on main.

What this adds

convert gains an --import flag:

  • Default (no flag): review-only. Prints the summary (approved / standards /
    links / skipped unknown CREs) and writes nothing to the DB — matching the RFC's
    explicit-approval default.
  • With --import: registers the approved subset via the canonical
    cre_main.register_standard path — one call per ParseResult results group,
    mirroring base_parser.BaseParser.register_resource.

Design choices

  • Synchronous register_standard (not the RQ pipeline): this is the canonical
    path for a ParserInterface Standard resource, and a one-shot CLI import should
    complete its writes and return a real exit code rather than enqueue async jobs.
  • Fail-fast partial-failure policy: the first results group that raises aborts
    the remaining groups with a non-zero exit, matching base_parser.
  • Output via cre_logging: uses the repo's structured logger (logger.info
    for progress, logger.error for failures) — no print.

Scope

F5 only: the register wiring + --import flag. No changes to F1–F4 logic, the
suggestions schema, the web app, or #963.

Testing

application/tests/cheatsheets_workstream_f_test.py (F1–F5, 29 tests) — F5 mocks
cre_main.register_standard and stubs the cache, so no live Postgres/Neo4j/Redis/RQ
is required:

  • without --import, register_standard is never called (write-safe default);
  • with --import, one register call per group with the resolved Standard;
  • fail-fast: a two-group result whose first group raises makes exactly one call;
  • unknown CRE ids are still reported alongside the import summary.

Verified: full F1–F5 suite green, black 24.4.2 --check clean, mypy no new errors.

…tream F, F5)

convert --import wires the ParseResult from suggestions_to_parse_result into the
canonical cre_main.register_standard path (one call per results group, mirroring
base_parser.register_resource), fail-fast on a group failure. Without --import,
convert stays review-only (summary, no DB writes) - the RFC's explicit-approval
default. Output uses the repo's cre_logging convention; unknown CREs are still
reported. No changes to F1-F4 logic or the schema.
@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Advanced

Run ID: 801c6f7d-82d8-4b74-aa71-c4962dedcf77

📥 Commits

Reviewing files that changed from the base of the PR and between 02c8d3b and 3da42dc.

📒 Files selected for processing (2)
  • application/tests/cheatsheets_workstream_f_test.py
  • application/utils/external_project_parsers/parsers/cheatsheets_workstream_f.py

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


Summary by CodeRabbit

  • New Features

    • Added an optional import mode for convert using --import, -y, or --yes.
    • Approved results can now be registered one group at a time.
    • Import processing stops and reports an error if registration fails.
  • Bug Fixes

    • Clarified that the default convert behavior is review-only and does not register results.
    • Improved command-line help text for import behavior.

Walkthrough

Changes

Workstream F import flow

Layer / File(s) Summary
Convert registration path
application/utils/external_project_parsers/parsers/cheatsheets_workstream_f.py
convert remains review-only by default. The --import, -y, and --yes options register non-empty result groups through cre_main.register_standard and stop on the first registration error.
Import behavior validation
application/tests/cheatsheets_workstream_f_test.py
Tests cover grouped registration, review-only execution, registration failures, and fail-fast behavior. The test documentation now includes F5.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Merge Risk: ⚪ Minimal · up to 3da42

The optional import path preserves review-only behavior by default and passes approved groups to the canonical registration path correctly.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 27.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: registering approved cheat-sheet suggestions into the graph through Workstream F checkpoint F5.
Description check ✅ Passed The description directly explains the changeset, including the --import flag, review-only default, synchronous registration, fail-fast behavior, logging, scope, and tests.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

1 participant