Skip to content

Conversation

monsieurswag
Copy link
Contributor

@monsieurswag monsieurswag commented Oct 8, 2025

… requirements

Summary by CodeRabbit

  • Bug Fixes
    • Prevents creation of invalid requirement entries by blocking annotations on non-assessable requirements.
    • Adds early validation with a clear error message to stop invalid nodes before they propagate.
    • Improves data integrity and reduces downstream processing failures caused by malformed requirements.
    • Enhances user feedback by surfacing precise validation errors at construction time.

Copy link
Contributor

coderabbitai bot commented Oct 8, 2025

Walkthrough

Introduces a validation check in tools/convert_library_v2.py to raise a ValueError when constructing requirement nodes if a non-assessable node includes a non-empty annotation. The check occurs after node assembly and before URN registration. No other public interfaces or control flow are altered.

Changes

Cohort / File(s) Summary
Validation guard for requirement nodes
tools/convert_library_v2.py
Adds a post-assembly validation that raises ValueError if a requirement node is non-assessable but has a non-empty annotation; prevents adding such nodes’ URNs.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor Caller
    participant Converter as convert_library_v2.py
    Caller->>Converter: build_requirement_node(data)
    activate Converter
    Converter->>Converter: assemble node
    alt Non-assessable AND annotation non-empty
        Converter-->>Caller: raise ValueError("non-assessable cannot have annotation")
    else Valid node
        Converter->>Converter: add URN to set
        Converter-->>Caller: return node
    end
    deactivate Converter
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • ab-smith

Poem

A nibble of checks in the carrot patch code,
I thump for the rules on the validation road.
No notes on a node that can’t be assessed—
Else hop! a ValueError, swiftly expressed.
Ears up, tails high—our graph stays blessed. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and accurately summarizes the addition of a validation rule in the library checker to prevent annotations on non-assessable requirements, which is the core change introduced by this pull request.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch CA-1289-Add-a-rule-in-the-library-checker-annotation-assessable

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 and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 776140c and aa7d6ee.

📒 Files selected for processing (1)
  • tools/convert_library_v2.py (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: enterprise-startup-functional-test (3.12)
  • GitHub Check: startup-docker-compose-test
  • GitHub Check: startup-functional-test (3.12)
  • GitHub Check: enterprise-startup-docker-compose-test
  • GitHub Check: build_community_frontend
  • GitHub Check: build_enterprise_frontend

Comment on lines +995 to +996
if node.get("assessable", False) is False and node.get("annotation", ""):
raise ValueError(f"Requirement(with name={repr(node.get("name"))}) can't have a non-empty 'annotation' because it is not an assessable requirement.")
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix syntax error: nested quotes in f-string.

The f-string on line 996 contains unescaped double quotes inside node.get("name"), which will cause a syntax error.

Apply this diff to fix the syntax error:

-                    if node.get("assessable", False) is False and node.get("annotation", ""):
-                        raise ValueError(f"Requirement(with name={repr(node.get("name"))}) can't have a non-empty 'annotation' because it is not an assessable requirement.")
+                    if node.get("assessable", False) is False and node.get("annotation", ""):
+                        raise ValueError(f"Requirement(with name={repr(node.get('name'))}) can't have a non-empty 'annotation' because it is not an assessable requirement.")

Optional improvement: Consider enhancing the error message to include the URN for easier debugging:

raise ValueError(f"Requirement '{node.get('urn')}' (name={repr(node.get('name'))}) cannot have an annotation because it is not assessable.")
🤖 Prompt for AI Agents
In tools/convert_library_v2.py around lines 995 to 996 the f-string uses nested
double quotes causing a syntax error; replace the inner double quotes with
single quotes (or escape them) so the f-string is valid, and update the message
to be clearer by including the URN: raise a ValueError that references
node.get('urn') and node.get('name') using single quotes inside the f-string
(e.g. "Requirement '{node.get('urn')}' (name={repr(node.get('name'))}) cannot
have an annotation because it is not assessable.").

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