MVP Pipeline Implementation with Utils update

[Synicix]

Summary:
In short, I implemented from_dot to fit with the design of graph + kernel_lut where kernel_lut contains only the unique kernels to prevent duplicates. This will provided me with a minimum Pipeline for me to resume work on the pipeline runner. Also, it removes the need for pipeline_builder to be implemented in order to create test cases.

Added:

• MVP pipeline
• from_dot function that requires the graph and kernel_to_node_name HashMap (Reason behind this is I still want to keep kernel_lut to only have 1 copy of each kernel)
• find_missing_keys, for PipelineJob input checking
• Uniffi bindings

Updated:

• utils functions to more generic implementation

Deferred:

• to_dot (Was spending too much time trying to deal with error handling based on @guzman-raphael reference code, and it is not needed for runner)
• to_svg (Not needed for runner so deferring it)

For reference where these changes come from:

[Synicix]

@guzman-raphael The code base is ready for a rough review, it is still missing tests and the list of todos above, but I plan to get it done by Wednesday night hopefully.

[codecov]

Codecov Report

❌ Patch coverage is 88.52459% with 14 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/uniffi/model/pipeline.rs	82.35%	9 Missing ⚠️
src/uniffi/model/mod.rs	0.00%	5 Missing ⚠️

📢 Thoughts on this report? Let us know!

[guzman-raphael]

Before introducing petgraph serialization (assuming for storage), we should ensure it is deterministic.

e.g.:

graph TD;
    A-->B;
    A-->C;

Loading

and

graph TD;
    A-->C;
    A-->B;

Loading

should serialize the same in YAML. If not, we should defer until we can guarantee this.

[Synicix]

File an issue to properly fix this issue. At the moment it is none deterministic.

• TODO: Disable hash for now.

[guzman-raphael]

We should not public expose here. If meant to be public, it should be moved into within orcapod::uniffi::* with appropriate FFI exposure.

[Synicix]

Add TODO move it to uniffi side of the library, and add to derives to function and struct that we want to export.

[Synicix]

Added

[guzman-raphael]

Why was this needed? Why not just: Blob {kind, location, ..Blob::default()}.

[Synicix]

Actually, very fair point, let me fix this.

[Copilot]

Pull Request Overview

This PR delivers an MVP implementation of pipeline construction from DOT notation, refactors utility helpers for generic use, and adds test fixtures to validate the new behavior.

• Introduce Pipeline::from_dot and a unique kernel_lut to prevent duplicate kernels
• Generalize get and add find_missing_keys in core/util for input validation
• Add end-to-end tests and fixtures for Pipeline and PipelineJob

Reviewed Changes

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

Show a summary per file

File	Description
tests/pipeline.rs	Add basic tests for pipeline creation and specs
tests/fixture/mod.rs	Provide pipeline and pipeline_job fixtures
src/core/util.rs	Refactor get to generic key type and add find_missing_keys
src/uniffi/pipeline.rs	Implement Pipeline::from_dot, node hashing, and PipelineJob::new
src/uniffi/orchestrator/docker.rs	Minor update to JSON label lookup invocation

Comments suppressed due to low confidence (3)

tests/fixture/mod.rs:226

• [nitpick] The fixture function name pipeline shadows the Pipeline type and can be confusing. Consider renaming it to create_pipeline or similar.

pub fn pipeline() -> Result<Pipeline> {

tests/fixture/mod.rs:280

• [nitpick] This fixture pipeline_job could be renamed to create_pipeline_job to avoid ambiguity with the PipelineJob type.

pub fn pipeline_job() -> Result<PipelineJob> {

src/core/util.rs:41

• [nitpick] The variable name temp is generic; renaming it to value or entry would improve readability.

    let temp = map.get(key).context(selector::KeyMissing {

[Copilot]

The import of vec isn’t used anywhere in this test. Consider removing it to keep imports clean.

Suggested change

	use std::{collections::HashMap, path::PathBuf, vec};
	use std::{collections::HashMap, path::PathBuf};

[Copilot]

[nitpick] Allocating a String inline for the label key is unnecessary. Consider defining a const POD_JOB_LABEL: &str = "org.orcapod.pod_job" and using &POD_JOB_LABEL.to_owned() or similar to avoid repeated to_owned() calls.

Suggested change

	serde_json::from_str(get(&run_info.labels, &"org.orcapod.pod_job".to_owned())?)?;
	serde_json::from_str(get(&run_info.labels, &POD_JOB_LABEL.to_owned())?)?;

[Copilot]

Joiner kernels should not be treated as root nodes. Either filter out Joiner before this check or return an error here to prevent silent omissions.

[Copilot]

[nitpick] The logic to strip after the last _ relies on naming conventions and may be confusing. Document the intention or restrict this slicing to node-based lookup rather than kernel hashes.

Suggested change

	// Document the slicing logic and validate the format of `kernel_key`
	// The `kernel_key` is expected to follow the format: "<prefix>_<suffix>"
	// where `<prefix>` is the meaningful part used for lookup.
	if !kernel_key.contains(char_to_cut_at) {
	return Err(OrcaError::new(Kind::InvalidInput, format!("Invalid kernel_key format: {}", kernel_key)));
	}

[guzman-raphael]

@Synicix I have completed my review of your PR and summarized my feedback into this PR to your fork. Let me know if you have any questions when you go over it.

I'd recommend merging it through GH to prevent merge conflicts between us and so my contributions show up in the commit history. If you'd like to make some changes yourself, it is probably best to do it after merging my PR to your fork.