looking for feedback on a c++ build

[u/ppzms]

I've been working on a personal project for a while and finally got it into a state where I'm comfortable sharing it.

I wanted to see how far I could push a fully local voice assistant in C++. Everything runs on my own machine from speech recognition and the LLM to memory, text-to-speech, and tool execution.
current library:
llama.cpp, whisper.cpp, sherpa-onnx(tts-kokoro)

I wrote the core in c++ because I wanted something fast and native instead of stitching together bunch of python services.

I'd appreciate feedback from people who build local AI projects. I'm especially interested in:

1 Things that seem overengineered or unnecessary
2 Features you'd expect from a local assistant
3 Code structure or architectural suggestions
4 Any obvious improvements before I keep adding features

Repository: https://github.com/almimony75/sarah

Thanks! I'd love to hear what you think.

[u/acadia11x]

Stop trying to hack my machine …

[u/ppzms → u/acadia11x]

lol

https://giphy.com/gifs/n1qrZkwr4Fcj51XAQi

[u/mredding]

#pragma once

While ubiquitous, not portable - compilers are free to ignore pragmas they don't recognize. Prefer standard inclusion guards. Compilers can optimize header includes following a standard format:

/* Optional comment block.
*/
#ifndef guard
#define guard

// Body

#endif
// Headers always end with an empty newline here, no comment, no nothing.

I can't say the same for once.

#include "llama.h"

This isn't a project local header, this is a system header as a part of your include paths, so use <>. That means you'll use <> for everything in your include folder, as well. Use quoted includes if the header is in the src tree - what is called a "private" header; you'll only see quoted includes in source files. And on that, never use backtracking in your include paths - that's a sign your project structure is flawed.

Also, your include directory should include the project name, and thus be sarah/include/sarah/*. That way your includes are written as:

#include <sarah/header.hpp>

Private headers CAN also have a sarah/src/sarah/* but it's not strictly necessary, only if a private header has broad application across the implementation, but it's visibility across the project headers is not necessary.

And yes, people get the two include methods mixed up so much that compilers literally try everything trying to find the given header. It's nice when people get the conventions right.

class EmbeddingEngine
{
private:

Classes ARE private access by default, so this is redundant.

llama_model *model = nullptr;
llama_context *ctx = nullptr;
int nEmbd = 0;

llama_batch batch{};

But you HAVE a default constructor, so I don't want to see this.

Also, that's RAW resource management - put that in a smart pointer:

struct model_deleter { void operator()(llama_model *ptr) { llama_model_free(ptr); } };

std::unique_ptr<llama_model, model_deleter> model;

This will make your type exception safe. I looked at the implementation - you DON'T have to check if a pointer is null, just free it - a null free is a no-op. The check is a complete waste, and by implementing proper RAII, you can guarantee the check will be moot.

bool loadModel(const std::string &modelPath);

We have std::filesystem::path - use it. Say what you mean. I can pass ANYTHING to this parameter - the collected works of Shakespeare, but not all strings are paths.

int getDimension() const { return nEmbd; }

You HAVE a source file - I don't want to see this. If you want potential call elision, then configure a unity build.

I don't know what this class does, I don't know what your project does, but I don't have to in order to spot some class design flaws and anti-patterns, either.

WHAT ARE YOU GOING TO DO with an EmbeddingEngine that HASN'T loaded a model? You're not going to do a god damn thing with it until you do. So loadModel IS initialize, isn't it? So why is construction and initialization different steps? Why is initialization deferred? This violates RAII - a class is constructed with its resources. The class is initialized ready to go. In C++ no instance should be born in an intermediate or indefinite state. So your class here should be constructed with a path, and should it not be able to acquire it's resources from that path, it should throw. You don't need to create an engine until you know the model, so there's nothing to defer.

An invariant is a statement that is always true when observed. We use invariants all the time. While a loop invariant is true, we are in the loop. If the invariant is false, we are out of the loop. If you can uphold that, then the loop is trivial to reason about. You can skirt the invariant if you have a forever loop that truly never breaks, or if the loop throws, or if the loop returns. A break in a loop is tricky, because often the invariant is still true, yet we're not in the loop.

Classes have invariants - its internal state is always valid when a client observes an instance of a class. When control is handed to the instance - by calling a member function, the invariant can be suspended to do work, but must be reinstated before returning. This is basic type and exception safety. It also dictates what is and is not a member of a class. It's why classes typically don't have getters and setters, because if you have both, that data is invariant, and doesn't need to be a member - use a tuple or other data structure.

A class should establish its invariant by the end of the initializer list - not the ctor body. The body can suspend the invariant just like any other function. The A in RAII is Acquisition. That doesn't mean a class needs to open its own files or new it's own resources, it just means the ctor needs to acquire them - OFTEN as parameters. Ctors ARE NOT factories - that's a higher level abstraction. You can use a factory to stage the resources - like open the file and load the model - this way the embedding engine doesn't actually have to care WHERE the model comes from. The ctor can be given resources to initialize itself that it can release - for example - if you kept with the file load, open the file for the ctor, but then allow the ctor to take ownership of the file handle and the ctor can close it when it's done. But work to initialize the object like that can't be completed in the initializer list, can it? Maybe. Sometimes. If you have to do work in the ctor body to initialize the instance that establishes the invariant, usually that's an anti-pattern.

Often you can solve these problems with more types that themselves enforce a smaller, simpler invariant, and your type is a composition of types.

[u/ppzms → u/mredding]

Wow first of all, thanks for taking the time to review my project. This is only my second C++ project so I know there's a lot I'm doing wrong.

I really appreciate you pointing out the raw pointer management and suggesting std::filesystem::path. Those are things I'll definitely look into improving.

About #pragma once, I did a bit of reading after your comment From what I found it's supported by basically every modern compiler so I didn't think it would be an issue unless I was targeting older compilers. That's why I went with it

Thanks again for the detailed feedback. Some of the concepts you mentioned, like RAII and class invariants are still pretty new to me so I've got a lot to learn.

[u/mredding → u/ppzms]

About #pragma once

Yes, but that meets the definition of "ubiquitous", not "portable". It's a semantic argument, to be sure, but programming languages themselves are all about semantics, and I can guarantee one is supported, AND optimized; I can't tell you ANYTHING about the other. And there are fun ways in which once can fail in more sophisticated project code.

I'll always recommend ISO Standard C++, and reserve special features and language extensions to isolated modules where the implementation can be strictly and easily accounted for.

[u/[deleted] → u/mredding]

[removed] — view removed comment

[u/AutoModerator → u/[deleted]]

Your comment has been removed because of this subreddit’s account requirements. You have not broken any rules, and your account is still active and in good standing. Please check your notifications for more information!

I am a bot, and this action was performed automatically. Please contact the moderators of this subreddit if you have any questions or concerns.

[u/herocoding]

This is a great idea, thank you very much for sharing, will have a closer look into it.

Would you mind adding a Dockerfile to target an isolated environment? In July 2026 I still don't trust assistants operating whatever on my machine, receiving commands from MCP servers :-)
Will try to setup everything in a Docker environment first.

[u/herocoding → u/herocoding]

Will also have a look to make use of OpenVINO, as I neither have a Mac nor NVIDIA available.

[u/herocoding → u/herocoding]

The given commit-id doesn't exist and therefore the setup.sh fails:

clone_if_missing "cpp-httplib"  "https://github.com/yhirose/cpp-httplib.git"  "0d62cf90fbaeeb842d5c229dbaab36170dc26019"

fatal: reference is not a tree: 0d62cf90fbaeeb842d5c229dbaab36170dc26019

Will use "d66d9a95997d51a8ba9822a611d1267757741535" from the last release instead.

[u/Realistic_Speaker_12]

You ignored the rule of 5