Fix: Prevent duplicate buffers in join operations

[kishore08-07]

Bug fix

Fixed bug

• This PR fixes #61:Previously, specifying the same buffer multiple times in a single join operation was allowed at build time, but caused a confusing runtime error (Broken) during workflow execution.

Fix applied

• Added a DuplicateBuffer error type and a safe_join method to check for duplicate buffers at join construction time.

• The safe_join method returns a clear error if duplicates are found, preventing the workflow from being built incorrectly.

• The existing join method now calls safe_join and panics with a clear message if duplicates are present, preserving the previous panic-on-error API.

• Added corresponding methods to Joinable, IterBufferable, and Builder for ergonomic usage.

• Added tests to ensure duplicate buffers are caught at construction time, both for the error case safe_join and the panic case join.

• This change ensures that duplicate buffer errors are detected early, providing a better user experience and preventing confusing runtime failures.

GenAI Use

We follow OSRA's policy on GenAI tools

• I used a GenAI tool in this PR.
• I did not use GenAI

Generated-by:
GPT-4.1

[Copilot]

Pull request overview

Adds an upfront validation step to join construction so specifying the same buffer multiple times is detected early (instead of failing later at runtime), addressing issue #61.

Changes:

• Introduces DuplicateBuffer and a safe_join path that returns a Result when duplicates are detected.
• Updates existing join/join_vec APIs to route through safe_join and keep the existing panic-on-error behavior.
• Adds regression tests covering both the safe_join error case and the join panic case.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
src/buffer/buffering.rs	Defines DuplicateBuffer, adds duplicate detection, and implements Joining::safe_join used by join construction.
src/buffer/bufferable.rs	Exposes safe_join/safe_join_vec on Joinable and IterBufferable, and routes existing join helpers through them.
src/builder.rs	Adds Builder::safe_join convenience wrapper returning Result<Chain, DuplicateBuffer>.
src/operation/join.rs	Adds tests ensuring duplicates are rejected via safe_join and cause a panic via join.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[mxgrey]

Thanks for the contribution! This is a good match of what I recommended in the original issue ticket. I left a few comments about adding missing inline documentation.

One additional point we should address is fn try_join. This method supports dynamic joining, needed for JSON diagram workflows. It already returns an error message if the input layout is incompatible with the type that is being joined into, but currently there is no check to prevent multiple buffers from being used.

We should change the error type of try_join to an enum with variants of IncompatibleLayout and DuplicateBuffer, and we should look through the BufferMap for buffers with duplicate IDs. If there are any duplicate IDs then try_join should return the DuplicateBuffer variant.

[kishore08-07]

@mxgrey Thank you for the detailed feedback!

• Added inline documentation to safe_join and safe_join_vec as suggested, clarifying their behavior and how they differ from the panicking variants.
• Updated try_join to use an enum error type (TryJoinError) with IncompatibleLayout and DuplicateBuffer variants, and added duplicate buffer detection for dynamic joins.

All review points are now addressed, kindly review when you have time and let me know if any further changes are required.

[mxgrey]

@kishore08-07 note that the CI tests are failing. When validating your changes be sure to run each of the following:

cargo test
cargo test -F diagram
cargo fmt

It's important to run cargo test and cargo test -F diagram separately since it's possible to encounter errors in one and not the other. Then cargo fmt ensures that the style is consistent.

[kishore08-07]

@kishore08-07 note that the CI tests are failing. When validating your changes be sure to run each of the following:

cargo test
cargo test -F diagram
cargo fmt

It's important to run cargo test and cargo test -F diagram separately since it's possible to encounter errors in one and not the other. Then cargo fmt ensures that the style is consistent.

@mxgrey I've run the above test commands as suggested. CI errors are resolved and all tests are now passing locally. Changes have been committed.
Thank you for the guidance!