Skip to content
This repository was archived by the owner on Feb 4, 2026. It is now read-only.

Implement tag re-evaluation when files are moved between folders#133

Merged
ploiu merged 10 commits intotag_flatteningfrom
copilot/update-file-parent-folder-tags
Nov 24, 2025
Merged

Implement tag re-evaluation when files are moved between folders#133
ploiu merged 10 commits intotag_flatteningfrom
copilot/update-file-parent-folder-tags

Conversation

Copy link
Contributor

Copilot AI commented Nov 24, 2025

Plan: Implement File Move Tag Re-evaluation ✅

This PR implements proper tag re-evaluation when a file is moved to a new parent folder, ensuring implicit tags from old ancestors are removed and new ancestor tags are properly implied.

All Changes Complete ✅

  • Create SQL query file: assets/queries/file/get_all_ancestors.sql
  • Add file_repository::get_all_ancestors function with tests
  • Add tags::repository::add_implicit_tags_to_files function (refactored to follow batch_remove_implicit_tags pattern)
  • Add file_service::remove_all_stale_ancestor_tags function
  • Add tags::service::imply_all_ancestor_tags function
  • Update file_service::update_file function
  • Update tags::service::update_file_tags function and documentation
  • Add comprehensive test coverage
  • Address all code review feedback

Latest Review Feedback Addressed

  • Refactored add_implicit_tags_to_files to follow batch_remove_implicit_tags pattern:
    • Early return when file_ids or tag_ids is empty
    • Chunking for both file_ids and tag_ids (999 per chunk)
    • Batch INSERT using VALUES clause instead of individual executes
  • Updated tests to match batch_remove_implicit_tags style:
    • Renamed works_with_empty_file_slicedoes_nothing_when_file_ids_is_empty
    • Renamed works_with_empty_tag_slicedoes_nothing_when_tag_ids_is_empty
    • Tests now verify nothing was touched in the db for empty cases
    • Tests for many IDs create 1001 entries to verify chunking works

Test Results:

  • 17/17 update_file_tests passing ✅
  • 5/5 imply_all_ancestor_tags tests passing ✅
  • 6/6 add_implicit_tags_to_files tests passing ✅
  • 5/5 get_all_ancestors tests passing ✅
Original prompt

This section details on the original issue you should resolve

<issue_title>file is edited</issue_title>
<issue_description># When a file is moved to a new parent folder

Step 1: Before the file is saved to the database, after initial validation:

in file_service::update_file, a file needs to have its implicated tags re-evaluated if the new parent id is different from the original parent id.

  • get all ancestors of the original file
    • this should be done via a new function named file_repository::get_all_ancestors(file_id: u32) -> Result<Vec<u32>, rusqlite::Error>>. Follow behavior of folder_repository::get_ancestor_folders_with_id (depth-first return order using a recursive cte). create a new sql file in assets/queries/file/get_all_ancestors.sql that this function uses
    • create tests similar to the tests implemented for folder_repository::get_ancestor_folders_with_id, but adapted to work for 1 specific file being used to retrieve ancestors
  • call tag_repository::batch_remove_implicit_tags for all old ancestor ids and the file's id

Step 2: After the file is saved to the database:

  • similarly to tag_service::pass_tags_to_children, add a new function named tags::service::implicate_tags_from_parents that
    • retrieves all ancestors using file_repository::get_all_ancestors that you created above
    • retrieves all explicit tags for those ancestors using tags::repository::get_tags_for_folder with TagTypes::Explicit passed
    • implies those tags to the file:
      • add a new function named tags::repository::add_implicit_tags_to_files(file_ids: &[u32], tag_ids: &[u32], implicit_from_id: u32, con: &Connection) -> Result<(), rusqlite::Error>
        • should function similarly to tags::repository::batch_remove_implicit_tags, but only for files and only for 1 implicit_from_id
        • should use insert or ignore to simplify logic and to ensure that the file keeps the tags from the closest ancestor
    • since retrieved ancestors from the database should be in depth-first order, iterate through the new ancestors and call tags::repository::add_implicit_tags_to_files for the file and the current iteration's ancestor's tags.

save Step 1 in a new function called file_service::remove_all_stale_ancestor_tags
save Step 2 in a new function named tags::service::imply_all_ancestor_tags, that is called in tags::service::update_file_tags. Update documentation of update_file_tags to include the file recalculating implied tags behavior.

Tests

File is moved (file_service tests)

  • should lose all implied tags from old ancestors
  • should keep all explicit tags
  • new ancestors should not override explicit tags
  • should implicate all explicit tags of new ancestors

File's explicit tags change (tags::tests::service.rs)

  • should reject change if tag is already implied on file
  • should reject change if tag is already explicitly on file

tags::service::implicate_tags_from_parents

  • should properly imply tags in depth-first order
  • should not override explicit tags

file_repository::get_all_ancestors

  • should return an empty vec if 0 is passed
  • should return ancestors in depth-first order (closest to the file first, furthest last)
  • should return an empty vec if no file is found

tags::repository::add_implicit_tags_to_files

  • similar tests to batch_remove_implicit_tags; should work, does not add stuff not specified, should handle large numbers of files and tags, etc</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 5 commits November 24, 2025 19:17
Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com>
Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com>
Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com>
Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com>
Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com>
Copilot AI changed the title [WIP] Update file service to re-evaluate tags on parent folder change Implement tag re-evaluation when files are moved between folders Nov 24, 2025
Copilot AI requested a review from ploiu November 24, 2025 19:37
Copy link
Owner

@ploiu ploiu left a comment

Choose a reason for hiding this comment

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

@copilot you didn't follow the original prompt all the way, some comments are not needed, and some tests need to change

… add_implicit_tags_to_files function

Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 24, 2025

@copilot you didn't follow the original prompt all the way, some comments are not needed, and some tests need to change

Addressed all feedback in 65d7ca5: removed unnecessary comments, reused database connection, moved documentation to repository layer, created the add_implicit_tags_to_files function for batch operations, and renamed the test.

Copilot AI requested a review from ploiu November 24, 2025 20:33
Copy link
Owner

@ploiu ploiu left a comment

Choose a reason for hiding this comment

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

@copilot

}

#[test]
fn works_with_empty_tag_slice() {
Copy link
Owner

Choose a reason for hiding this comment

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

Rename this to detail that no database interactions should happen if tags is empty

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed to does_nothing_when_tag_ids_is_empty in cc76806.

}

#[test]
fn works_with_empty_file_slice() {
Copy link
Owner

Choose a reason for hiding this comment

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

Update the title to detail no database interactions should happen if files is empty

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed to does_nothing_when_file_ids_is_empty in cc76806.

…ags pattern

Co-authored-by: ploiu <43047560+ploiu@users.noreply.github.com>
Copilot AI requested a review from ploiu November 24, 2025 21:11
@ploiu ploiu marked this pull request as ready for review November 24, 2025 22:26
@ploiu ploiu merged commit bb98c8c into tag_flattening Nov 24, 2025
@ploiu ploiu deleted the copilot/update-file-parent-folder-tags branch November 24, 2025 22:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants