-
Notifications
You must be signed in to change notification settings - Fork 45
chore(docs): added AGENTS.MD #106
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
Conversation
Signed-off-by: Parthib Mukherjee <[email protected]>
kramaranya
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @hawkaii for working on this!
Could we provide more information in this doc to align with other examples?
For example, pipelines have a good content https://github.com/kubeflow/pipelines/blob/master/AGENTS.md
Also kubernetes https://github.com/kubernetes/kubernetes/blob/1f98145048835b8a9fd6ec9d769f57fc548c8304/AGENTS.md
I personally like what langchain has https://github.com/langchain-ai/langchain/blob/master/AGENTS.md
…and development principles Signed-off-by: Parthib Mukherjee <[email protected]>
Removed introductory line about AGENTS guide. Signed-off-by: Parthib Mukherjee <[email protected]>
Pull Request Test Coverage Report for Build 18630153766Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
andreyvelich
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @hawkaii!
Overall looks good, I left a few suggestions.
Maybe @franciscojavierarceo or @kubeflow/wg-pipeline-leads can provide additional suggestions based on their experience in Feast or KFP.
AGENTS.md
Outdated
| ## Project Overview | ||
|
|
||
| **Purpose**: Kubeflow SDK provides a unified Python SDK for AI practitioners to interact with multiple Kubeflow projects via consistent APIs, focusing on user workflows over infrastructure details. | ||
|
|
||
| **Problem It Solves**: Reduces Kubernetes and multi-project complexity, offering simple, local-first Python interfaces for training, tuning, pipelines (planned), and model lifecycle management. | ||
|
|
||
| **Key Benefits**: | ||
| - Unified experience across Kubeflow projects | ||
| - Simplified AI workflows with minimal infrastructure knowledge | ||
| - Local development support (install via `pip`) with optional cluster backends | ||
|
|
||
| **Today's Scope**: | ||
| - **Available**: Kubeflow Trainer (train/fine-tune with different backends) | ||
| - **Planned**: Katib (HPO), Pipelines (workflows), Model Registry | ||
| - See README "Supported Kubeflow Projects" for current status |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this help AI Agents to write/debug code or we can remove this to reduce context ?
| - **Available**: Kubeflow Trainer (train/fine-tune with different backends) | ||
| - **Planned**: Katib (HPO), Pipelines (workflows), Model Registry | ||
| - See README "Supported Kubeflow Projects" for current status | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be useful to introduce Context Awareness:
### Context Awareness
Before writing code, agents should:
- Read docstrings and existing test cases for pattern alignment
- Match import patterns from neighboring files
- Preserve existing logging and error-handling conventions
| - **Build**: Hatchling (optional `uv build`) | ||
| - **Pre-commit**: Config provided and enforced in CI | ||
|
|
||
| ## Quick Start |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering if it is better to wrap command to special tokens, like this ?
<!-- BEGIN: AGENT_COMMANDS -->
Preferred commands:
- Install deps: `make install-dev`
- Verify: `make verify`
- Test: `make test-python`
<!-- END: AGENT_COMMANDS -->
| - Use `pytest` as the testing framework | ||
| - See `kubeflow/trainer/test/common.py` for fixtures and patterns | ||
|
|
||
| **Test Quality Checklist:** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tell that unit tests structure must be consistent between each other. As example add: kubeflow/trainer/backends/kubernetes/backend_test.py
| def test_filter_completed_jobs(): | ||
| """Test filtering completed jobs from a list.""" | ||
| jobs = ["job-1", "job-2", "job-3"] | ||
| completed = {"job-1", "job-2"} | ||
|
|
||
| result = filter_completed_jobs(jobs, completed) | ||
|
|
||
| assert result == ["job-3"] | ||
| assert len(result) == 1 | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also show that tests cases must be set before the tests functions with @pytest.mark.parametrize like here:
| @pytest.mark.parametrize( |
…ment workflow Signed-off-by: Parthib Mukherjee <[email protected]>
|
@andreyvelich i just had claude generate it and i merged it. No discussion :) |
|
That was much faster for me. :) |
andreyvelich
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @hawkaii!
/lgtm
/assign @Electronic-Waste @kramaranya @szaher @astefanutti
|
We should be good to move this forward. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andreyvelich The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it:
adds
AGENTS.MDfile for AI-assisted developmentFixes #86
Checklist: