chore: remove convention dead code - #207
Merged
Merged
Conversation
|
Contributor
Greptile SummaryRemoves unreachable Convention-related code and its obsolete tests.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| server/osa/domain/deposition/port/convention_repository.py | Removes the unused exists method from the repository protocol; the previously reported test callers have also been removed. |
| server/osa/infrastructure/persistence/repository/convention.py | Removes the corresponding unused PostgreSQL repository implementation consistently with the port. |
| server/tests/integration/persistence/test_convention_repo.py | Removes both obsolete exists tests, resolving the previous thread's test failure. |
| server/osa/domain/deposition/service/convention.py | Removes the unread node_domain service dependency. |
| server/osa/domain/deposition/util/di/provider.py | Updates dependency injection consistently with the simplified convention service. |
| server/osa/infrastructure/persistence/adapter/storage.py | Deletes unused convention-keyed filesystem path helpers. |
| server/osa/infrastructure/s3/storage.py | Deletes the equivalent unused convention-keyed S3 path helpers. |
| server/osa/domain/shared/model/srn.py | Removes the unused Convention resource enum member. |
| server/uv.lock | Synchronizes the editable OSA package version without changing resolved dependencies. |
Reviews (2): Last reviewed commit: "chore: remove convention dead code" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
20
to
23
| self, *, limit: int | None = None, offset: int | None = None | ||
| ) -> "List[Convention]": ... | ||
|
|
||
| @abstractmethod | ||
| async def exists(self, id: ConventionSlug) -> bool: ... | ||
|
|
||
| @abstractmethod |
Contributor
There was a problem hiding this comment.
When the convention repository integration tests run, they call repo.exists(...) after this method and its PostgreSQL implementation have been removed, causing both assertions to fail with AttributeError.
Contributor
Author
There was a problem hiding this comment.
False positive — this PR's own diff removes those tests: test_exists_true and test_exists_false are deleted in the server/tests/integration/persistence/test_convention_repo.py hunk of this same PR (the two assertions this comment refers to). Verified on the branch head: git grep '\.exists(' over server/ finds no remaining caller of ConventionRepository.exists in source or tests — the only repository-exists caller left is ontology_repo.exists (osa/domain/semantics/service/schema.py:37), a different port this PR doesn't touch. No change needed.
Scaffolding that no code path reaches, removed ahead of the Convention removal itself (#180) so the real change is smaller: - `get_source_staging_dir` / `get_source_output_dir` / `_conv_id` in both storage adapters — the only convention-keyed storage paths, with zero callers. The live ingest layout is keyed on `ingest_run_id`. - `FeatureTable` value object — exported, never constructed. - `ConventionRepository.exists()` — no call sites; `DepositionService.create` null-checks `get()` instead. - `ConventionService.node_domain` — injected, never read. - `ResourceType.conv` — never constructed. `uv.lock` picks up the stale version sync the pre-commit hook regenerates. No behaviour change.
rorybyrne
force-pushed
the
chore/remove-convention-dead-code
branch
from
August 3, 2026 16:58
d0a651f to
075d526
Compare
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
1 participant
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.
First of three PRs implementing #180. Pure deletion — no behaviour change — so the
Convention removal itself lands as a smaller, more reviewable diff.
Everything here was verified unreachable before deleting:
get_source_staging_dir/get_source_output_dir/_conv_id(both storage adapters)osa/ortests/. Not on any port. The live ingest layout is keyed oningest_run_id(infrastructure/storage/layout.py).FeatureTablevalue objectdomain/feature/model/__init__.py, never constructed —grep "FeatureTable("matches only the class definition.ConventionRepository.exists()+ implDepositionService.createnull-checksget()instead.ConventionService.node_domainResourceType.convBecause none of the storage paths were ever written to, no blob migration is
required when Convention goes.
Notes
uv.lockpicks up a stale version sync (0.0.7→0.0.11). Not incidental —the
typre-commit hook regenerates it and aborts the commit otherwise.WorkerPoolcron scaffolding (_build_schedules_from_conventions, a stubreturning
[]) is deliberately left in place. It's dead today, but PR 2rewires it to the new ingester registry rather than deleting and re-adding it.
Verification
ruff checkandty check osaclean.Refs #180