Skip to content

Various fixes and cleanups#558

Merged
chrisaddy merged 1 commit intomasterfrom
05-22-various_fixes_and_cleanups
May 23, 2025
Merged

Various fixes and cleanups#558
chrisaddy merged 1 commit intomasterfrom
05-22-various_fixes_and_cleanups

Conversation

@forstmeier
Copy link
Copy Markdown
Collaborator

@forstmeier forstmeier commented May 23, 2025

Overview

Changes

  • unify Mise commands into single file
  • add build step to Python unit tests
  • various cleanup and tweaks

Comments

Some of this was necessary (e.g. test fixes) but others were just cleanup/preference tweaks so let me know what you think @chrisaddy.

Summary by CodeRabbit

  • Chores
    • Updated workflow configurations to standardize on Ubuntu runners and simplify job strategies.
    • Refined task runner commands and reorganized task naming for improved clarity and test management.
    • Removed unused dependencies from some service configurations and added development dependencies for testing and monitoring.
    • Updated compose files to include descriptive metadata.
    • Improved documentation formatting and updated issue templates to remove the "backlog" label.
    • Enhanced project metadata with new descriptions and clarified workflow instructions.

@graphite-app graphite-app Bot requested a review from chrisaddy May 23, 2025 02:09
@forstmeier forstmeier requested review from Copilot and removed request for chrisaddy May 23, 2025 02:09
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 23, 2025

Walkthrough

This update revises configuration and metadata across several files, including workflow runners, issue templates, dependency specifications, and task definitions. Notable changes include switching GitHub Actions runners to Ubuntu, updating Python task scripts, modifying dependency lists, and reorganizing or removing labels and tasks for improved clarity and consistency.

Changes

File(s) Change Summary
.github/ISSUE_TEMPLATE/BUG.md
.github/ISSUE_TEMPLATE/FEATURE.md
Removed the "backlog" label from default labels in bug and feature issue templates; now only "bug" or "feature" labels are applied by default.
.github/workflows/deploy.yaml
.github/workflows/quality.yaml
Changed GitHub Actions runners from macos-latest to ubuntu-latest and removed the OS matrix strategy, simplifying the workflow to run only on Ubuntu. Updated job names and runner fields accordingly.
.flox/env/manifest.toml Removed a commented-out reference to libpqxx.pkg-path.
.mise.toml Updated Python install and test tasks, renamed and reorganized application service run/test tasks, and added new behavioral test and cleanup tasks. Changed dependency sync command and test execution steps.
AGENTS.md Added blank lines for improved section separation; no content changes.
CLAUDE.md Updated instructions to reference "To Do" project status and clarified issue workflow steps.
application/datamanager/compose.yaml
compose.yaml
Added or modified top-level name fields for metadata; introduced YAML document start marker in compose.yaml.
application/datamanager/features/environment.py
application/datamanager/features/steps/health_steps.py
Removed single-line docstrings from functions; no code logic changes.
application/datamanager/mise.toml Deleted file containing Docker Compose tasks for behavioral tests and cleanup.
application/datamanager/pyproject.toml
application/positionmanager/pyproject.toml
Removed loguru and/or prometheus-fastapi-instrumentator dependencies from project dependency lists.
application/datamanager/src/datamanager/main.py Removed extraneous blank lines; no functional code changes.
pyproject.toml Added behave, prometheus-fastapi-instrumentator, and loguru to the [dev] dependency group.
workflows/pyproject.toml Added a description field to project metadata.

Sequence Diagram(s)

sequenceDiagram
    participant Developer
    participant GitHub Actions
    participant Ubuntu Runner

    Developer->>GitHub Actions: Push code / open PR
    GitHub Actions->>Ubuntu Runner: Run quality/deploy workflow
    Ubuntu Runner-->>GitHub Actions: Execute steps (tests, deploy, etc.)
    GitHub Actions-->>Developer: Report workflow results
Loading

Possibly related PRs

Suggested labels

application

Suggested reviewers

  • chrisaddy

Poem

In the warren of code, the rabbits convene,
Tidying labels and workflows—so neat and so clean!
With Ubuntu they leap, leaving Mac in the dust,
Dependencies shuffled, as all bunnies must.
Tasks renamed, tests aligned,
In this patch, harmony’s signed!
🐇✨

Note

⚡️ AI Code Reviews for VS Code, Cursor, Windsurf

CodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback.
Learn more here.

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@forstmeier forstmeier requested a review from chrisaddy May 23, 2025 02:09
Copy link
Copy Markdown
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@forstmeier forstmeier added this to the Rebuild milestone May 23, 2025
@graphite-app
Copy link
Copy Markdown

graphite-app Bot commented May 23, 2025

Graphite Automations

"Assign author to pull request" took an action on this PR • (05/23/25)

1 assignee was added to this PR based on John Forstmeier's automation.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

A set of miscellaneous cleanups and configuration updates across project metadata, dependency lists, CI/mise tasks, and documentation.

  • Added descriptive metadata and standardized dependency declarations.
  • Refactored mise tasks and CI workflows, including test runners and OS targets.
  • Cleaned up obsolete configs, adjusted issue templates, and minor documentation fixes.

Reviewed Changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.

Show a summary per file
File Description
workflows/pyproject.toml Added description metadata to project
pyproject.toml Added behave, instrumentation, and logging dev deps
compose.yaml Introduced YAML header and test suite name
application/datamanager/src/datamanager/main.py Adjusted whitespace around instrumentation setup
application/datamanager/pyproject.toml Removed duplicated logging and instrumentation deps
application/datamanager/mise.toml Removed Docker-based behave tasks
application/datamanager/features/steps/*.py Dropped docstrings from step implementations
application/datamanager/features/environment.py Dropped docstring from before_all hook
application/datamanager/compose.yaml Renamed integration tests service title
CLAUDE.md Updated issue creation section with project status
AGENTS.md Minor formatting cleanup in code style and PR docs
.mise.toml Renamed and extended tasks; updated test commands
.github/workflows/quality.yaml Simplified runner OS for Python quality checks
.github/workflows/deploy.yaml Changed deploy runner from macOS to Ubuntu
.github/ISSUE_TEMPLATE/FEATURE.md Removed backlog label
.github/ISSUE_TEMPLATE/BUG.md Removed backlog label
.flox/env/manifest.toml Commented out unused libpqxx pkg-path
Comments suppressed due to low confidence (8)

application/datamanager/mise.toml:1

  • Removing the Docker-based behave tasks may leave behavioral tests unexecuted; consider adding equivalent CI or mise tasks to maintain coverage.
-[tasks."test:docker:behave"]

application/datamanager/features/steps/health_steps.py:6

  • [nitpick] Docstring removed from step_impl; consider retaining it or updating with a clear description to aid readability of behave steps.
-    """Send a GET request to the specified endpoint."""

application/datamanager/features/environment.py:4

  • [nitpick] Docstring removed from before_all; adding a brief description helps future maintainers understand test setup logic.
-    """Set up test environment."""

application/datamanager/compose.yaml:1

  • [nitpick] Inconsistent capitalization: consider renaming to "Data Manager Integration Tests" to match Title Case style used elsewhere.
name: Data manager integration tests

.github/workflows/quality.yaml:8

  • Limiting quality checks to Ubuntu may miss platform-specific issues; consider restoring a matrix of OS runners.
runs-on: ubuntu-latest

.github/workflows/deploy.yaml:10

  • [nitpick] Switched deployment runner from macOS to Ubuntu; verify that no macOS-specific steps or tools are required by the deployment process.
runs-on: ubuntu-latest

.github/ISSUE_TEMPLATE/FEATURE.md:5

  • Removed the backlog label; ensure any automation or filters that rely on that label are updated accordingly.
labels: ["feature"]

.github/ISSUE_TEMPLATE/BUG.md:5

  • Removed the backlog label; verify that workflows or queries referencing it are adjusted to prevent missing issues.
labels: ["bug"]

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c8bebfd and 88ba857.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (18)
  • .flox/env/manifest.toml (0 hunks)
  • .github/ISSUE_TEMPLATE/BUG.md (1 hunks)
  • .github/ISSUE_TEMPLATE/FEATURE.md (1 hunks)
  • .github/workflows/deploy.yaml (1 hunks)
  • .github/workflows/quality.yaml (1 hunks)
  • .mise.toml (3 hunks)
  • AGENTS.md (1 hunks)
  • CLAUDE.md (2 hunks)
  • application/datamanager/compose.yaml (1 hunks)
  • application/datamanager/features/environment.py (0 hunks)
  • application/datamanager/features/steps/health_steps.py (0 hunks)
  • application/datamanager/mise.toml (0 hunks)
  • application/datamanager/pyproject.toml (0 hunks)
  • application/datamanager/src/datamanager/main.py (1 hunks)
  • application/positionmanager/pyproject.toml (0 hunks)
  • compose.yaml (1 hunks)
  • pyproject.toml (1 hunks)
  • workflows/pyproject.toml (1 hunks)
💤 Files with no reviewable changes (6)
  • .flox/env/manifest.toml
  • application/datamanager/features/environment.py
  • application/datamanager/pyproject.toml
  • application/positionmanager/pyproject.toml
  • application/datamanager/features/steps/health_steps.py
  • application/datamanager/mise.toml
🔇 Additional comments (15)
.github/ISSUE_TEMPLATE/FEATURE.md (1)

5-5: Streamline default labels for feature requests
Removed the "backlog" label so that new feature issues are now tagged only with "feature", aligning with the streamlined labeling strategy.

AGENTS.md (1)

11-11: Approve formatting improvements for readability
Added blank lines before each section heading to enhance visual separation and improve the document’s scannability.

Also applies to: 15-15, 19-19

.github/ISSUE_TEMPLATE/BUG.md (1)

5-5: Streamline default labels for bug reports
Removed the "backlog" label so that new bug issues are now tagged only with "bug", matching the update made for feature requests.

CLAUDE.md (1)

18-18: Standardize GitHub Project status terminology
Updated the status label to "To Do" in both the issue creation and workflow sections for consistent capitalization and clarity across documentation.

Also applies to: 30-31

.github/workflows/deploy.yaml (1)

10-10: Standardize runner to ubuntu-latest
Aligns the deployment workflow with the rest of the CI by using ubuntu-latest instead of macOS. This simplifies maintenance and ensures consistency across workflows.

workflows/pyproject.toml (1)

3-3: Add project description metadata
Including a description field improves clarity for package consumers and aligns with other modules’ metadata standards.

.github/workflows/quality.yaml (1)

7-8: Simplify matrix and standardize runner
Removed the multi-OS matrix and hardcoded runs-on: ubuntu-latest, aligning with the deploy workflow for a unified CI environment.

application/datamanager/compose.yaml (1)

1-1: Rename integration tests service for clarity
Updated the top-level name to "Data manager integration tests". Please verify that related CI tasks, Compose references in .mise.toml, and documentation are updated to match this new name.

application/datamanager/src/datamanager/main.py (1)

68-68: Whitespace-only change
Removed an extraneous blank line; no functional impact.

compose.yaml (1)

1-2: Add YAML document start and name metadata
Introducing --- and a name field clarifies the Compose project configuration and aligns with the new structured testing workflows.

pyproject.toml (1)

42-44: Centralize new development dependencies
Adding behave, prometheus-fastapi-instrumentator, and loguru to the root dev group correctly consolidates behavioral testing and instrumentation tools. Ensure these are removed from subpackage configurations to avoid duplication.

.mise.toml (4)

6-6: Verify the Python sync command scope
Changing from uv sync --all-groups to uv sync --all-packages may alter which dependency groups get installed. Confirm that development and testing dependencies are still included when running this task.


33-36: Split build and run for tests
The new two-step docker compose build tests and docker compose run -T tests approach improves caching and clarity.


38-45: Explicit production run task
The renamed application:service:run:production task makes the environment clear. Validate that {{arg(name="service_name")}} resolves to a valid service and that the image naming convention (pocketsizefund/<service_name>:latest) matches your CI/CD pipeline.


47-53: Explicit development run task
The application:service:run:development task with hot reload is a great addition. Ensure that the working directory and module path (src.<service_name>.main:application) matches your project layout.

Comment thread .mise.toml
Comment on lines +54 to 59
[tasks."application:service:test:integration"]
description = "Run integration tests"
run = """
cd application/{{arg(name="service_name")}}
docker-compose up --build --abort-on-container-exit --remove-orphans
"""
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Unify Docker Compose CLI usage
The integration test task still uses docker-compose (hyphenated) while other tasks use docker compose. This inconsistency can cause failures if one CLI is unavailable.

Apply this diff to standardize on the Compose v2 CLI:

-[tasks."application:service:test:integration"]
-run = """
- cd application/{{arg(name="service_name")}}
- docker-compose up --build --abort-on-container-exit --remove-orphans
-"""
+[tasks."application:service:test:integration"]
+run = """
+cd application/{{arg(name="service_name")}}
+docker compose up --build --abort-on-container-exit --remove-orphans
+"""

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In .mise.toml around lines 54 to 59, the integration test task uses the old
hyphenated `docker-compose` command, which is inconsistent with other tasks
using the newer `docker compose` CLI. Update the command by replacing
`docker-compose` with `docker compose` to unify the Docker Compose CLI usage
across all tasks and avoid potential failures.

Comment thread .mise.toml
Comment on lines +61 to +67
[tasks."application:service:test:behavioral"]
description = "Run behavioral tests"
run = """
cd application/{{arg(name="service_name")}}
docker-compose up --build --abort-on-container-exit
"""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Standardize behavioral test invocation
The behavioral test task also uses docker-compose instead of docker compose. Aligning them improves reliability across environments.

-[tasks."application:service:test:behavioral"]
- description = "Run behavioral tests"
- run = """
- cd application/{{arg(name="service_name")}}
- docker-compose up --build --abort-on-container-exit
- """
+[tasks."application:service:test:behavioral"]
+description = "Run behavioral tests"
+run = """
+cd application/{{arg(name="service_name")}}
+docker compose up --build --abort-on-container-exit
+"""
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
[tasks."application:service:test:behavioral"]
description = "Run behavioral tests"
run = """
cd application/{{arg(name="service_name")}}
docker-compose up --build --abort-on-container-exit
"""
[tasks."application:service:test:behavioral"]
description = "Run behavioral tests"
run = """
cd application/{{arg(name="service_name")}}
docker compose up --build --abort-on-container-exit
"""
🤖 Prompt for AI Agents
In .mise.toml around lines 61 to 67, the behavioral test task uses the
deprecated `docker-compose` command. Replace `docker-compose` with the modern
`docker compose` syntax in the run script to standardize and improve
compatibility across environments.

Comment thread .mise.toml
Comment on lines +68 to +74
[tasks."application:service:cleanup:behavioral"]
description = "Clean up behavioral tests"
run = """
cd application/{{arg(name="service_name")}}
docker-compose down -v
"""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Standardize cleanup of behavioral tests
Similarly, the cleanup task should use docker compose down -v to match the Compose v2 CLI syntax.

-[tasks."application:service:cleanup:behavioral"]
- description = "Clean up behavioral tests"
- run = """
- cd application/{{arg(name="service_name")}}
- docker-compose down -v
- """
+[tasks."application:service:cleanup:behavioral"]
+description = "Clean up behavioral tests"
+run = """
+cd application/{{arg(name="service_name")}}
+docker compose down -v
+"""
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
[tasks."application:service:cleanup:behavioral"]
description = "Clean up behavioral tests"
run = """
cd application/{{arg(name="service_name")}}
docker-compose down -v
"""
[tasks."application:service:cleanup:behavioral"]
description = "Clean up behavioral tests"
run = """
cd application/{{arg(name="service_name")}}
docker compose down -v
"""
🤖 Prompt for AI Agents
In .mise.toml around lines 68 to 74, the cleanup task uses the old Docker
Compose CLI syntax `docker-compose down -v`. Update this command to use the
Compose v2 CLI syntax by changing it to `docker compose down -v` to standardize
the cleanup process.

@chrisaddy chrisaddy merged commit 56b85ab into master May 23, 2025
6 checks passed
@chrisaddy chrisaddy deleted the 05-22-various_fixes_and_cleanups branch May 23, 2025 03:08
@coderabbitai coderabbitai Bot mentioned this pull request May 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants