Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce46713478
ℹ️ 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".
src/compact/engine.ts
Outdated
| const obs = getObservation(doc.observationId); | ||
| let detail = formatObservationDetail({ | ||
| ...doc, | ||
| commitHash: obs?.commitHash, | ||
| relatedCommits: obs?.relatedCommits, |
There was a problem hiding this comment.
Scope observation lookup by project in detail formatting
compactDetail enriches each MemorixDocument by calling getObservation(doc.observationId), but getObservation matches only numeric ID and ignores projectId. For cross-project detail requests (the fallback path that fetches docs from Orama with projectId), a local observation with the same ID can be picked and its commitHash/relatedCommits copied into another project's detail, producing incorrect ✓ Repository-backed provenance and commit annotations. This is a correctness regression introduced by the new evidence-basis wiring.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR introduces “evidence basis” semantics as a computed, display-layer concept to surface when an observation is repository-backed (git-derived) versus direct/neutral, without changing storage/schema. It updates compact detail and timeline formatting to include repository-backed cues and adds focused tests for the derivation + formatting behavior.
Changes:
- Add
EvidenceBasis+ helpers (resolveEvidenceBasis,evidenceBasisLine) insrc/memory/disclosure-policy.ts. - Extend compact detail provenance headers and timeline anchor annotations to show repository-backed cues.
- Add
vitestcoverage for evidence basis derivation and output formatting.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tests/compact/evidence-basis.test.ts | Adds unit tests covering derivation rules and formatting in detail/timeline outputs. |
| src/memory/disclosure-policy.ts | Introduces evidence-basis computation + a display helper line. |
| src/compact/index-format.ts | Displays repository-backed verification in detail provenance headers and timeline anchor annotations. |
| src/compact/engine.ts | Threads commitHash/relatedCommits from in-memory observations into detail formatting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let detail = formatObservationDetail({ | ||
| ...doc, | ||
| commitHash: obs?.commitHash, | ||
| relatedCommits: obs?.relatedCommits, |
There was a problem hiding this comment.
In compactDetail, this reintroduces an ID-only getObservation(doc.observationId) lookup for every formatted document. When documents includes Orama fallback results from another project, getObservation returns the current project's observation with the same numeric ID (observations are stored per-project in-memory), which can attach the wrong commitHash/relatedCommits and display incorrect “Repository-backed” evidence (and potentially leak unrelated commit hashes). Consider only using the in-memory observation when obs?.projectId === doc.projectId (or threading the already project-validated obs alongside the doc when building documentMap).
| let detail = formatObservationDetail({ | |
| ...doc, | |
| commitHash: obs?.commitHash, | |
| relatedCommits: obs?.relatedCommits, | |
| const safeObs = obs && obs.projectId === doc.projectId ? obs : undefined; | |
| let detail = formatObservationDetail({ | |
| ...doc, | |
| commitHash: safeObs?.commitHash, | |
| relatedCommits: safeObs?.relatedCommits, |
src/memory/disclosure-policy.ts
Outdated
| // Repository-backed: grounded in an actual git commit | ||
| if ( | ||
| fields.commitHash || | ||
| (fields.relatedCommits?.length ?? 0) > 0 || | ||
| fields.source === 'git' || | ||
| fields.sourceDetail === 'git-ingest' | ||
| ) { |
There was a problem hiding this comment.
The comment says repository-backed is “grounded in an actual git commit”, but the condition also treats source === 'git' and sourceDetail === 'git-ingest' as repository-backed even when commitHash is missing. Either tighten the logic to require commit evidence (commitHash/relatedCommits) or update the comment to match the broader “git provenance implies repository-backed” rule.
src/memory/disclosure-policy.ts
Outdated
| * Return a compact one-line verification annotation for provenance headers | ||
| * and timeline anchor annotations. |
There was a problem hiding this comment.
evidenceBasisLine is documented as being used for “timeline anchor annotations”, but formatTimeline builds its own hard-coded suffix string instead. To avoid drift in phrasing/casing, either reuse evidenceBasisLine for the timeline path (possibly with an option for the lower-case variant) or adjust the docstring so it only claims the usages that actually exist.
| * Return a compact one-line verification annotation for provenance headers | |
| * and timeline anchor annotations. | |
| * Return a compact one-line verification annotation for provenance headers. |
src/compact/index-format.ts
Outdated
| if (hasSrc && anchorEffectiveSource) { | ||
| lines.push(`*Expanding: ${sourceKindLabel(anchorEffectiveSource)}*`); | ||
| const anchorBasis = resolveEvidenceBasis({ sourceDetail: anchor.sourceDetail, source: anchor.source }); | ||
| const basisSuffix = anchorBasis === 'repository' ? ' — ✓ repository-backed' : ''; |
There was a problem hiding this comment.
Timeline anchor annotations hard-code — ✓ repository-backed instead of reusing evidenceBasisLine, which produces ✓ Repository-backed (different casing/wording). This duplication makes it easy for the two outputs to drift. Consider centralizing the formatting (e.g., a dedicated helper for the timeline suffix) so both detail and timeline stay consistent.
| const basisSuffix = anchorBasis === 'repository' ? ' — ✓ repository-backed' : ''; | |
| const basisLine = evidenceBasisLine(anchorBasis); | |
| const basisSuffix = basisLine ? ` — ${basisLine}` : ''; |
Summary
What changed
EvidenceBasishelpers insrc/memory/disclosure-policy.tsScope
This phase stays intentionally small:
Validation
npm run buildnpx vitest run tests/compact/evidence-basis.test.ts tests/compact/detail-provenance.test.ts tests/compact/timeline-provenance.test.ts