-
Notifications
You must be signed in to change notification settings - Fork 259
test(skills): add issue-derived Pythonic code evals #1099
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
befe112
test(skills): add issue-derived Pythonic code evals
phernandez f83bfd9
fix(skills): make eval workspace uv-compatible
phernandez a8fca87
fix(skills): exclude eval virtualenv outputs
phernandez 081abd4
fix(skills): isolate paired eval trials
phernandez a3a8b24
fix(skills): reject eval artifact symlinks
phernandez File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| # Evaluation Workspace | ||
|
|
||
| - Use Python 3.12 or newer. | ||
| - Read each target file completely before editing it. | ||
| - Keep changes scoped and preserve observable behavior unless the task says otherwise. | ||
| - Use full type annotations and prefer direct functions and typed values over unnecessary classes. | ||
| - Run the focused tests under `evals/files` and the configured Ruff checks for changed Python. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| [project] | ||
| name = "pythonic-code-eval" | ||
| version = "0.0.0" | ||
| requires-python = ">=3.12" | ||
| dependencies = [] | ||
|
|
||
| [dependency-groups] | ||
| dev = [ | ||
| "pytest>=8.0", | ||
| "ruff>=0.9", | ||
| ] | ||
|
|
||
| [tool.pytest.ini_options] | ||
| addopts = "-q" | ||
| testpaths = ["evals/files"] | ||
|
|
||
| [tool.ruff] | ||
| line-length = 100 | ||
| target-version = "py312" | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| { | ||
| "skill_name": "pythonic-code", | ||
| "evals": [ | ||
| { | ||
| "id": 1096, | ||
| "eval_name": "shared-runtime-ownership", | ||
| "source_issue": "https://github.com/basicmachines-co/basic-memory/issues/1096", | ||
| "prompt": "Review evals/files/shared_runtime_ownership.md. Describe the runtime-neutral target package ownership and dependency direction, then recommend the smallest first cleanup PR within an incremental rollout that does not change runtime behavior. Account for each misplaced portable facade, project-scoped repository bundle, and search persistence type or helper in the snapshot, even when deferring it from the first PR. Do not edit the fixture. Be concrete about moves or renames, compatibility during the paired cloud rollout, documentation, and validation.", | ||
| "expected_output": "A staged architecture recommendation that moves portable note behavior out of cloud-named ownership, gives project-scoped repositories a runtime-neutral name, lowers search DTO/SQL dependencies beneath repositories, uses temporary compatibility exports for the paired cloud revision, and records the dependency direction without proposing another manager or all-at-once package rewrite.", | ||
| "files": [ | ||
| "evals/files/shared_runtime_ownership.md" | ||
| ], | ||
| "assertions": [ | ||
| "The response identifies basic_memory.cloud as misleading ownership for portable core note behavior and proposes a neutral core destination.", | ||
| "The response gives LocalAcceptedNoteRepositories a runtime-neutral project-scoped name rather than preserving origin-based naming.", | ||
| "The response moves or re-homes AcceptedNoteSearchRow and vector-delete behavior so repository modules do not depend upward on indexing orchestration.", | ||
| "The rollout is incremental and includes temporary compatibility exports or an equivalent coordinated core/cloud migration strategy.", | ||
| "The response records the intended dependency direction and names concrete core plus cloud validation without introducing managers, registries, or an all-at-once package rewrite." | ||
| ] | ||
| }, | ||
| { | ||
| "id": 1097, | ||
| "eval_name": "accepted-snapshot-persistence", | ||
| "source_issue": "https://github.com/basicmachines-co/basic-memory/issues/1097", | ||
| "prompt": "Refactor evals/files/accepted_snapshot.py and its tests. A previous mutation path returned success after persisting content and search but before observations and relations existed. Make that class of omission structurally difficult for future full-note mutations while preserving the intentionally narrower move behavior. Keep transaction ownership explicit and add the regression coverage needed to prove the change.", | ||
| "expected_output": "One direct accepted-snapshot persistence operation writes content, search, observations, and relations through the caller-owned session. Create and edit use it; move uses an explicitly narrower private or move-specific operation. Tests prove create and edit persist the full snapshot immediately while move does not replace unchanged graph state.", | ||
| "files": [ | ||
| "evals/files/accepted_snapshot.py", | ||
| "evals/files/test_accepted_snapshot.py" | ||
| ], | ||
| "assertions": [ | ||
| "One clearly named production operation persists content, search, observations, and relations for a full accepted snapshot.", | ||
| "Both create_note and edit_note use the complete snapshot operation rather than separately remembering a graph call.", | ||
| "The content/search-only operation is private, move-specific, or otherwise unavailable as the tempting default for full-note mutations.", | ||
| "The caller-owned session is passed through explicitly and the refactor does not create a hidden transaction or service hierarchy.", | ||
| "Focused tests prove complete create and edit persistence plus the intentionally narrower move behavior." | ||
| ] | ||
| }, | ||
| { | ||
| "id": 1098, | ||
| "eval_name": "accepted-preparation-surface", | ||
| "source_issue": "https://github.com/basicmachines-co/basic-memory/issues/1098", | ||
| "prompt": "Refactor evals/files/accepted_preparation.py and its tests to make the accepted-note preparation path easier to trace and cheaper to compose. Preserve create preparation and full EntityService behavior, keep genuine replaceable capabilities typed, and avoid changing the observable Markdown, permalink, conflict, or parsing semantics.", | ||
| "expected_output": "Preparation becomes direct module-level behavior over frozen typed values and narrow behavioral capabilities. The accepted path no longer constructs the full EntityService merely to call prepare_create_entity_content, property-only source protocols disappear, and the existing service can delegate to the same function without a replacement manager or registry.", | ||
| "files": [ | ||
| "evals/files/accepted_preparation.py", | ||
| "evals/files/test_accepted_preparation.py" | ||
| ], | ||
| "assertions": [ | ||
| "Property-only prepared-data protocols are replaced by a small number of frozen typed values.", | ||
| "Behavioral protocols remain only for genuine replaceable capabilities such as storage existence, permalink resolution, or Markdown parsing.", | ||
| "The accepted-note composition path does not construct the full EntityService solely to prepare a create.", | ||
| "Preparation is expressed with direct module-level functions and ordinary arguments or partial binding, without a replacement Manager, strategy hierarchy, or registry.", | ||
| "Focused tests preserve create preparation and EntityService delegation semantics, including the existing-file conflict." | ||
| ] | ||
| } | ||
| ] | ||
| } |
150 changes: 150 additions & 0 deletions
150
.agents/skills/pythonic-code/evals/files/accepted_preparation.py
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,150 @@ | ||
| """Condensed accepted-note preparation composition based on Basic Memory issue #1098.""" | ||
|
|
||
| from dataclasses import dataclass | ||
| from typing import Protocol | ||
|
|
||
|
|
||
| @dataclass(frozen=True, slots=True) | ||
| class NoteSchema: | ||
| title: str | ||
| file_path: str | ||
| body: str | ||
|
|
||
|
|
||
| class PreparedEntityFieldsSource(Protocol): | ||
| @property | ||
| def title(self) -> str: ... | ||
|
|
||
| @property | ||
| def file_path(self) -> str: ... | ||
|
|
||
| @property | ||
| def permalink(self) -> str: ... | ||
|
|
||
|
|
||
| class PreparedMarkdownSource(Protocol): | ||
| @property | ||
| def markdown(self) -> str: ... | ||
|
|
||
| @property | ||
| def search_text(self) -> str: ... | ||
|
|
||
| @property | ||
| def entity_fields(self) -> PreparedEntityFieldsSource: ... | ||
|
|
||
|
|
||
| @dataclass(frozen=True, slots=True) | ||
| class PreparedEntityFields: | ||
| title: str | ||
| file_path: str | ||
| permalink: str | ||
|
|
||
|
|
||
| @dataclass(frozen=True, slots=True) | ||
| class PreparedMarkdown: | ||
| markdown: str | ||
| search_text: str | ||
| entity_fields: PreparedEntityFields | ||
|
|
||
|
|
||
| class FileStore(Protocol): | ||
| async def exists(self, file_path: str) -> bool: ... | ||
|
|
||
|
|
||
| class PermalinkResolver(Protocol): | ||
| async def resolve(self, file_path: str, title: str) -> str: ... | ||
|
|
||
|
|
||
| class MarkdownParser(Protocol): | ||
| async def parse(self, file_path: str, markdown: str) -> None: ... | ||
|
|
||
|
|
||
| class EntityService: | ||
| """Full file/DB service; accepted composition currently builds it only to prepare.""" | ||
|
|
||
| def __init__( | ||
| self, | ||
| *, | ||
| file_store: FileStore, | ||
| permalink_resolver: PermalinkResolver, | ||
| markdown_parser: MarkdownParser, | ||
| entity_repository: object, | ||
| observation_repository: object, | ||
| relation_repository: object, | ||
| search_service: object, | ||
| ) -> None: | ||
| self.file_store = file_store | ||
| self.permalink_resolver = permalink_resolver | ||
| self.markdown_parser = markdown_parser | ||
| self.entity_repository = entity_repository | ||
| self.observation_repository = observation_repository | ||
| self.relation_repository = relation_repository | ||
| self.search_service = search_service | ||
|
|
||
| def _build_fields( | ||
| self, | ||
| schema: NoteSchema, | ||
| permalink: str, | ||
| ) -> PreparedEntityFields: | ||
| return PreparedEntityFields( | ||
| title=schema.title, | ||
| file_path=schema.file_path, | ||
| permalink=permalink, | ||
| ) | ||
|
|
||
| async def _build_prepared( | ||
| self, | ||
| schema: NoteSchema, | ||
| fields: PreparedEntityFields, | ||
| ) -> PreparedMarkdown: | ||
| markdown = f"---\npermalink: {fields.permalink}\n---\n# {schema.title}\n\n{schema.body}\n" | ||
| await self.markdown_parser.parse(schema.file_path, markdown) | ||
| return PreparedMarkdown( | ||
| markdown=markdown, | ||
| search_text=f"{schema.title}\n\n{schema.body}", | ||
| entity_fields=fields, | ||
| ) | ||
|
|
||
| async def prepare_create_entity_content( | ||
| self, | ||
| schema: NoteSchema, | ||
| ) -> PreparedMarkdownSource: | ||
| if await self.file_store.exists(schema.file_path): | ||
| raise FileExistsError(schema.file_path) | ||
| permalink = await self.permalink_resolver.resolve(schema.file_path, schema.title) | ||
| fields = self._build_fields(schema, permalink) | ||
| return await self._build_prepared(schema, fields) | ||
|
|
||
| async def create_entity(self, schema: NoteSchema) -> PreparedMarkdownSource: | ||
| prepared = await self.prepare_create_entity_content(schema) | ||
| return prepared | ||
|
|
||
|
|
||
| @dataclass(frozen=True, slots=True) | ||
| class LocalAcceptedNotePreparerFactory: | ||
| file_store: FileStore | ||
| permalink_resolver: PermalinkResolver | ||
| markdown_parser: MarkdownParser | ||
| entity_repository: object | ||
| observation_repository: object | ||
| relation_repository: object | ||
| search_service: object | ||
|
|
||
| def create_note_preparer(self) -> EntityService: | ||
| return EntityService( | ||
| file_store=self.file_store, | ||
| permalink_resolver=self.permalink_resolver, | ||
| markdown_parser=self.markdown_parser, | ||
| entity_repository=self.entity_repository, | ||
| observation_repository=self.observation_repository, | ||
| relation_repository=self.relation_repository, | ||
| search_service=self.search_service, | ||
| ) | ||
|
|
||
|
|
||
| async def prepare_accepted_note_create( | ||
| factory: LocalAcceptedNotePreparerFactory, | ||
| schema: NoteSchema, | ||
| ) -> PreparedMarkdownSource: | ||
| preparer = factory.create_note_preparer() | ||
| return await preparer.prepare_create_entity_content(schema) |
130 changes: 130 additions & 0 deletions
130
.agents/skills/pythonic-code/evals/files/accepted_snapshot.py
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| """Condensed accepted-note persistence flow based on Basic Memory issue #1097.""" | ||
|
|
||
| from dataclasses import dataclass | ||
| from typing import Protocol | ||
|
|
||
|
|
||
| @dataclass(frozen=True, slots=True) | ||
| class Entity: | ||
| id: int | ||
| project_id: int | ||
|
|
||
|
|
||
| @dataclass(frozen=True, slots=True) | ||
| class PreparedNoteSnapshot: | ||
| markdown: str | ||
| search_text: str | ||
| observations: tuple[str, ...] | ||
| relations: tuple[str, ...] | ||
|
|
||
|
|
||
| class AcceptedNoteRepositories(Protocol): | ||
| async def accept_content( | ||
| self, | ||
| session: object, | ||
| entity: Entity, | ||
| markdown: str, | ||
| ) -> None: ... | ||
|
|
||
| async def refresh_search( | ||
| self, | ||
| session: object, | ||
| entity: Entity, | ||
| search_text: str, | ||
| ) -> None: ... | ||
|
|
||
| async def replace_observations( | ||
| self, | ||
| session: object, | ||
| entity: Entity, | ||
| observations: tuple[str, ...], | ||
| ) -> None: ... | ||
|
|
||
| async def replace_relations( | ||
| self, | ||
| session: object, | ||
| entity: Entity, | ||
| relations: tuple[str, ...], | ||
| ) -> None: ... | ||
|
|
||
|
|
||
| async def persist_accepted_note_write( | ||
| session: object, | ||
| *, | ||
| entity: Entity, | ||
| prepared: PreparedNoteSnapshot, | ||
| repositories: AcceptedNoteRepositories, | ||
| ) -> None: | ||
| """Persist accepted content and hot search state in the caller's transaction.""" | ||
| await repositories.accept_content(session, entity, prepared.markdown) | ||
| await repositories.refresh_search(session, entity, prepared.search_text) | ||
|
|
||
|
|
||
| async def replace_accepted_note_graph( | ||
| session: object, | ||
| *, | ||
| entity: Entity, | ||
| prepared: PreparedNoteSnapshot, | ||
| repositories: AcceptedNoteRepositories, | ||
| ) -> None: | ||
| """Replace observations and outgoing relations for parsed accepted Markdown.""" | ||
| await repositories.replace_observations(session, entity, prepared.observations) | ||
| await repositories.replace_relations(session, entity, prepared.relations) | ||
|
|
||
|
|
||
| async def create_note( | ||
| session: object, | ||
| *, | ||
| entity: Entity, | ||
| prepared: PreparedNoteSnapshot, | ||
| repositories: AcceptedNoteRepositories, | ||
| ) -> None: | ||
| await persist_accepted_note_write( | ||
| session, | ||
| entity=entity, | ||
| prepared=prepared, | ||
| repositories=repositories, | ||
| ) | ||
| await replace_accepted_note_graph( | ||
| session, | ||
| entity=entity, | ||
| prepared=prepared, | ||
| repositories=repositories, | ||
| ) | ||
|
|
||
|
|
||
| async def edit_note( | ||
| session: object, | ||
| *, | ||
| entity: Entity, | ||
| prepared: PreparedNoteSnapshot, | ||
| repositories: AcceptedNoteRepositories, | ||
| ) -> None: | ||
| await persist_accepted_note_write( | ||
| session, | ||
| entity=entity, | ||
| prepared=prepared, | ||
| repositories=repositories, | ||
| ) | ||
| await replace_accepted_note_graph( | ||
| session, | ||
| entity=entity, | ||
| prepared=prepared, | ||
| repositories=repositories, | ||
| ) | ||
|
|
||
|
|
||
| async def move_note( | ||
| session: object, | ||
| *, | ||
| entity: Entity, | ||
| prepared: PreparedNoteSnapshot, | ||
| repositories: AcceptedNoteRepositories, | ||
| ) -> None: | ||
| """A move changes content/search paths but preserves the parsed note graph.""" | ||
| await persist_accepted_note_write( | ||
| session, | ||
| entity=entity, | ||
| prepared=prepared, | ||
| repositories=repositories, | ||
| ) |
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.