feat(models)!: accept multiple source URLs per section - #76
Conversation
`BaseSource.url` changes from a scalar `HttpUrl` to `list[HttpUrl]` (`min_length=1`), so one section can back its rows with several provenance URLs. All entries flow into the edge `source_record_urls` list via `retrieval_sources`; the pipeline already wrapped the single URL in a list, so it now iterates `self.source.url` directly. Docs (table/examples/tutorial/advanced), fixtures, agent prompts, and tests are updated to the list form. BREAKING CHANGE: the legacy scalar `url: https://...` is rejected; configs must use a list (`url:\n - https://...`).
|
Warning Review limit reached
Next review available in: 43 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthrough
ChangesSource URL List Migration
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@examples/agent/QC_REPORT.md`:
- Around line 33-34: Recompute the config sha256 hashes for every affected
report entry after the source.url scalar-to-list change, replacing the stale
parent-commit hash values while preserving the updated URL structure.
In `@src/tablassert/agent.py`:
- Around line 2009-2014: Update the prompt exemplars labeled (a) and (b) near
the shown source mappings to include a non-empty one-item source.url list in
each source mapping. Keep the existing source kind, local path, and other fields
unchanged so both examples remain schema-valid and demonstrate the required URL
field.
- Around line 2612-2614: Update the local payload contract represented by
table_list so every local path requires an original source URL, rather than
describing it as optional. Reject local payloads missing that URL before
constructing BaseSource, and ensure the agent never invents provenance.
In `@src/tablassert/models.py`:
- Around line 92-93: Update the field description near the provenance model to
state that source_record_urls is recorded on the primary entry within the edge
sources list and emitted in the RIG, rather than as a flat edge field. Keep the
existing format-validation and non-fetching semantics.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 795f0121-136a-40f1-b564-af39b5e6d632
📒 Files selected for processing (32)
CHANGELOG.mddocs/configuration/advanced-example.mddocs/configuration/table.mddocs/examples.mddocs/examples/tutorial-table.yamldocs/tutorial.mdexamples/agent/QC_REPORT.mdsrc/tablassert/agent.pysrc/tablassert/lib.pysrc/tablassert/models.pytests/agent_fixtures/GENE_DISEASE/reference_config.yamltests/agent_fixtures/PMC11708054/ALAMV6.yamltests/agent_fixtures/PMC11708054/reference_config.yamltests/fixtures/minimal_section.yamltests/fixtures/minimal_section_with_sections.yamltests/test_agent_branches.pytests/test_agent_build.pytests/test_agent_coverage.pytests/test_agent_derive.pytests/test_agent_eval.pytests/test_agent_multisection.pytests/test_agent_propose.pytests/test_agent_storage.pytests/test_agent_supervisor.pytests/test_cli_validation.pytests/test_cover_agent_core.pytests/test_cover_agent_eval.pytests/test_cover_agent_propose.pytests/test_e2e_smoke.pytests/test_lib.pytests/test_models.pytests/test_progress.py
- agent.py: add a one-item `source.url` list to the (a)/(b) few-shot exemplars so they are genuinely schema-valid, and reword the local-payload contract to state `source.url` is required (no fabrication) now that the field is a non-empty list. - models.py: clarify the `url` description — `source_record_urls` is emitted on the primary entry inside the edge `sources` list, not as a flat edge field. - QC_REPORT.md / QC_REVIEW.md: recompute the `config sha256` values over the committed (now list-form) configs and mirror them so the two artifacts stay consistent (CI guard compares them).
…-urls # Conflicts: # CHANGELOG.md
BaseSource.urlchanges from a scalarHttpUrltolist[HttpUrl](min_length=1), so one section can back its rows with several provenance URLs — all flow into the edgesource_record_urlslist. The legacy scalarurl: https://...is no longer accepted.Model
urlis a list:src/tablassert/models.py—BaseSource.url: list[HttpUrl] = Field(..., min_length=1, ...). One or more URLs per section; order is preserved.src/tablassert/lib.py—retrieval_sourcesnow receives[str(u) for u in self.source.url]. The single-URL path already wrapped the value in a one-element list, so this iterates the field directly with no behavior change for the one-URL case.Design
urlis strictly a list — a scalar is rejected, and an empty list is rejected viamin_length=1. No alias or coercion shim, so the old form fails loudly rather than being silently accepted.source_record_urls(previously always a single-entry list).Docs
docs/configuration/table.md(Excel/Text field rows and examples),docs/tutorial.md,docs/examples/tutorial-table.yaml,docs/examples.md,docs/configuration/advanced-example.md,examples/agent/QC_REPORT.md.CHANGELOG.md— a Breaking Changes entry under Unreleased with the migration snippet.src/tablassert/agent.py) and golden fixtures (tests/fixtures,tests/agent_fixtures) updated so derived configs stay schema-valid.Testing
.venv/bin/python -m pytest -o addopts="--cov=tablassert --cov-report=term-missing" -q→747 passed, 34 skipped(87% coverage).tests/test_models.py:test_source_url_accepts_multiple(order preserved),test_source_url_rejects_scalar,test_source_url_rejects_empty.test_docs_examplesvalidates the shipped examples against the live schema.ruff check --fix/ruff formatclean; pre-commit (ruff + cargo-fmt) passed on commit.Questions for the reviewer
source_record_urlssemantics. A section maps a single local file (local) but can now list severalurls, all landing insource_record_urls. If downstream Translator/RIG tooling expects a 1:1 source-record-url, say so and I'll split into one primary + extras instead of a flat list.Summary by CodeRabbit
Breaking Changes
Documentation
Tests