Add standalone OWASP AI and Top 10 resource importers - #960
Add standalone OWASP AI and Top 10 resource importers#960Bornunique911 wants to merge 8 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Summary by CodeRabbit
WalkthroughAdds four OWASP parser modules for Top 10 2025, API Top 10 2023, LLM Top 10 2025, and AISVS. Adds JSON datasets, CLI import wiring, and unit tests for parsed sections and CRE links. ChangesNew OWASP parsers and import wiring
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (7)
application/utils/external_project_parsers/parsers/owasp_llm_top10_2025.py (1)
19-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd return type annotation for mypy compliance.
Same missing
-> ParseResultannotation as flagged inowasp_top10_2025.py. As per coding guidelines, "Runmake mypyfor Python type checking".🔧 Proposed fix
- def parse(self, cache: db.Node_collection, ph: prompt_client.PromptHandler): + def parse(self, cache: db.Node_collection, ph: prompt_client.PromptHandler) -> ParseResult:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/utils/external_project_parsers/parsers/owasp_llm_top10_2025.py` at line 19, The parse method in OWASP LLM Top 10 2025 is missing its return type annotation, which breaks mypy compliance. Update the parse signature in the parser class to explicitly declare the ParseResult return type, matching the pattern used in owasp_top10_2025.py and the surrounding parser interfaces.Source: Coding guidelines
application/utils/external_project_parsers/parsers/owasp_top10_2025.py (3)
19-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd return type annotation for mypy compliance.
parseis missing a-> ParseResultreturn annotation, unlike the baseParserInterface.parsesignature. As per coding guidelines, "Runmake mypyfor Python type checking".🔧 Proposed fix
- def parse(self, cache: db.Node_collection, ph: prompt_client.PromptHandler): + def parse(self, cache: db.Node_collection, ph: prompt_client.PromptHandler) -> ParseResult:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/utils/external_project_parsers/parsers/owasp_top10_2025.py` at line 19, The parse method in OWASPTop10 2025 parser is missing the return type annotation required to match ParserInterface.parse and satisfy mypy. Update the parse signature on the OWASPTop10 parser class to explicitly return ParseResult, keeping the existing parameters cache and ph unchanged, so it aligns with the base interface and type checking expectations.Source: Coding guidelines
31-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSilent skip hides unresolved
cre_idmappings.If a
cre_idin the bundled JSON doesn't resolve to an existing CRE (e.g., typo or CRE not yet imported), the link is silently dropped with no signal. A debug/warning log here would make data issues in the bundled JSON easier to catch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/utils/external_project_parsers/parsers/owasp_top10_2025.py` around lines 31 - 34, The CRE lookup loop in the OWASP Top 10 parser silently drops unresolved mappings, so update the logic in the parser that iterates over entry.get("cre_ids", []) to emit a debug or warning when cache.get_CREs(external_id=cre_id) returns nothing. Use the existing parser flow and relevant symbols like the OWASP Top 10 parsing function and cache.get_CREs to add a log message that includes the missing cre_id and enough context to spot bad bundled JSON data.
13-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate parsing logic across OWASP parsers.
This class's
parse()(Lines 19-47) is line-for-line identical in structure toOwaspLlmTop10_2025.parse()inowasp_llm_top10_2025.py, differing only inname/data_file. Per the PR stack, two more parsers (owasp_api_top10_2023,owasp_aisvs) likely follow the same shape. Consider extracting the shared "load JSON → buildStandard→ linkcre_ids" logic into a small helper (e.g., inbase_parser_defs.pyor a new mixin) parameterized byname/data_file, to avoid four parsers drifting independently as bugs are fixed.♻️ Example shared-helper approach
class JsonStandardParser(ParserInterface): data_file: Path def parse(self, cache: db.Node_collection, ph: prompt_client.PromptHandler) -> ParseResult: with self.data_file.open("r", encoding="utf-8") as handle: raw_entries = json.load(handle) entries = [] for entry in raw_entries: standard = defs.Standard( name=self.name, sectionID=entry["section_id"], section=entry["section"], hyperlink=entry["hyperlink"], ) for cre_id in entry.get("cre_ids", []): cres = cache.get_CREs(external_id=cre_id) if not cres: continue standard.add_link( defs.Link(ltype=defs.LinkTypes.LinkedTo, document=cres[0].shallow_copy()) ) entries.append(standard) return ParseResult( results={self.name: entries}, calculate_gap_analysis=False, calculate_embeddings=False, ) class OwaspTop10_2025(JsonStandardParser): name = "OWASP Top 10 2025" data_file = Path(__file__).resolve().parent.parent / "data" / "owasp_top10_2025.json"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/utils/external_project_parsers/parsers/owasp_top10_2025.py` around lines 13 - 47, The parse() implementation in OwaspTop10_2025 duplicates the same JSON-to-Standard mapping logic used by the other OWASP parser classes, so extract the shared “load JSON, build defs.Standard, attach cre_ids links” flow into a common helper or mixin (for example a base parser in base_parser_defs.py). Keep only the per-parser name and data_file differences in OwaspTop10_2025.parse() and have OwaspLlmTop10_2025, owasp_api_top10_2023, and owasp_aisvs reuse the shared implementation to prevent future drift.application/utils/external_project_parsers/parsers/owasp_api_top10_2023.py (2)
13-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate parser logic across OWASP modules.
This class is structurally identical to
OwaspAisvs(and presumably the other two new parsers): load JSON → buildStandard→ resolvecre_idsviacache.get_CREs→ link → return aParseResultwith gap analysis/embeddings disabled. Consider extracting a shared base class or helper function (e.g., takingname,data_fileas class attributes and a commonparse()implementation) to avoid maintaining four near-identical copies.♻️ Sketch of a shared base implementation
class OwaspJsonStandardParser(ParserInterface): data_file: Path def parse(self, cache: db.Node_collection, ph: prompt_client.PromptHandler) -> ParseResult: with self.data_file.open("r", encoding="utf-8") as handle: raw_entries = json.load(handle) entries = [] for entry in raw_entries: standard = defs.Standard( name=self.name, sectionID=entry["section_id"], section=entry["section"], hyperlink=entry["hyperlink"], ) for cre_id in entry.get("cre_ids", []): cres = cache.get_CREs(external_id=cre_id) if not cres: continue standard.add_link( defs.Link(ltype=defs.LinkTypes.LinkedTo, document=cres[0].shallow_copy()) ) entries.append(standard) return ParseResult( results={self.name: entries}, calculate_gap_analysis=False, calculate_embeddings=False, ) class OwaspApiTop10_2023(OwaspJsonStandardParser): name = "OWASP API Security Top 10 2023" data_file = Path(__file__).resolve().parent.parent / "data" / "owasp_api_top10_2023.json"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/utils/external_project_parsers/parsers/owasp_api_top10_2023.py` around lines 13 - 47, The OWASP parser classes are duplicating the same JSON-to-Standard parsing flow, so extract the shared implementation from OwaspApiTop10_2023 and its sibling parsers into a common base class or helper. Move the repeated logic for loading data_file, building defs.Standard objects, resolving cre_ids through cache.get_CREs, adding defs.Link entries, and returning ParseResult into a reusable parse() on a shared parser type, with each concrete class only supplying name and data_file.
19-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd return type annotation.
parsedoesn't declare-> ParseResult, unlike the interface it implements. As per coding guidelines,make mypyis run for type checking, so annotating consistently helps mypy catch mismatches.Diff
- def parse(self, cache: db.Node_collection, ph: prompt_client.PromptHandler): + def parse(self, cache: db.Node_collection, ph: prompt_client.PromptHandler) -> ParseResult:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/utils/external_project_parsers/parsers/owasp_api_top10_2023.py` at line 19, The parse method in OwaspApiTop10Parser is missing the explicit ParseResult return annotation required by the interface it implements. Update the parse signature in the parser class to include the same return type as the base contract, keeping the existing cache and ph parameters unchanged, so the method matches the expected typing used by mypy.Source: Coding guidelines
application/utils/external_project_parsers/parsers/owasp_aisvs.py (1)
17-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd return type annotation to
parse.
parsereturnsParseResultbut lacks a return type annotation, weakening mypy's ability to verify theParserInterfacecontract.🔧 Proposed fix
- def parse(self, cache: db.Node_collection, ph: prompt_client.PromptHandler): + def parse( + self, cache: db.Node_collection, ph: prompt_client.PromptHandler + ) -> ParseResult:As per coding guidelines, "Run
make mypyfor Python type checking" for**/*.py.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/utils/external_project_parsers/parsers/owasp_aisvs.py` around lines 17 - 45, The parse method in owasp_aisvs.py is missing a return type annotation even though it returns ParseResult. Update the parse signature on the Parser implementation to explicitly annotate the return type as ParseResult so mypy can verify the ParserInterface contract, keeping the existing cache and ph parameters unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@application/utils/external_project_parsers/parsers/owasp_aisvs.py`:
- Around line 17-45: The parse method in owasp_aisvs.py is missing a return type
annotation even though it returns ParseResult. Update the parse signature on the
Parser implementation to explicitly annotate the return type as ParseResult so
mypy can verify the ParserInterface contract, keeping the existing cache and ph
parameters unchanged.
In `@application/utils/external_project_parsers/parsers/owasp_api_top10_2023.py`:
- Around line 13-47: The OWASP parser classes are duplicating the same
JSON-to-Standard parsing flow, so extract the shared implementation from
OwaspApiTop10_2023 and its sibling parsers into a common base class or helper.
Move the repeated logic for loading data_file, building defs.Standard objects,
resolving cre_ids through cache.get_CREs, adding defs.Link entries, and
returning ParseResult into a reusable parse() on a shared parser type, with each
concrete class only supplying name and data_file.
- Line 19: The parse method in OwaspApiTop10Parser is missing the explicit
ParseResult return annotation required by the interface it implements. Update
the parse signature in the parser class to include the same return type as the
base contract, keeping the existing cache and ph parameters unchanged, so the
method matches the expected typing used by mypy.
In `@application/utils/external_project_parsers/parsers/owasp_llm_top10_2025.py`:
- Line 19: The parse method in OWASP LLM Top 10 2025 is missing its return type
annotation, which breaks mypy compliance. Update the parse signature in the
parser class to explicitly declare the ParseResult return type, matching the
pattern used in owasp_top10_2025.py and the surrounding parser interfaces.
In `@application/utils/external_project_parsers/parsers/owasp_top10_2025.py`:
- Line 19: The parse method in OWASPTop10 2025 parser is missing the return type
annotation required to match ParserInterface.parse and satisfy mypy. Update the
parse signature on the OWASPTop10 parser class to explicitly return ParseResult,
keeping the existing parameters cache and ph unchanged, so it aligns with the
base interface and type checking expectations.
- Around line 31-34: The CRE lookup loop in the OWASP Top 10 parser silently
drops unresolved mappings, so update the logic in the parser that iterates over
entry.get("cre_ids", []) to emit a debug or warning when
cache.get_CREs(external_id=cre_id) returns nothing. Use the existing parser flow
and relevant symbols like the OWASP Top 10 parsing function and cache.get_CREs
to add a log message that includes the missing cre_id and enough context to spot
bad bundled JSON data.
- Around line 13-47: The parse() implementation in OwaspTop10_2025 duplicates
the same JSON-to-Standard mapping logic used by the other OWASP parser classes,
so extract the shared “load JSON, build defs.Standard, attach cre_ids links”
flow into a common helper or mixin (for example a base parser in
base_parser_defs.py). Keep only the per-parser name and data_file differences in
OwaspTop10_2025.parse() and have OwaspLlmTop10_2025, owasp_api_top10_2023, and
owasp_aisvs reuse the shared implementation to prevent future drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 7bac8fd0-698c-4ed9-a6ce-11b1f6fb025b
📒 Files selected for processing (14)
application/cmd/cre_main.pyapplication/tests/owasp_aisvs_parser_test.pyapplication/tests/owasp_api_top10_2023_parser_test.pyapplication/tests/owasp_llm_top10_2025_parser_test.pyapplication/tests/owasp_top10_2025_parser_test.pyapplication/utils/external_project_parsers/data/owasp_aisvs_1_0.jsonapplication/utils/external_project_parsers/data/owasp_api_top10_2023.jsonapplication/utils/external_project_parsers/data/owasp_llm_top10_2025.jsonapplication/utils/external_project_parsers/data/owasp_top10_2025.jsonapplication/utils/external_project_parsers/parsers/owasp_aisvs.pyapplication/utils/external_project_parsers/parsers/owasp_api_top10_2023.pyapplication/utils/external_project_parsers/parsers/owasp_llm_top10_2025.pyapplication/utils/external_project_parsers/parsers/owasp_top10_2025.pycre.py
a4a595d to
4bc99ab
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
application/web/web_main.py (1)
1477-1483: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winUnbounded
per_pageallows oversized pagination.The prior
min(per_page, MAX_ITEMS_PER_PAGE)cap is gone, so a caller can request an arbitrarily largeper_page, forcing the DB/serializer to materialize the entire CRE set in one request (DoS / memory pressure). Reinstate an upper bound.🛡️ Proposed fix
- if ( - request.args.get("per_page") is not None - and int(request.args.get("per_page")) > 0 - ): - per_page = int(request.args.get("per_page")) + if ( + request.args.get("per_page") is not None + and int(request.args.get("per_page")) > 0 + ): + per_page = min(int(request.args.get("per_page")), MAX_ITEMS_PER_PAGE)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/web/web_main.py` around lines 1477 - 1483, The pagination handler in application/web/web_main.py no longer caps per_page, so requests can force oversized result sets. In the logic around request.args.get("per_page") and database.all_cres_with_pagination, reinstate the existing MAX_ITEMS_PER_PAGE upper bound by clamping the parsed per_page value before passing it onward. Keep the positive-value check, but ensure the final per_page used by the pagination call cannot exceed the maximum.
🧹 Nitpick comments (4)
application/web/web_main.py (2)
411-415: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated guard.
The
if not base_nodes or not compare_nodes: return Nonecheck is repeated verbatim. Drop the second occurrence.♻️ Proposed cleanup
if not base_nodes or not compare_nodes: return None - - if not base_nodes or not compare_nodes: - return None - compare_nodes_by_cre: dict[str, list[defs.Standard]] = {}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/web/web_main.py` around lines 411 - 415, The guard in web_main’s comparison logic is duplicated verbatim, so remove the repeated `if not base_nodes or not compare_nodes: return None` check and keep only one occurrence in the surrounding function to avoid redundant control flow.
293-297: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffPotential N+1 query loading all OpenCRE documents.
This issues one
get_CREs(internal_id=...)call per row returned bysession.query(db.CRE).all(). On production-sized graphs this becomes a large fan-out of queries permap_analysisrequest that hits the OpenCRE fast-path. Consider batch-loading CREs (single query) instead of per-id round trips.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/web/web_main.py` around lines 293 - 297, The _get_opencre_documents helper is doing an N+1 query pattern by calling collection.get_CREs(internal_id=cre.id) once per db.CRE row. Update this path to batch-load all needed CREs in a single query (or a small fixed number of queries) and then map them back to defs.CRE objects in memory, keeping the behavior of map_analysis/OpenCRE fast-path the same while removing the per-row round trips.application/tests/web_main_test.py (2)
1471-1471: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLeftover debug
This
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/tests/web_main_test.py` at line 1471, Remove the leftover debugging output in the test so the suite stays quiet; delete the print statement in the web_main_test flow that logs the response status and data, and keep the surrounding test logic unchanged.
1450-1451: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEnv mutation leaks across tests.
Setting
os.environ["CRE_ALLOW_IMPORT"] = "True"without restoring it (the priorpatch.dictcontext handled cleanup) leaves the flag enabled for subsequent tests, which can perturb import-gating andget_configassertions depending on test ordering. Prefer@patch.dict(os.environ, {"CRE_ALLOW_IMPORT": "True"})or restore intearDown.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/tests/web_main_test.py` around lines 1450 - 1451, The test in test_import_from_cre_csv mutates os.environ directly and leaves CRE_ALLOW_IMPORT enabled for later tests. Update this test to use temporary environment patching via patch.dict on os.environ (or restore the variable in tearDown) so the flag is automatically cleaned up after the test, keeping the import-gating behavior isolated for get_config and related assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@application/web/web_main.py`:
- Line 1522: `get_config` is interpreting `CRE_ALLOW_IMPORT` differently from
the import gating logic, so the config endpoint can disagree with the backend.
Update the `get_config` handling to use the same truthy membership check as the
import checks near the `CRE_ALLOW_IMPORT` gating logic and the related config
return path, so `"1"`, `"true"`, and `"yes"` are reported consistently. Keep the
behavior aligned with the existing import अनुमति/guard code rather than
comparing only against `"1"`.
- Around line 1352-1355: The prompt generation path in
prompt_client.PromptHandler currently calls generate_text without the LLM error
translation wrapper, so provider failures are not mapped correctly. Update the
web_main prompt handling flow to wrap
prompt.generate_text(message.get("prompt")) with the existing
llm_error_utils-style error translation used elsewhere, and ensure the
translated exception/response preserves provider 429 and similar LLM-specific
status codes instead of falling through as a generic 500.
- Around line 705-748: The Heroku guard in web_main’s gap analysis flow only
checks for missing standards, but it still falls through to
gap_analysis.schedule(...) on cache misses; update this branch to short-circuit
with a 404 when no cached/upstream analysis is available on Heroku. Use the
existing helpers _fetch_upstream_map_analysis and
_build_direct_cre_overlap_map_analysis in the same gap analysis path, and keep
the Heroku-specific logic in the current request handler before any job
scheduling.
---
Outside diff comments:
In `@application/web/web_main.py`:
- Around line 1477-1483: The pagination handler in application/web/web_main.py
no longer caps per_page, so requests can force oversized result sets. In the
logic around request.args.get("per_page") and database.all_cres_with_pagination,
reinstate the existing MAX_ITEMS_PER_PAGE upper bound by clamping the parsed
per_page value before passing it onward. Keep the positive-value check, but
ensure the final per_page used by the pagination call cannot exceed the maximum.
---
Nitpick comments:
In `@application/tests/web_main_test.py`:
- Line 1471: Remove the leftover debugging output in the test so the suite stays
quiet; delete the print statement in the web_main_test flow that logs the
response status and data, and keep the surrounding test logic unchanged.
- Around line 1450-1451: The test in test_import_from_cre_csv mutates os.environ
directly and leaves CRE_ALLOW_IMPORT enabled for later tests. Update this test
to use temporary environment patching via patch.dict on os.environ (or restore
the variable in tearDown) so the flag is automatically cleaned up after the
test, keeping the import-gating behavior isolated for get_config and related
assertions.
In `@application/web/web_main.py`:
- Around line 411-415: The guard in web_main’s comparison logic is duplicated
verbatim, so remove the repeated `if not base_nodes or not compare_nodes: return
None` check and keep only one occurrence in the surrounding function to avoid
redundant control flow.
- Around line 293-297: The _get_opencre_documents helper is doing an N+1 query
pattern by calling collection.get_CREs(internal_id=cre.id) once per db.CRE row.
Update this path to batch-load all needed CREs in a single query (or a small
fixed number of queries) and then map them back to defs.CRE objects in memory,
keeping the behavior of map_analysis/OpenCRE fast-path the same while removing
the per-row round trips.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 418134eb-596c-4224-a4d6-ddf0618a9861
📒 Files selected for processing (2)
application/tests/web_main_test.pyapplication/web/web_main.py
1e4de18 to
090da0b
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Please rebase onto latest |
|
Maintainer note: Rebased this branch onto latest |
090da0b to
d57f5ae
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
application/cmd/cre_main.py (2)
1233-1237: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRoute explicit-reference review outcomes without retrieval.
resolve()definesunknown_referenceandconflicting_referencesas review outcomes. The current branch sends both to semantic retrieval. This invokes embedding work and incrementssemanticfor sections that must not enter that path. Continue to retrieval only forResolutionOutcome.no_reference.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/cmd/cre_main.py` around lines 1233 - 1237, Update the resolution branching around resolve() so only ResolutionOutcome.no_reference continues into semantic retrieval. Handle unknown_reference and conflicting_references as review outcomes without invoking retrieval or incrementing semantic, while preserving the existing explicit handling for ResolutionOutcome.resolved.
1191-1192: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winBuild
known_idsfrom CRE records.
get_embeddings_by_doc_type()excludes every CRE without an embedding row. The explicit resolver then classifies an existing CRE ID as unknown. Buildknown_idsfrom persisted CRE external IDs. Keepcre_embeddingsonly for the semantic candidate pool.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@application/cmd/cre_main.py` around lines 1191 - 1192, Update the logic around get_embeddings_by_doc_type so known_ids is built from persisted CRE records’ external IDs rather than cre_embeddings.keys(), ensuring CREs without embedding rows remain known. Keep cre_embeddings exclusively as the semantic candidate pool for subsequent resolution.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@application/cmd/cre_main.py`:
- Around line 1233-1237: Update the resolution branching around resolve() so
only ResolutionOutcome.no_reference continues into semantic retrieval. Handle
unknown_reference and conflicting_references as review outcomes without invoking
retrieval or incrementing semantic, while preserving the existing explicit
handling for ResolutionOutcome.resolved.
- Around line 1191-1192: Update the logic around get_embeddings_by_doc_type so
known_ids is built from persisted CRE records’ external IDs rather than
cre_embeddings.keys(), ensuring CREs without embedding rows remain known. Keep
cre_embeddings exclusively as the semantic candidate pool for subsequent
resolution.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: bb31391a-a3aa-4f61-a23a-3fcd76bb63c6
📒 Files selected for processing (2)
application/cmd/cre_main.pycre.py
🚧 Files skipped from review as they are similar to previous changes (1)
- cre.py
|
@Bornunique911 thank you for the AI / Top 10 / API / LLM importer split and for rebasing — this is useful material. Same policy as on #953: we will not merge production JSON mapping importers under We'll reshape toward fixtures after the orchestrator lands and aim to merge the fixture-oriented stack together around end of August. Holding this PR until then; thanks again for the patience. |
d57f5ae to
61024d1
Compare
… the correct fixture locations
…update response handling
…status code function
2 participants
Supersedes #858. This replacement PR keeps only the AI/API/LLM/Top10 parser modules, mapping data, parser tests, and the minimal CLI wiring needed for standalone review from main.