Skip to content

Various fixes and hardening: #51

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
leducp merged 1 commit into from
Jul 22, 2026
Merged

Various fixes and hardening: #51

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@ imgui.ini
CMakeSettings.json
.cache/
compile_commands.json
/TODO.md
12 changes: 11 additions & 1 deletion app/autosave.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,20 @@ namespace piper::studio
std::string const json = piper::v2::serialize(doc.graph, doc.pipeline_name);
{
std::ofstream f(tmp);
if (not f.is_open() or not (f << json))
if (not f.is_open())
{
return;
}
f << json;
// close() flushes; a failed flush (e.g. disk full) sets
// failbit. Checking after close catches a truncated write
// before it renames over the last good autosave.
f.close();
if (not f)
{
std::filesystem::remove(tmp, ec);
return;
}
}
std::filesystem::rename(tmp, path, ec);
if (ec)
Expand Down
31 changes: 31 additions & 0 deletions app/canvas_adapter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,37 @@ namespace piper::studio
{
return canvas::Connect::TypeMismatch;
}

PinRef const ra = pin_id_to_ref(a.id);
PinRef const rb = pin_id_to_ref(b.id);
if (not ra.attr.empty() and ra.node == rb.node)
{
return canvas::Connect::SameNode;
}

// An input pin may hold only one incoming link. Reject a second
// source so fan-in can't silently commit (the engine would then
// last-writer-win).
canvas::Pin const* input = nullptr;
if (a.kind == canvas::PinKind::Input)
{
input = &a;
}
else if (b.kind == canvas::PinKind::Input)
{
input = &b;
}
if (input != nullptr)
{
PinRef const in_ref = pin_id_to_ref(input->id);
for (auto const& l : graph_.links())
{
if (l.to == in_ref)
{
return canvas::Connect::AlreadyConnected;
}
}
}
return canvas::Connect::Allow;
}

Expand Down
6 changes: 6 additions & 0 deletions app/include/piper/app/main_window.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,12 @@ namespace piper::studio
// O(N + L) sweep; called once per draw().
void recompute_lints(Document& doc);

// Required (non-optional) input pins with no incoming link, as
// {node, attr}. Feeds both the lint and the Run gate -- running a
// graph with an unwired required input would throw at tick.
std::vector<std::pair<NodeId, std::string>>
unwired_required_inputs(Document const& doc) const;

void align_selection(Document& doc, AlignMode mode);
void distribute_selection(Document& doc, bool horizontal);

Expand Down
188 changes: 133 additions & 55 deletions app/main_window.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "piper/builtin_nodes.h"
#include "piper/canvas/event.h"
#include "piper/commands.h"
#include "piper/connect.h"
#include "piper/serialize_v2.h"

namespace piper::studio
Expand Down Expand Up @@ -283,7 +284,11 @@ namespace piper::studio
}
std::error_code ec;
std::string const abs = std::filesystem::absolute(path, ec).string();
std::string const key = ec ? path : abs;
std::string key = abs;
if (ec)
{
key = path;
}
recent_files_.erase(
std::remove(recent_files_.begin(), recent_files_.end(), key),
recent_files_.end());
Expand Down Expand Up @@ -1312,9 +1317,11 @@ namespace piper::studio
// modes panel.
if (not doc.graph.mode_profiles().empty())
{
char const* preview = doc.active_mode_profile.empty()
? "(none)"
: doc.active_mode_profile.c_str();
char const* preview = "(none)";
if (not doc.active_mode_profile.empty())
{
preview = doc.active_mode_profile.c_str();
}
ImGui::TextUnformatted("Profile");
ImGui::SameLine();
ImGui::SetNextItemWidth(-FLT_MIN);
Expand Down Expand Up @@ -1458,15 +1465,23 @@ namespace piper::studio
auto const it = doc.probe_history.find(n.id);
ProbeView pv;
pv.name = &n.name;
pv.hist = (it == doc.probe_history.end()) ? &empty : &it->second;
pv.hist = &empty;
if (it != doc.probe_history.end())
{
pv.hist = &it->second;
}
pv.color = probe_palette[probes.size() % palette_size];
probes.push_back(pv);
}
bool const any_probe = not probes.empty();

for (auto const& p : probes)
{
float const last = p.hist->empty() ? 0.0f : p.hist->back();
float last = 0.0f;
if (not p.hist->empty())
{
last = p.hist->back();
}
ImGui::PushStyleColor(ImGuiCol_Text, p.color);
ImGui::Text("%s = %.4g", p.name->c_str(), last);
ImGui::PopStyleColor();
Expand Down Expand Up @@ -1583,6 +1598,8 @@ namespace piper::studio
doc.engine.reset();
return;
}
// Run the mode selected in the UI; build() defaults to base mode.
doc.engine->set_mode(doc.active_mode_profile);
doc.engine_built_revision = doc.command_stack.revision();
doc.engine_running = true;
// Fresh capture buffer on start: avoids a visual discontinuity
Expand Down Expand Up @@ -1627,6 +1644,7 @@ namespace piper::studio
doc.engine_running = false;
return;
}
doc.engine->set_mode(doc.active_mode_profile);
doc.engine_built_revision = doc.command_stack.revision();
doc.probe_latest.clear();
doc.probe_history.clear();
Expand Down Expand Up @@ -1699,6 +1717,53 @@ namespace piper::studio
stage_play_next_advance_ = now + std::chrono::milliseconds(2000);
}

std::vector<std::pair<NodeId, std::string>>
MainWindow::unwired_required_inputs(Document const& doc) const
{
std::set<std::pair<NodeId, std::string>> connected;
for (auto const& l : doc.graph.links())
{
connected.insert({ l.to.node, l.to.attr });
connected.insert({ l.from.node, l.from.attr });
}

std::vector<std::pair<NodeId, std::string>> missing;
for (auto const& n : doc.graph.nodes())
{
// Optional inputs set is_optional in the spec and the step
// falls back when unwired (e.g. dt_in on sin_wave/low_pass/pid).
NodeType const* spec = registry_.find(n.type);
for (auto const& a : n.attrs)
{
if (a.role != AttributeSpec::Role::Input)
{
continue;
}
bool optional = false;
if (spec != nullptr)
{
for (auto const& s : spec->attributes)
{
if (s.name == a.name and s.is_optional)
{
optional = true;
break;
}
}
}
if (optional)
{
continue;
}
if (connected.count({ n.id, a.name }) == 0)
{
missing.push_back({ n.id, a.name });
}
}
}
return missing;
}

void MainWindow::recompute_lints(Document& doc)
{
doc.lint_diagnostics.clear();
Expand Down Expand Up @@ -1760,44 +1825,6 @@ namespace piper::studio
doc.lint_diagnostics.push_back(d);
}

// Skip optional inputs: their spec sets is_optional and the
// step is expected to fall back when unwired (e.g. dt_in
// on sin_wave / low_pass / pid).
NodeType const* spec = registry_.find(n.type);
for (auto const& a : n.attrs)
{
if (a.role != AttributeSpec::Role::Input)
{
continue;
}
bool optional = false;
if (spec != nullptr)
{
for (auto const& s : spec->attributes)
{
if (s.name == a.name and s.is_optional)
{
optional = true;
break;
}
}
}
if (optional)
{
continue;
}
if (connected.count({ n.id, a.name }) == 0)
{
Diagnostic d;
d.kind = Diagnostic::Kind::SchemaError;
d.message = "input '" + n.name + "." + a.name
+ "' has no source";
d.node_id = n.id;
d.attr_name = a.name;
doc.lint_diagnostics.push_back(d);
}
}

// Only flag missing per-node labels for nodes whose type
// advertises mode labels (preset3 etc.). Ordinary nodes
// don't dispatch on labels, so "no entry" means the engine
Expand All @@ -1815,6 +1842,21 @@ namespace piper::studio
doc.lint_diagnostics.push_back(d);
}
}

for (auto const& [nid, attr] : unwired_required_inputs(doc))
{
std::string nname;
if (Node const* n = doc.graph.find_node(nid); n != nullptr)
{
nname = n->name;
}
Diagnostic d;
d.kind = Diagnostic::Kind::SchemaError;
d.message = "input '" + nname + "." + attr + "' has no source";
d.node_id = nid;
d.attr_name = attr;
doc.lint_diagnostics.push_back(d);
}
}

void MainWindow::add_node_at(Document& doc,
Expand Down Expand Up @@ -2322,8 +2364,13 @@ namespace piper::studio
{
if (d->dirty) { ++dirty_count; }
}
char const* plural = "s";
if (dirty_count == 1)
{
plural = "";
}
ImGui::Text("%d document%s with unsaved changes:",
dirty_count, (dirty_count == 1) ? "" : "s");
dirty_count, plural);
for (auto const& d : documents_)
{
if (not d->dirty) { continue; }
Expand Down Expand Up @@ -2538,6 +2585,10 @@ namespace piper::studio
adoc.active_mode_profile.clear();
adoc.adapter.set_active_mode_profile(adoc.active_mode_profile);
adoc.adapter.rebuild();
if (adoc.engine != nullptr)
{
adoc.engine->set_mode(adoc.active_mode_profile);
}
}
for (auto const& mp : adoc.graph.mode_profiles())
{
Expand All @@ -2548,6 +2599,10 @@ namespace piper::studio
adoc.adapter.set_active_mode_profile(adoc.active_mode_profile);
adoc.adapter.rebuild();
adoc.lint_dirty = true;
if (adoc.engine != nullptr)
{
adoc.engine->set_mode(adoc.active_mode_profile);
}
}
}
ImGui::EndCombo();
Expand Down Expand Up @@ -3307,20 +3362,27 @@ namespace piper::studio
}
if (not to_open.empty())
{
std::string const fname =
std::filesystem::path(to_open).filename().string();
if (load_file(to_open))
{
push_toast(ToastLevel::Info,
"Recovered " + std::filesystem::path(to_open)
.filename().string());
push_toast(ToastLevel::Info, "Recovered " + fname);
// Only drop the autosave once its contents are safely
// loaded -- a failed load must keep the sole copy.
std::error_code ec;
std::filesystem::remove(to_open, ec);
autosave_pending_.erase(
std::remove(autosave_pending_.begin(), autosave_pending_.end(), to_open),
autosave_pending_.end());
if (autosave_pending_.empty())
{
ImGui::CloseCurrentPopup();
}
}
std::error_code ec;
std::filesystem::remove(to_open, ec);
autosave_pending_.erase(
std::remove(autosave_pending_.begin(), autosave_pending_.end(), to_open),
autosave_pending_.end());
if (autosave_pending_.empty())
else
{
ImGui::CloseCurrentPopup();
push_toast(ToastLevel::Error,
"Could not recover " + fname + " -- autosave kept");
}
}
if (not to_discard.empty())
Expand Down Expand Up @@ -3898,6 +3960,22 @@ namespace piper::studio
{
break;
}
// Authoritative domain check for node<->node links
// (same-node, kind/type mismatch, fan-in). Label
// endpoints resolve type/direction at build via the
// label cluster, so skip the node-only validator there.
bool const involves_label =
doc.graph.find_label(from.node) != nullptr
or doc.graph.find_label(to.node) != nullptr;
if (not involves_label)
{
piper::TypeCheck const tc;
if (piper::validate_connection(doc.graph, from, to, tc)
!= piper::Connect::Allow)
{
break;
}
}
// Either endpoint may be a label (wildcard pin); the
// declared data_type comes from whichever side is a
// real Node attribute.
Expand Down
2 changes: 1 addition & 1 deletion core/include/piper/serialize_v2.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

namespace piper::v2
{
constexpr int format_version = 3;
constexpr int format_version = 2;
constexpr int min_supported_version = 2;

// ---- Graph bundle (one or many pipelines) ----
Expand Down
Loading
Loading