Repository navigation
claims: two claims of one kind in one sentence get two uids (#125) - #210
Conversation
…hose quotes overlap stay one (#125) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012wbKzQZyG5Aq6poybwciHt
|
You have reached your Codex usage limits for security reviews. Please try again later. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Reviewer's GuideThe PR adds an optional positional discriminator to claim evidence and incorporates it into UID derivation only when multiple non-overlapping claims of the same kind begin in one sentence. Curation assigns this discriminator by normalized kind, source sentence, and quote overlap before gates run, preserving legacy UIDs for existing and solitary claims while keeping identities stable across recuration. Sequence diagram for assigning stable UIDs to same-sentence claimssequenceDiagram
participant Curate
participant Number as curate::number
participant UID as claims::uid
participant Gates
participant Store
Curate->>Number: number(found)
Number->>Number: Group by normalized kind and sentence
Number->>Number: Merge overlapping quotes
Number-->>Curate: Set Evidence.nth
Curate->>Gates: check claims
Gates->>UID: uid(kind, first evidence)
UID-->>Gates: Hash kind, source, sentence, optional nth
Gates->>Store: Write claim operation
Flow diagram for same-sentence claim numberingflowchart TD
A[Located drafts] --> B[Sort by normalized kind, source, sentence, and quote offset]
B --> C{Same claim group?}
C -->|Overlapping quote| D[Reuse nth]
C -->|Non-overlapping quote| E[Increment nth]
C -->|First or new group| F[Set nth to 0]
D --> G[Derive UID]
E --> G
F --> G
G --> H[Claims retain identity across recuration]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db422d1815
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…, and takes its own start when its sentence holds another claim of its kind (review on #210) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012wbKzQZyG5Aq6poybwciHt
|
You have reached your Codex usage limits for security reviews. Please try again later. |
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6836a9aade
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
You have reached your Codex usage limits for security reviews. Please try again later. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6836a9aade
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…uote (#211) (#214) Closes #211. From the Codex review of #210 (round 2), moved to an issue under that PR's stop rule. ## What `curate::keyed` gives a draft the `claim_at` of the claim its quote overlaps in the same sentence. It looked at every derivation of that kind in the sentence, active or not. A claim first quoted with its whole sentence and later narrowed kept its old wide quote in the lookup. A later draft anywhere in the sentence then overlapped it and took the same uid, so one of two current claims was hidden. The lookup now reads each uid's active derivation only (`claims` joined to `derivations`). A draft joins a claim by the range the claim quotes now. ## Test `an_old_wide_quote_joins_no_claim_to_a_narrowed_one`: a claim quoted with the whole sentence, then narrowed, then a recuration that gives the narrow claim and one apart from it. The narrow claim keeps its uid, and the other gets its own. Matching against every derivation turns it red. ## Review stop rule (declared before the first round) A finding is fixed here only if a plain environment reaches it and it hides a current claim or changes the uid of a claim written before this change. Everything else goes to an issue. After two rounds in a row whose findings all go to issues, or a 👍, this PR is merged with no further commit once `check` and CodeQL are green. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_012wbKzQZyG5Aq6poybwciHt ## Summary by Sourcery Match draft claims against active quotes so narrowed claims no longer hide distinct claims in the same sentence. Bug Fixes: - Ensure drafts join only the currently active derivation of a claim, preventing an older wider quote from incorrectly absorbing a separate current claim. Tests: - Add regression coverage for preserving claim identity when a sentence-wide quote is narrowed and a separate claim is later recurated. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…216) Closes #213. Two knowledge.db migrations in `claims::schema` checked the old schema and then changed it with no lock between the two. A worker and a command (`oboete recurate`, `oboete search`) opening the same older knowledge.db at once could both find the old schema; the second then failed on "duplicate column name: claim_at", or rebuilt the queue table a second time. ## What - `claims::migrate(k, needed, change)`: reads `needed` first, so a current knowledge.db takes no write lock. When a change is needed: - outside a transaction, it takes `BEGIN IMMEDIATE`, asks `needed` again, and changes only if it still holds; - inside one (a consumer's step or rewind, both immediate), the lock is already held, so it changes at once, as before. - The `claim_at` column (#210) and the queue's key rebuild (#208) both go through it. - `db::ensure_column`, which the issue named, takes `&mut Connection`; `schema` is called with the step's transaction (`&Connection`), so the same check-again shape is written here with `Transaction::new_unchecked`. ## Test `two_connections_migrate_one_older_knowledge_db_at_once`: for an older evidence table, then an older queue table, a first connection holds the write lock (as a step does) while a second runs `schema`; the first then migrates and commits. Both succeed, the column is there once, the queue has its three-column key, and the rows are kept. On `main`'s `claims.rs` it fails 3 runs in 3 with "duplicate column name: claim_at". ## Review stop rule (declared before the first round) A finding is fixed here only if a plain environment reaches it (a worker, a hook and the CLI commands on one home; Linux, macOS or Windows) and it loses a row, stops a worker run, or leaves the schema half changed. Anything else goes to an issue. After two rounds in a row whose findings all go to issues, this PR is merged with no further commit once `check` and CodeQL are green. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_012wbKzQZyG5Aq6poybwciHt ## Summary by Sourcery Make knowledge.db schema migrations safe when workers and commands open an older database concurrently. Bug Fixes: - Prevent concurrent knowledge.db migrations from applying schema changes more than once or failing with duplicate-column errors. Enhancements: - Centralize migration checks so current databases avoid write locks while outdated databases recheck their schema under an immediate transaction. - Apply lock-safe migration handling to the claim_at column and recurate queue key migrations. Tests: - Add coverage for concurrent migrations of older evidence and queue schemas, verifying successful upgrades and preservation of rows. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 5 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
1 participant
Part of #125 (the Task 6 item: two claims in one sentence get two uids).
Why
A claim's uid is the hash of its kind and the sentence its first quote starts in (MUST-M18), so a recuration that rewords it derives the same uid. Two claims of one kind in one sentence therefore shared a uid. The later one's derivation became the active one, and the other claim was no longer shown. The dev arms' claim ops have such groups with quotes apart from each other: 2 in the full arm (290 claims), 5 in the whole arm (563) and 1 in the short arm (890). One of them held four proposals in one sentence, of which one was shown.
What (rewritten in round 2)
The first version numbered a sentence's claims by their order in one answer. Codex's round 1 showed why that cannot be an identity: the set of claims drafted with a claim changes between derivations. Model answers vary, the gates drop drafts, and a recuration finds one more claim. So a number moved from one claim to another. This version keys a claim by where it starts, and a claim already derived keeps its key:
Evidence.claim_at: Option<i64>, on a claim's first quote, and a newclaim_atcolumn of theevidencetable (added to an older knowledge.db, empty, as its ops had none).claims::uidadds it when present, and the op leaves it out when it isNone. So a claim alone in its sentence keeps the uid it always had, and every op written before this change keeps its uid.curate::keyed, when an answer's drafts are located and before the gates. Per kind and sentence:NonegetsNone;claim_atfrom theevidencetable with the rest of the quote, so it retracts the claim it leaves out (round 1, first finding).Limit: two windows of one
recuratespan are sent before the consumers run, so a claim in the second window does not see one drafted in the first. That needs a sentence longer than a window, sent again, with same-kind claims in both parts. The curation phase runs the consumers between windows.Tests
two_decisions_in_one_sentence_get_two_uids_and_keep_them_on_recuration: three drafts, two of which overlap, give two uids. A recuration with both claims reworded and in the other order keeps both.a_recuration_retracts_the_proposal_it_leaves_out_of_a_sentence: the second proposal is retracted, and the first stays.a_claim_found_earlier_in_a_sentence_leaves_the_uid_of_the_one_there: a recuration that finds a new earlier claim leaves the old claim's uid, and with it the owner's corrections of it.claims_in_two_windows_of_one_sentence_get_two_uids: a sentence split across windows.an_older_evidence_table_gets_its_claim_at_column.claim_at, the derived claims ignored, andclaim_atnever set.Review stop rule (declared before the first round)
A finding is fixed here only if a plain environment reaches it (the default or documented config, agent-produced text, Linux, macOS or Windows) and it either hides a current claim or changes the uid of a claim written before this change. Everything else goes to an issue. After two rounds in a row whose findings all go to issues, this PR is merged with no further commit once
checkand CodeQL are green.Added before round 2: a further finding about how a claim's identity is chosen goes to an issue, and this PR merges or closes by the rule above. The identity semantics are not changed a third time here.
🤖 Generated with Claude Code
https://claude.ai/code/session_012wbKzQZyG5Aq6poybwciHt