Repository navigation
Conversation
Adds pass2.domain_range_policy (off|warn|strict, default warn; read but not surfaced in the shipped YAML). Inheritance-aware endpoint-type check: reversed edges are swapped under warn/strict; warn tags remaining violators with domain_range_violation=true; strict reports them as validation errors (triggering the existing retry) and _partial_recover drops them before the hallucinated-anchor pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reviewer's GuideThe PR adds an optional, inheritance-aware domain/range policy to pass2 extraction. Reversed edges are normalized, warn mode preserves and tags other violations for downstream metadata, and strict mode feeds violations into retry and recovery cleanup, with configuration validation and comprehensive unit-path coverage. Sequence diagram for strict domain/range extractionsequenceDiagram
participant Extractor
participant LLM
participant Validator
participant Recovery
Extractor->>LLM: llm_complete_with_retry
LLM-->>Extractor: extraction
Extractor->>Extractor: _normalize_scalars
Extractor->>Extractor: _apply_domain_range_policy
Extractor->>Validator: validate_extraction
Validator-->>Extractor: Domain/range violation
Extractor->>LLM: retry with validation errors
LLM-->>Extractor: extraction2
Extractor->>Extractor: _apply_domain_range_policy
Extractor->>Validator: validate_extraction
Validator-->>Extractor: errors2
Extractor->>Recovery: _partial_recover
Recovery-->>Extractor: cleaned extraction
Flow diagram for pass2 domain/range policy handlingflowchart TD
A["_extract_chunk"] --> B["_normalize_scalars"]
B --> C["_apply_domain_range_policy"]
C --> D{"Policy"}
D -->|off| E["Edges unchanged"]
D -->|warn| F["Swap reversed edges"]
F --> G["Tag remaining violations"]
D -->|strict| H["Swap reversed edges"]
H --> I["validate_extraction"]
I -->|violation| J["Retry extraction"]
J --> K["_partial_recover"]
K --> L["Drop violating edges and unanchored nodes"]
G --> M["edge_metadata.json"]
E --> N["Backfill extraction"]
L --> N
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/mykg/pass2.py" line_range="125" />
<code_context>
+ )
+ edge["from"], edge["to"] = edge["to"], edge["from"]
+ elif status == "violation" and policy == "warn":
+ edge["domain_range_violation"] = True
+ log.warning(
+ " %s — kept and tagged (policy=warn)",
</code_context>
<issue_to_address>
**issue (broader_impact):** The warning tag is copied only from the first occurrence of a deduplicated edge. When the same logical edge is extracted from multiple files and a later occurrence is tagged while the first occurrence is untagged, `deduplicate_edges` keeps the first copy and silently drops `domain_range_violation` from `edge_metadata.json`.
**Triggers:** When a logical edge is extracted in multiple files and its endpoint types are available only in a later occurrence, causing only that occurrence to be classified as a violation.
**Suggested fix:** When merging duplicate edges, explicitly OR `domain_range_violation` across all occurrences instead of relying on the first occurrence's copied fields.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and the new default behavior can swap reversed edges, tag violations, or drop them under strict mode, so an incorrect domain/range inference could write wrong relationships to the extracted graph. Reverting stops future changes but does not undo already stored edges; they are bounded and can be repaired by re-running extraction.
Blocking findings: src/mykg/pass2.py:125
| ) | ||
| edge["from"], edge["to"] = edge["to"], edge["from"] | ||
| elif status == "violation" and policy == "warn": | ||
| edge["domain_range_violation"] = True |
There was a problem hiding this comment.
issue (broader_impact): The warning tag is copied only from the first occurrence of a deduplicated edge. When the same logical edge is extracted from multiple files and a later occurrence is tagged while the first occurrence is untagged, deduplicate_edges keeps the first copy and silently drops domain_range_violation from edge_metadata.json.
Triggers: When a logical edge is extracted in multiple files and its endpoint types are available only in a later occurrence, causing only that occurrence to be classified as a violation.
Suggested fix: When merging duplicate edges, explicitly OR domain_range_violation across all occurrences instead of relying on the first occurrence's copied fields.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Owner
Author
|
Abandoned: domain/range policy feature dropped. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pass2.domain_range_policy(off|warn|strict, defaultwarn), exposed asconfig.PASS2_DOMAIN_RANGE_POLICY; invalid values log a warning and fall back towarn. Intentionally not added to eithermykg_config.yaml.pass2.py:_ancestors,_domain_range_status(inheritance-aware, uses response nodes +prior_nodes),_apply_domain_range_policyrun after_normalize_scalarson first attempt and retry.domain_range_violation: true. The tag reachesedge_metadata.jsonthrough the assembler unchanged.validate_extractionreportsDomain/range violation: ...(which triggers the existing retry), and_partial_recoverdrops violators before the hallucinated-anchor pass.Testing
pytest tests/test_pass2.py tests/test_extraction_edge_paths.py: 63 passed_test_files2with ollama-local, under warn and under strict: both runs finished and wroteknowledge_graph_validation.json. The LLM produced no reversed or violating edges (warn: 18 edges, strict: 24, 0 violators, 0 swaps). The scripted stub testtest_extract_chunk_policiescovers that path: off leaves edges unchanged, warn swaps and tags, strict retries and drops the violator.🤖 Generated with Claude Code
Summary by Sourcery
Add optional domain/range policy enforcement to pass2 edge extraction.
New Features:
Enhancements:
Tests: