Harden wrapped media tag parsing and batch tag ordering#22
Harden wrapped media tag parsing and batch tag ordering#22brianmeyer merged 3 commits intomasterfrom
Conversation
|
Closing temporarily while I push a CI-stabilization follow-up onto the same branch and reopen/reuse the branch with fresh checks. |
![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: ceb95e473f
ℹ️ 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".
|
|
||
| canonical_path = representative.memory_root_path or top_hit.memory_root_path | ||
| if canonical_path: | ||
| representative.filepath = canonical_path |
There was a problem hiding this comment.
Assigning representative.filepath = canonical_path replaces the URI-style path (recallforge://{collection}/{file_path}) with memory_root_path, which is only the raw file_path from storage metadata. In cross-collection searches this makes distinct results with the same file path collide and can cause incorrect dedup/merging (for example search_batch merges by filepath), so the rolled-up result should keep a collection-qualified filepath and only use the root path for display/provenance fields.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 549da58bae
ℹ️ 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".
| canonical_path = representative.memory_root_path or top_hit.memory_root_path | ||
| if canonical_path: | ||
| representative.filepath = canonical_path | ||
| representative.display_path = canonical_path |
There was a problem hiding this comment.
Setting representative.filepath to memory_root_path drops the recallforge://<collection>/... namespace and rewrites rolled-up results to a raw storage path. In mixed-collection searches, two different memories with the same root-relative path now collide because downstream merging/dedup keys on filepath (e.g. search_batch), so results can be merged incorrectly or hidden.
Useful? React with 👍 / 👎.
| representative.memory_primary_evidence_path = top_hit.filepath | ||
| representative.memory_supporting_paths = [ | ||
| item.filepath | ||
| for item in group | ||
| if item.filepath != representative.filepath |
There was a problem hiding this comment.
memory_supporting_paths is filtered against representative.filepath instead of memory_primary_evidence_path, so when the top hit is a child asset the same path appears in both primary and supporting evidence. This duplicates provenance entries and can evict a real supporting sibling because the list is truncated to 5 items.
Useful? React with 👍 / 👎.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 participant
Summary
Testing