fix: stop forcing document paths to uppercase, guard platform context file case - #281
Open
Ruari-Phipps wants to merge 1 commit into
Open
fix: stop forcing document paths to uppercase, guard platform context file case#281Ruari-Phipps wants to merge 1 commit into
Ruari-Phipps wants to merge 1 commit into
Conversation
Contributor
Coverage Report
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refines Document case-handling to avoid renaming ordinary documents by preserving their original path casing, while still enforcing the platform’s exact-case requirement for the special CONTEXT.MD file. It updates discovery/projection parsing accordingly and adjusts the unit/integration test fixtures to cover the new behavior.
Changes:
- Stop uppercasing all
Document.pathvalues; preserve original case and only validate exact-case forCONTEXT.MD. - Fix document discovery to stop force-uppercasing discovered filenames; improve projection name derivation to strip both
.mdand.MD. - Update/add tests and test-project fixtures to cover preserved-case behavior and the
CONTEXT.MDconstraint.
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Updates the locked editable package version for polyai-adk. |
src/poly/resources/documents.py |
Preserves document path casing; validates CONTEXT.MD exact-case; avoids uppercasing discovered filenames; adjusts projection name parsing. |
src/poly/tests/resources_test.py |
Updates unit tests to assert case preservation and adds validation tests for CONTEXT.MD. |
src/poly/tests/project_test.py |
Updates round-trip/discovery expectations to match preserved-case behavior and adds CONTEXT.MD to fixtures. |
src/poly/tests/test_projects/test_project/test_project.json |
Adds a CONTEXT.MD document fixture entry. |
src/poly/tests/test_projects/test_project/context/test_document.md |
Adds a preserved-case test document fixture file. |
src/poly/tests/test_projects/test_project/context/CONTEXT.MD |
Adds the platform context file fixture with exact-case name. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
119
to
123
| for file_name in os.listdir(context_path): | ||
| if not file_name.upper().endswith(".MD"): | ||
| continue | ||
| file_path = os.path.join(context_path, file_name.upper()) | ||
| file_path = os.path.join(context_path, file_name) | ||
| file_paths.append(file_path) |
Comment on lines
209
to
213
| self.assertEqual(restored.resource_id, "test.md") | ||
| self.assertEqual(restored.name, "test") | ||
| self.assertEqual(restored.path, "TEST.MD") | ||
| self.assertEqual(restored.contents, "hello world\n") | ||
| self.assertEqual(restored.file_path, os.path.join("context", "TEST.MD")) | ||
| self.assertEqual(restored.path, "test.md") | ||
| self.assertEqual(restored.file_path, os.path.join("context", "test.md")) | ||
| self.assertEqual(restored.compute_hash(), doc.compute_hash()) |
Comment on lines
+50
to
+53
| if self.path.upper() == PLATFORM_CONTEXT_FILE and self.path != PLATFORM_CONTEXT_FILE: | ||
| raise ValueError( | ||
| f"Document path must be {PLATFORM_CONTEXT_FILE} (case-sensitive) for the platform context file." | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stops forcing every Document's path to uppercase on construction, and instead only enforces exact-case for the platform's special
CONTEXT.MDcontext file.Motivation
#231 normalized all document paths to uppercase to fix a case-sensitivity conflict, but that force-uppercased every document rather than just the one path (
CONTEXT.MD) the platform treats specially, causing unwanted renames for ordinary documents.discover_resources()also still forced the discovered path to uppercase, which could construct a file path that doesn't exist on case-sensitive filesystems.Changes
Document.__post_init__uppercase normalization; paths now preserve their original casevalidate()now only errors when a path case-insensitively matchesCONTEXT.MDbut isn't the exact-caseCONTEXT.MDdiscover_resources()no longer force-uppercases discovered file namesfrom_projection()strips both.mdand.MDsuffixes when derivingnameCONTEXT.MDvalidation and aCONTEXT.MDfixtureTest strategy
poly <command>)Checklist
ruff check .andruff format --check .passpytestpassespolyCLI interface (or migration path documented)Screenshots / Logs
N/A