Feature/schema synchronization and export - #41
Merged
Conversation
…n code and documentation
|
Warning Review limit reached
Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR renames collector metadata from ChangesSchema, parser, and downstream normalization updates
CLI, fixtures, and test updates
Docs, schema-sync guide, and tooling
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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 |
…arious components
…rove clarity Update parser tests to reflect changes in data structure and enhance error handling Add Pylint configuration for max line length in pyproject.toml files across multiple packages Refactor acceptance criteria and business value structures in PDF dataset tests for consistency Update normalize issues service tests to align with new data structure and improve clarity Add unit tests for schema export functionality to ensure correct schema versioning and path resolution
miroslavpojer
marked this pull request as ready for review
June 29, 2026 16:01
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/adapters/collector_gh/SCHEMA_SYNC.md (1)
188-188: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winFix broken relative link to
docs/contracts.md.From
packages/adapters/collector_gh/SCHEMA_SYNC.md, the repository root is three levels up (../../../), not two. The current link../../docs/contracts.mdresolves topackages/docs/contracts.md, which does not exist.-- **Input Contract Docs:** [../../docs/contracts.md](../../docs/contracts.md#input-contract-doc-issuesjson) +- **Input Contract Docs:** [../../../docs/contracts.md](../../../docs/contracts.md#input-contract-doc-issuesjson)🤖 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 `@packages/adapters/collector_gh/SCHEMA_SYNC.md` at line 188, The relative link to docs/contracts.md in SCHEMA_SYNC.md is resolving to the wrong location. Update the markdown link so it points from packages/adapters/collector_gh/SCHEMA_SYNC.md to the repository-root docs/contracts.md using the correct three-level relative path, and verify the reference still works from that document.docs/cookbooks/normalize-issues.md (1)
125-140: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUpdate JSON example to use
producerkey.The adapter mapping prose at Line 125 correctly states
metadata.producer.* → audit.producer.*, but the input JSON example at Lines 134-140 still shows"generator"instead of"producer". Since fixtures and the collector output were updated to useproduceracross the stack, this example will confuse users.{ "metadata": { - "generator": { + "producer": { "name": "AbsaOSS/living-doc-collector-gh", "version": "1.2.0", "build": "abc123" } } }🤖 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 `@docs/cookbooks/normalize-issues.md` around lines 125 - 140, The JSON example in the normalize-issues cookbook still uses the old metadata.generator key even though the mapping and collector now use metadata.producer; update the example under the audit mapping section to match the producer terminology so it stays consistent with the documented transform. Use the surrounding mapping text and the example block in normalize-issues.md as the location cues.
🧹 Nitpick comments (3)
packages/adapters/collector_gh/tests/test_parser.py (1)
248-248: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove these imports to module scope.
The new test methods import inside the function bodies, and the local
parseimport is redundant because this module already callsparseelsewhere. Import_build_body_from_structuredonce at the top of the file and drop the in-method imports.As per coding guidelines, "Place all Python imports at the top of the file (no imports inside methods/functions)".
♻️ Proposed fix
+from living_doc_adapter_collector_gh.parser import _build_body_from_structured ... - from living_doc_adapter_collector_gh.parser import _build_body_from_structured ... - from living_doc_adapter_collector_gh.parser import _build_body_from_structured ... - from living_doc_adapter_collector_gh.parser import _build_body_from_structured ... - from living_doc_adapter_collector_gh.parser import parseAlso applies to: 271-271, 279-279, 286-286
🤖 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 `@packages/adapters/collector_gh/tests/test_parser.py` at line 248, The new tests in test_parser.py are importing _build_body_from_structured and parse inside test methods, which violates the module-level import guideline and makes the local parse import redundant. Move the needed imports to the top of the file alongside the existing test imports, then remove the in-method imports from the affected test cases while keeping the test logic in the same functions.Source: Coding guidelines
packages/adapters/collector_gh/src/living_doc_adapter_collector_gh/schema_export.py (2)
52-60: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDeduplicate the schema version constant.
1.0.0is hard-coded here twice, whilecompatibility.pyalready exposesSCHEMA_VERSION. These copies will drift on the next schema bump, and then the exported filename, embedded$schema_version, and public API can disagree.Suggested refactor
import json from pathlib import Path +from living_doc_adapter_collector_gh.compatibility import SCHEMA_VERSION from living_doc_adapter_collector_gh.models import AdapterResult @@ - schema["$schema_version"] = "1.0.0" + schema["$schema_version"] = SCHEMA_VERSION @@ - return "1.0.0" + return SCHEMA_VERSIONAlso applies to: 71-81
🤖 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 `@packages/adapters/collector_gh/src/living_doc_adapter_collector_gh/schema_export.py` around lines 52 - 60, The schema version is duplicated in the schema export flow, which can cause the embedded $schema_version and generated filename to drift from the public API. Update schema_export.py to use the single source of truth from compatibility.py (SCHEMA_VERSION) in the schema_export logic, including the AdapterResult.model_json_schema() export and the doc-issues filename built from get_schema_version(), so both values stay consistent when the version changes.
64-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove CLI output to logging and keep imports at module top.
This new file uses
print()for status output and importssysinside the__main__block. As per coding guidelines,**/*.py: Use logging, notSuggested refactor
import json +import logging +import sys from pathlib import Path from living_doc_adapter_collector_gh.models import AdapterResult +logger = logging.getLogger(__name__) @@ - print(f"Schema exported to: {output_file}") + logger.info("Schema exported to: %s", output_file) @@ -if __name__ == "__main__": - import sys - +if __name__ == "__main__": @@ - print(f"Default location: {get_default_schema_path() / f'doc-issues-v{get_schema_version()}-schema.json'}") + logger.info( + "Default location: %s", + get_default_schema_path() / f"doc-issues-v{get_schema_version()}-schema.json", + )Also applies to: 84-93
🤖 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 `@packages/adapters/collector_gh/src/living_doc_adapter_collector_gh/schema_export.py` around lines 64 - 66, The CLI status output in schema_export.py still uses print() and sys is imported inside the __main__ block, both of which violate the Python guidelines. Update the module to use a logger for the “Schema exported”/usage messages instead of print, and move any sys import to the top-level imports alongside the rest of the module’s imports. Keep the changes centered around the schema export entrypoint and its __main__ handling so the behavior stays the same without inline imports or stdout logging.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.
Inline comments:
In `@docs/cookbooks/normalize-issues.md`:
- Around line 62-71: The example in normalize-issues.md still uses the old
metadata.generator reference in the auto-detection snippet; update the detection
logic in the code block to use metadata.producer.name so it matches the
documented field and the current implementation. Keep the surrounding example
and the CollectorGhAdapter check unchanged, and make sure any related wording in
the same section consistently refers to producer rather than generator.
In `@packages/adapters/collector_gh/SCHEMA_SYNC.md`:
- Line 43: The compatibility range documented in SCHEMA_SYNC.md is inconsistent
with the CONFIRMED_MIN constant in compatibility.py. Update the narrative near
the Producer Compatibility Range reference to match the actual minimum version
used by CONFIRMED_MIN, or change CONFIRMED_MIN in compatibility.py if the
intended floor is 1.0.0. Verify both the documentation and the constant reflect
the same supported range.
In
`@packages/adapters/collector_gh/src/living_doc_adapter_collector_gh/detector.py`:
- Around line 45-50: Reject non-string producer versions in extract_version so
it matches the str contract: in detector.py, the extract_version function should
validate that payload["metadata"]["producer"]["version"] is actually a string
before returning it, not just truthy. Keep the existing AdapterError handling,
but if the value is missing, empty, or not a string, raise an AdapterError from
extract_version rather than returning an int or other type.
In
`@packages/adapters/collector_gh/src/living_doc_adapter_collector_gh/models.py`:
- Around line 22-23: The module docs for the schema export example are
misleading because `schema_export` does not print the schema JSON to stdout;
update the example in `models.py` to match the actual behavior of
`schema_export.py` and `schema_export` so it shows the correct way to generate
the schema file without redirecting stdout. Make sure the documentation clearly
reflects that the command writes the artifact directly and only emits status
output, so readers can locate and run the export path correctly.
In
`@packages/adapters/collector_gh/src/living_doc_adapter_collector_gh/parser.py`:
- Around line 183-193: Validate raw_ac as a list of dict-like items before
building structured_ac in parser.py’s parse flow; if acceptance_criteria is
missing, keep it None, but if it is present and not a list (or contains non-dict
entries), raise a per-item validation error instead of iterating it. In the
AdapterItemAcceptanceCriterion mapping, require description rather than
defaulting it to an empty string so malformed criteria are surfaced by parse()’s
item-level error handling. Use the raw_ac/structured_ac conversion block and
AdapterItemAcceptanceCriterion construction as the main points to update.
In `@packages/datasets_pdf/schemas/pdf_ready_v1.schema.json`:
- Around line 97-101: The PDF-ready schema is out of sync with the updated
models, so regenerate the schema from the current PDF-ready model definitions
rather than keeping the old nullable markdown string shapes. Update the schema
fields that correspond to Sections.business_value, preconditions, and
acceptance_criteria to match the structured list/object types used by
pdf_ready_v1 models and the normalize_issues builder, and ensure the generated
schema reflects the new nested structure instead of legacy nullable strings.
---
Outside diff comments:
In `@docs/cookbooks/normalize-issues.md`:
- Around line 125-140: The JSON example in the normalize-issues cookbook still
uses the old metadata.generator key even though the mapping and collector now
use metadata.producer; update the example under the audit mapping section to
match the producer terminology so it stays consistent with the documented
transform. Use the surrounding mapping text and the example block in
normalize-issues.md as the location cues.
In `@packages/adapters/collector_gh/SCHEMA_SYNC.md`:
- Line 188: The relative link to docs/contracts.md in SCHEMA_SYNC.md is
resolving to the wrong location. Update the markdown link so it points from
packages/adapters/collector_gh/SCHEMA_SYNC.md to the repository-root
docs/contracts.md using the correct three-level relative path, and verify the
reference still works from that document.
---
Nitpick comments:
In
`@packages/adapters/collector_gh/src/living_doc_adapter_collector_gh/schema_export.py`:
- Around line 52-60: The schema version is duplicated in the schema export flow,
which can cause the embedded $schema_version and generated filename to drift
from the public API. Update schema_export.py to use the single source of truth
from compatibility.py (SCHEMA_VERSION) in the schema_export logic, including the
AdapterResult.model_json_schema() export and the doc-issues filename built from
get_schema_version(), so both values stay consistent when the version changes.
- Around line 64-66: The CLI status output in schema_export.py still uses
print() and sys is imported inside the __main__ block, both of which violate the
Python guidelines. Update the module to use a logger for the “Schema
exported”/usage messages instead of print, and move any sys import to the
top-level imports alongside the rest of the module’s imports. Keep the changes
centered around the schema export entrypoint and its __main__ handling so the
behavior stays the same without inline imports or stdout logging.
In `@packages/adapters/collector_gh/tests/test_parser.py`:
- Line 248: The new tests in test_parser.py are importing
_build_body_from_structured and parse inside test methods, which violates the
module-level import guideline and makes the local parse import redundant. Move
the needed imports to the top of the file alongside the existing test imports,
then remove the in-method imports from the affected test cases while keeping the
test logic in the same functions.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4c00090f-c19a-484a-a2cc-7b9c70422a66
📒 Files selected for processing (38)
.gitignoreMakefileapps/cli/pyproject.tomlapps/cli/src/living_doc_cli/commands/normalize_issues.pyapps/cli/tests/integration/test_cli_invocation.pyapps/cli/tests/test_cli.pydocs/architecture.mddocs/contracts.mddocs/cookbooks/normalize-issues.mddocs/troubleshooting.mdpackages/adapters/collector_gh/SCHEMA_SYNC.mdpackages/adapters/collector_gh/pyproject.tomlpackages/adapters/collector_gh/schemas/README.mdpackages/adapters/collector_gh/schemas/doc-issues-v1.0.0-schema.jsonpackages/adapters/collector_gh/src/living_doc_adapter_collector_gh/__init__.pypackages/adapters/collector_gh/src/living_doc_adapter_collector_gh/compatibility.pypackages/adapters/collector_gh/src/living_doc_adapter_collector_gh/detector.pypackages/adapters/collector_gh/src/living_doc_adapter_collector_gh/models.pypackages/adapters/collector_gh/src/living_doc_adapter_collector_gh/parser.pypackages/adapters/collector_gh/src/living_doc_adapter_collector_gh/schema_export.pypackages/adapters/collector_gh/tests/fixtures/collector_v1.0.0/input/doc-issues.jsonpackages/adapters/collector_gh/tests/fixtures/collector_v1.2.0/input/doc-issues.jsonpackages/adapters/collector_gh/tests/test_compatibility.pypackages/adapters/collector_gh/tests/test_detector.pypackages/adapters/collector_gh/tests/test_parser.pypackages/adapters/collector_gh/tests/test_schema_export.pypackages/core/pyproject.tomlpackages/datasets_pdf/pyproject.tomlpackages/datasets_pdf/schemas/pdf_ready_v1.schema.jsonpackages/datasets_pdf/src/living_doc_datasets_pdf/pdf_ready/v1/models.pypackages/datasets_pdf/tests/test_pdf_ready_models.pypackages/services/normalize_issues/pyproject.tomlpackages/services/normalize_issues/src/living_doc_service_normalize_issues/builder.pypackages/services/normalize_issues/tests/fixtures/golden/expected_output.jsonpackages/services/normalize_issues/tests/fixtures/golden/input.jsonpackages/services/normalize_issues/tests/integration/test_golden_files.pypackages/services/normalize_issues/tests/test_builder.pypackages/services/normalize_issues/tests/test_service.py
… criteria, business value, and preconditions; enhance readability and maintainability.
…y tests for version warnings
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
1 participant
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
Implements schema synchronization and export functionality for
doc-issues.json, adds acceptance criteria handling across the pipeline, and fixes producer metadata references.Release Notes:
doc-issues.json—collector_ghcan now export a versioned JSON Schema (doc-issues-v1.0.0-schema.json) and validates documents against it at runtime.SCHEMA_SYNC.mdguide documenting the schema versioning and synchronization workflow forcollector_gh.metadata.generator→metadata.produceracross code, tests, fixtures, and documentation to align with the canonical schema field name.acceptance_criteria) added to thedoc-issuesschema,collector_ghmodels/parser,datasets_pdfmodels, andnormalize_issuesbuilder — full end-to-end propagation."format": "date-time"constraint togenerated_atinpdf_ready_v1.schema.jsonfor strict ISO 8601 validation.Summary by CodeRabbit
itemswith producer metadata, including structured acceptance-criteria and section formatting..gitignore; tightened a PDF-ready schema datetime format.