fix(longitudinal): reconcile unique Commons observation identity with main - #289
seonghobae wants to merge 10 commits into
Conversation
|
Warning Review limit reachedNext included review available in 6 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
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 |
|
Updated exact head |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head6500c81d6ea7ec633d21c7b15906d0e7c89e53f3. -
Head SHA:
6500c81d6ea7ec633d21c7b15906d0e7c89e53f3 -
Workflow run: 32700326072
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (2 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (2 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs: TRACEABILITY.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: TRACEABILITY.md"]
R2 --> V2["docs review"]
Evidence --> S3["Test: longitudinal_observation_record_identity.rs"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test: longitudinal_observation_record_identity.rs"]
R3 --> V3["targeted test run"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage Decision
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (2 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (2 files)"]
R1 --> V1["required checks"]
Evidence --> S2["Docs: TRACEABILITY.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: TRACEABILITY.md"]
R2 --> V2["docs review"]
Evidence --> S3["Test: longitudinal_observation_record_identity.rs"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test: longitudinal_observation_record_identity.rs"]
R3 --> V3["targeted test run"]
|
# Conflicts: # CHANGELOG.md
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
|
|
||
| ### Added | ||
| - Active PR #287 rejects padded or otherwise noncanonical published narrative, style-mapping, interpretation-unit, and approved-selection references before deterministic rendering. Numeric score authority and the protected-main narrative boundary are unchanged; this is active-PR evidence only and not a release claim. | ||
| - Merged #289 rejects reuse of a Commons observation-record identity by a distinct longitudinal source observation once integrated; exact source replay remains idempotent, and a rejected collision does not replace accepted evidence. |
There was a problem hiding this comment.
🟡 Changelog marks the current unmerged PR as merged
The Added entry leads with Merged #289, but this is the open PR under review, and the traceability row edited in the same change labels it **Active PR #289** (docs/TRACEABILITY.md:51). Representing an active PR as protected-main implementation contradicts the documentation-status rule.
| - Merged #289 rejects reuse of a Commons observation-record identity by a distinct longitudinal source observation once integrated; exact source replay remains idempotent, and a rejected collision does not replace accepted evidence. | |
| - Active PR #289 rejects reuse of a Commons observation-record identity by a distinct longitudinal source observation once integrated; exact source replay remains idempotent, and a rejected collision does not replace accepted evidence. |
Was this helpful? React with 👍 or 👎 to provide feedback.
| ### Added | ||
| - Active PR #287 rejects padded or otherwise noncanonical published narrative, style-mapping, interpretation-unit, and approved-selection references before deterministic rendering. Numeric score authority and the protected-main narrative boundary are unchanged; this is active-PR evidence only and not a release claim. | ||
| - Merged #289 rejects reuse of a Commons observation-record identity by a distinct longitudinal source observation once integrated; exact source replay remains idempotent, and a rejected collision does not replace accepted evidence. | ||
| - Merged #287 rejected padded or otherwise noncanonical published narrative, style-mapping, interpretation-unit, and approved-selection references before deterministic rendering. Numeric score authority and the protected-main narrative boundary are unchanged. |
There was a problem hiding this comment.
🟡 Changelog and traceability disagree on #287 status
This entry relabels #287 as Merged #287, but the traceability row still calls the same work **Active PR #287** (docs/TRACEABILITY.md:35). The two docs contradict each other on whether #287 is protected-main implementation.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Why
Protected
maintreatsobservation_record_refas the opaque Commons identity of one immutable longitudinal observation, while the currentLongitudinalObservationSetonly enforces uniqueness of the Gyeot source tuple(enrollment_ref, source_system_ref, source_observation_ref). Two distinct source observations can therefore reuse one Commons record identity and both be accepted. That is a data-integrity defect and conflicts with the repository's immutable product-identity direction.Historical PR #262 contains the same narrow domain fix but is based at
503a4e640eeba0f5e126fa4c4078d8d21aebb93b, one protected-main commit behind current4499d9c0889c082487ddbd7fd8d0d5d18257995d. The intervening #251 integration touches scoring/runtime files (src/lib.rs,src/scoring_engine.rs,tests/scoring_engine_adapter_contract.rs,tests/session_http_framing.rs) and does not overlap this longitudinal source/test slice. Historical checks, reviews, and resolved threads on #262 nevertheless do not transfer to a new exact head.Current-main TDD lineage
3d8e198b16026765ebd5a43c923cbe9b2183aaeeadds the focused identity-collision contract directly on current protected main. Against current-main source it references the missingObservationIdentityConflictbehavior and therefore cannot compile/pass by construction; no local execution claim is made.feeaab354ad6aa78c288183e46d7c0995b4ab51fapplies the narrow domain guard. The final source blobc7fbd40e17d294826fc5d70ef0ee6951f3ff1b67and regression-test bloba09ba88cb530d73ea9bd757665d92123296bac95are byte-identical to the corresponding final, reviewed blobs on fix(longitudinal): reject reused Commons observation identities #262 while this branch preserves direct ancestry from current protected main.What
observation_record_ref.IdempotencyConflict.ObservationIdentityConflicterror so operators can distinguish a Commons record-identity collision from source idempotency conflict.Gyeot remains the collection owner and TEPP remains the temporal/multilevel/multiple-membership analysis owner. This PR does not add persistence, HTTP, scoring, psychometric kernels, or cross-service database access.
Documentation reconciliation
The predecessor also changed
docs/TRACEABILITY.mdwith text explicitly naming active PR #262. That stale active-PR statement is intentionally not copied verbatim. The canonical traceability row will be reconciled on this current landing vehicle so it identifies this PR asIMPLEMENTED_ON_ACTIVE_PRwithout promoting the behavior to protected-main truth before merge.Supersession
This is the current-protected-main landing vehicle for #262. Once this PR is confirmed open at an unchanged exact head and current base, #262 should remain closed rather than rebased or merged as a competing writer lane.
Acceptance
Do not merge until the unchanged final exact head passes Runtime CI, exact line/branch coverage, rustfmt/Clippy/rustdoc, security/SAST/SBOM/provenance, zero valid unresolved findings, and qualifying independent non-author review where required. Pending, queued, skipped, cancelled, stale, predecessor, synthetic, or model-only evidence is not passing. Never self-approve.