Skip to content

feat: tags#210

Merged
InftyAI-Agent merged 7 commits intoInftyAI:mainfrom
kerry-he:feature/tags
Mar 12, 2026
Merged

feat: tags#210
InftyAI-Agent merged 7 commits intoInftyAI:mainfrom
kerry-he:feature/tags

Conversation

@kerry-he
Copy link
Contributor

@kerry-he kerry-he commented Mar 11, 2026 •
edited
Loading

What this PR does / why we need it

Allows tags to be specified as a list of strings, e.g.,

alphatrion.experiment.craft_experiment.CraftExperiment.start(name="experiment", tags=["foo", "bar"])

I will add a separate PR to account for this in the dashboard UI.

Which issue(s) this PR fixes

Fixes #209

Special notes for your reviewer

Does this PR introduce a user-facing change?

Experiments can now be tagged with lists of strings.

@InftyAI-Agent InftyAI-Agent added needs-triage Indicates an issue or PR lacks a label and requires one. needs-priority Indicates a PR lacks a label and requires one. do-not-merge/needs-kind Indicates a PR lacks a label and requires one. labels Mar 11, 2026
@InftyAI-Agent InftyAI-Agent requested a review from kerthcet March 11, 2026 06:14
@kerthcet
Copy link
Member

kerthcet commented Mar 11, 2026

Thanks @kerry-he quoted the offline discussion here for transparency:

maybe let's have a separate tags table, because I don't know how labels will evolve in the future, 
it could be immutable, it could be separated to different kinds of labels, system labels and user labels 
to avoid abuse, since we're developing from scratch, and have a dedicated tags table is not a big deal, 
maybe let's start with a new table, it won't be a mistake decision in the future anyway. 
We'll make lables immutable and tags mutable today.WDYT?

@kerry-he
Copy link
Contributor Author

kerry-he commented Mar 12, 2026

/kind feature

@InftyAI-Agent InftyAI-Agent added feature Categorizes issue or PR as related to a new feature. and removed do-not-merge/needs-kind Indicates a PR lacks a label and requires one. labels Mar 12, 2026
@kerry-he
Copy link
Contributor Author

kerry-he commented Mar 12, 2026

Fixed, tags are now a separarate field and uses its own table.

@kerry-he kerry-he changed the title FEAT: tags feat: tags Mar 12, 2026
@kerry-he kerry-he mentioned this pull request Mar 12, 2026
Merged
kerthcet
kerthcet reviewed Mar 12, 2026
View reviewed changes
Copy link
Member

@kerthcet kerthcet left a comment

Choose a reason for hiding this comment

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

general LGTM, thanks @kerry-he

alphatrion/storage/sql_models.py
experiment_id = Column(UUID(as_uuid=True), nullable=False)
tag = Column(String, nullable=False)

created_at = Column(DateTime(timezone=True), default=lambda: datetime.now(UTC))
Copy link
Member

@kerthcet kerthcet Mar 12, 2026

Choose a reason for hiding this comment

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

Can we add the updated_at for consistency

alphatrion/storage/sqlstore.py Outdated
exp_tag = ExperimentTag(
team_id=team_id,
experiment_id=uid,
tag=tag.strip(),
Copy link
Member

@kerthcet kerthcet Mar 12, 2026

Choose a reason for hiding this comment

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

why stripe twice?

alphatrion/storage/sqlstore.py Outdated
.join(ExperimentTag, ExperimentTag.experiment_id == Experiment.uuid)
.filter(
Experiment.team_id == team_id,
Experiment.is_del == 0,
Copy link
Member

@kerthcet kerthcet Mar 12, 2026

Choose a reason for hiding this comment

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

move the is_del==0 under tag, or index team_id & tag will not work

tests/unit/experiment/test_experiment.py
) as exp:
exp_obj = exp._get_obj()
assert exp_obj is not None

Copy link
Member

@kerthcet kerthcet Mar 12, 2026

Choose a reason for hiding this comment

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

Can you add one more assert that there're two records in the experiment_tags table.

alphatrion/server/graphql/resolvers.py Outdated
) -> list[Experiment]:
metadb = runtime.storage_runtime().metadb
if label_name:
if tag:
Copy link
Member

@kerthcet kerthcet Mar 12, 2026

Choose a reason for hiding this comment

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

One potential problem here is what if label name and tag are both selected. We should either clear another one in the frontend or support filtering the database with both two. But not a big concern.

@kerthcet
Copy link
Member

kerthcet commented Mar 12, 2026

You can use make format to format all the codes in case of lint validation failure.

@kerthcet
Copy link
Member

kerthcet commented Mar 12, 2026

I guess you still need to run alembic revision --autogenerate -m "add new table tags" to generate the migration script. @kerry-he

Then we're ready to merge

@kerthcet
Copy link
Member

kerthcet commented Mar 12, 2026

/lgtm
/approve

Thanks @kerry-he

@InftyAI-Agent InftyAI-Agent added lgtm Looks good to me, indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 12, 2026
@InftyAI-Agent InftyAI-Agent merged commit 7c04e31 into InftyAI:main Mar 12, 2026
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kerthcet kerthcet kerthcet left review comments

Assignees

@kerthcet kerthcet

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. feature Categorizes issue or PR as related to a new feature. lgtm Looks good to me, indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a label and requires one. needs-triage Indicates an issue or PR lacks a label and requires one.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add tags to experiments

4 participants

@kerry-he @kerthcet @InftyAI-Agent @kerry-hiverge