Fix extractIndexFromPath RFC 6901 prefix matching and clean up test comments#1863
Conversation
…ments Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
This PR tightens JSON Pointer path matching for DIFC PathLabeledData to follow RFC 6901 semantics, ensuring item-path matching only occurs on proper /-delimited segments (and not on raw string prefixes like /items3 matching /items).
Changes:
- Remove the
extractIndexFromPathbranch that treateditemsPathas a raw prefix without requiring a/separator. - Update coverage tests to assert an error for the no-slash-separator case (
items3vsitems). - Minor test comment cleanups (remove coverage-percentage notes / improve wording).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| internal/difc/path_labels.go | Enforces strict itemsPath + "/" matching when extracting an index from a JSON Pointer path. |
| internal/difc/path_labels_coverage_test.go | Updates/renames a test to validate the corrected no-slash-separator behavior; clarifies resolve caching test comments. |
| internal/difc/labels_test.go | Minor comment cleanup and formatting in TestLabel_Intersect. |
| internal/difc/agent_test.go | Minor comment cleanup in TestAgentLabels_AddIntegrityTags. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
extractIndexFromPathhad a buggy thirdHasPrefixbranch that matched/items3against itemsPath/items, incorrectly associating it as index 3. Per RFC 6901,/items3is a distinct key — only paths separated by/(e.g.,/items/3) should match.Changes
path_labels.go: Remove the erroneous third branch; valid matches now requireitemsPath + "/"prefixpath_labels_coverage_test.go:TestExtractIndexFromPath_ThirdHasPrefix→TestExtractIndexFromPath_NoSlashSeparator; flip expectation from success to errorTestPathLabeledData_ResolveNotCalledTwicecomment — the guard being exercised is inGetItems(), not insideresolve()labels_test.go,agent_test.go: Remove stale historical coverage percentages from test comments🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.