Repository navigation
feat(pg): Version-aware Argument citation API (L-R-1 #634, PR 6) - #949
Conversation
A citation list may name each cited Version explicitly:
[{work_id, manifestation_id, pages}], manifestation_id null for the Work
in general. Such a list is stored as given, may cite one Work several
times (only exact repeats are refused), and refuses a Version this Library
lacks (ARGUMENT_SOURCE_VERSION_NOT_FOUND, 404) or one of another Work
(ARGUMENT_SOURCE_VERSION_MISMATCH, 409). Legacy {work_id, pages} lists keep
their pin resolution. The named Versions are held FOR KEY SHARE after the
Works, so a Version move or delete waits for the command. Citation rows
read back carry manifestation_id. The PostgreSQL sync handler validates the
Version-aware envelope for CREATE_ARGUMENT and SET_ARGUMENT_SOURCES and
compares pins when deciding a stale base.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017jgbjrTtg1i9iUrndjRxvp
This comment has been minimized.
This comment has been minimized.
|
Architecture Data flow Review ✨ Nothing flagged · reviewed Tip Click the link under each diagram to open it on a canvas you can zoom, pan and step through 🪧 More tips
Thanks for using PR Lens! It's free for OSS. If it helped, a star helps other teams find it. |
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Citation validation, locking, and persistence backend/domain/research/arguments.py, backend/domain/research/__init__.py, tests/pg/test_arguments.py, tests/test_arguments_parity.py, AGENTS.md |
The Research domain accepts Version-aware citation entries, checks that pins exist and belong to the named Work, and includes Version IDs in citation readback. Legacy lists retain their existing behavior, including pinning pinpoint citations only when a Work has one Version. Tests cover validation, Version changes, concurrency, and parity normalization. |
Sync source-list validation and revisions backend/sync/handlers/argument.py, backend/pg_facade/research.py, tests/pg/test_arguments_sync.py, docs/work-identity-model.md |
Sync handlers validate Version-aware source-list entries and reject malformed envelopes. Tests cover Version refusals, pin-aware revision comparisons, and concurrent updates. The facade and model documentation describe the new citation shape. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Feature
Sequence Diagram(s)
sequenceDiagram
participant Client as Sync client
participant Handler as Argument sync handler
participant Domain as Research arguments
participant PostgreSQL
Client->>Handler: Submit CREATE_ARGUMENT or SET_ARGUMENT_SOURCES
Handler->>Handler: Validate source-list envelope
Handler->>Domain: Pass validated payload
Domain->>PostgreSQL: Lock cited Works and Versions
Domain->>PostgreSQL: Store or replace citation rows
PostgreSQL-->>Domain: Return database result
Domain-->>Handler: Return result or refusal
Handler-->>Client: Return operation response
Possibly related issues
- Fooftilly/PRKS issue 41 requests citations that select a specific edition, version, or translation. This change adds Version-aware Argument citations.
- Fooftilly/PRKS issue 614 identifies L-R-1 Research as an unfinished PostgreSQL release slice. This change implements a feature within that slice.
Possibly related PRs
- Fooftilly/PRKS#938: This PR extends the PostgreSQL Argument implementation and citation-list contracts introduced there.
Suggested labels: enhancement, area:architecture, area:sync
Suggested reviewers: claude
Merge Risk: 
Minimal · up to 7d8a9
-
No merge-blocking issue was established; the change is ready for normal checks.
Pre-merge checks: 
7 · 
1
-
Failed checks: 1 warning Passed checks: 7 passed-
Full details: Prks Engineering Invariants
-
Explanation
PASS. The changed behavior stays within the existing PostgreSQL transition and research/sync modules.
AGENTS.mdandREADME.mdstate that PostgreSQL is not the active runtime and that SQLite remains authoritative; this PR does not change the server, SQLite code, or route cutover. No schema files, storage paths, logging, credentials, or security controls changed. The new citation behavior also matches the documented research invariant for Version-aware citations and exact-duplicate handling.
Full details: Offline And Sync Coherence
-
Explanation
PASS. The PR changes PostgreSQL Argument citation handling, not the current SQLite/browser offline path. The rollout status already marks Arguments and Stances as durable. The PostgreSQL sync handler preserves the documented aggregate model and adds explicit envelope validation, version refusals, stale-base convergence/conflict handling, and transactional create/replace behavior. New tests cover malformed envelopes, ledgered refusals with no mutation, stale pin conflicts, merged Works, and concurrent version-aware updates. The PR does not change service-worker, durable-queue, recovery, or offline projection code.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 42.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 19 functions across 7 files. (2 skipped: 2 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
This comment has been minimized.
This comment has been minimized.
|
@coderabbitai review |
✅ Action performed
|
Summary
This is L-R-1 PR 6. On PostgreSQL, a citation can now name the exact Version (Manifestation) it cites. Before this change, only the legacy shape
{work_id, pages}was accepted, and a pinpoint on a Work with several Versions was refused withARGUMENT_SOURCE_VERSION_REQUIRED. The existing citation code inbackend/domain/research/arguments.pyis extended; there is no second citation system.Request contract
The same entry points accept it:
create_argument(sources=…)andreplace_argument_sources(…)on the facade, andCREATE_ARGUMENT/SET_ARGUMENT_SOURCESin sync. A list comes in one of two shapes. If any entry carriesmanifestation_id, the list is Version-aware.{work_id, manifestation_id, pages}manifestation_idis a Version id, ornullfor the Work in general. The list is stored exactly as given, in order. A Work may appear several times (Work-level, several Versions, several pinpoints). Only an exact repeat of(work_id, manifestation_id, pages)is refused (#60 §8.5).{work_id, pages}ARGUMENT_SOURCE_VERSION_REQUIRED(409). A repeated Work is still refused, as on SQLite.A list that mixes the two shapes is refused with
invalid_sources.Refusals of a Version-aware list. HTTP-style errors are
ResearchErrorcodes; sync results are uppercase codes.argument_source_version_not_found/ARGUMENT_SOURCE_VERSION_NOT_FOUNDargument_source_version_mismatch/ARGUMENT_SOURCE_VERSION_MISMATCHwork_not_found(unchanged)manifestation_idneither a string nornull; exact repeatinvalid_sourcesinvalid_pages(unchanged)In sync, each client-checkable shape error is
INVALID_ENVELOPE. That covers an extra key, an untrimmed id, an exact repeat, more thanMAX_SOURCESentries, and bad pages.Response contract
Every citation row in Argument detail and the Argument list carries
manifestation_id, which isnullfor a Work-level citation. That is enough to send a pinned citation back without losing its identity. All other fields are unchanged.Revisions and conflicts
The list is still one aggregate,
argument-sources/<A>, and it advances only on a change.SET_ARGUMENT_SOURCEScompares against the stored list including pins. The same pins with a stale base converge; a different pin conflicts withREVISION_CONFLICT.Concurrency
FOR KEY SHARE, then the named Versions, by id, then the Argument row. This is documented inbackend/domain/research/__init__.py.ON UPDATE CASCADE. A citation that waited on a move re-reads after the retry and is refused withMISMATCH.argument_sources_manifestation_fkey.CREATE_ARGUMENTandSET_ARGUMENT_SOURCESstay SERIALIZABLE peer writers of a Work merge. A pinned citation added after a merge's snapshot makes the merge fail with 40001, and the retry sees it.Related issues
Refs #634
Scope / non-goals
server.pystill serves the Research routes from SQLite, so the live API does not accept the new shape yet.Validation
Local results
tests/pg(scripts/test-postgres)scripts/test-postgres --parity)tests/pg/test_arguments.py+test_arguments_sync.pytest_argument_sync,test_work_identity,test_contract_parity,test_research_network,test_engineering_invariantsscripts/check_invariants.pyNew tests, mapped to the requested cases:
test_a_work_with_one_versiontest_a_work_with_several_versions_is_cited_version_by_versiontest_a_non_primary_version_survives_a_change_of_pages_or_ordertest_a_version_missing_or_of_another_work_or_library_refuses_the_listtest_a_pinned_version_cannot_be_deleted_and_follows_a_move; racestest_a_version_move_waits_for_the_citation_pinning_it,test_a_citation_racing_a_move_of_its_version,test_a_version_delete_waits_for_the_citation_pinning_it_and_is_refusedtest_two_version_aware_lists_of_one_argument_queue,test_a_citation_after_a_merge_snapshot_fails_the_merge_and_its_retry_sees_it, synctest_two_lists_on_one_base_one_lands_the_other_conflictstest_a_legacy_list_cannot_rewrite_a_list_citing_one_work_twicetest_a_version_the_legacy_list_cannot_name_refuses_the_listtest_create_and_replace_with_pins_and_the_decision_table,test_a_version_refusal_is_ledgered_and_changes_nothingtest_the_envelope_of_a_version_aware_listRaces hold one transaction open until the other is observed waiting in
pg_locks. The merge race uses events; no test depends on timing. E2E was not run locally because no route or frontend code changed.GitHub CI
Reported separately once the run finishes.
Migration / compatibility impact
argument_sourcestable from feat(pg): Research schema on PostgreSQL (L-R-1 #634, PR 1) #936 already has the identity this PR uses.manifestation_id. The parity comparator drops that key on both sides.Checklist
Generated by Claude Code
Summary by CodeRabbit