Skip to content

Integrate properties for tasks#859

Merged
ehayes2000 merged 7 commits intomainfrom
integrate-properties
Jan 9, 2026
Merged

Integrate properties for tasks#859
ehayes2000 merged 7 commits intomainfrom
integrate-properties

Conversation

@ehayes2000
Copy link
Copy Markdown
Contributor

Add property fetching with documents

@ehayes2000 ehayes2000 requested a review from a team as a code owner January 8, 2026 19:21
@ehayes2000 ehayes2000 requested a review from a team as a code owner January 8, 2026 19:34
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 8, 2026

if refs.is_empty() {
None
} else {
let ids: Vec<String> = refs.iter().map(|r| r.entity_id.clone()).collect();
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.

do you want to resolve entity_type + entity_id to an actual name?
in properties context entity_type = task -> document + subtype task. so if AI makes more tool calls i can see it making a mistake here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Will revisit

.content
.properties
.clone()
.map(|p| Properties::from_properties(EntityType::Task, p));
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.

is this hardcoding intentional?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The endpoint returns nothing if it's not a task which formats correctly. We only care about properties for tasks for now.

properties_db_client::entity_properties::get::get_entity_properties_values(
&self.macro_db,
&self.document_id,
models_properties::EntityType::Task,
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.

same here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

also yes

Base automatically changed from attachment-refactor to main January 8, 2026 22:52
@ehayes2000 ehayes2000 force-pushed the integrate-properties branch 2 times, most recently from b23522f to 8aea631 Compare January 9, 2026 19:43
@ehayes2000 ehayes2000 force-pushed the integrate-properties branch from 8aea631 to b1a4f5f Compare January 9, 2026 23:02
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Jan 9, 2026

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

@ehayes2000 ehayes2000 merged commit 9d96662 into main Jan 9, 2026
40 checks passed
@ehayes2000 ehayes2000 deleted the integrate-properties branch January 9, 2026 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants