NitroDigest questions and OOP ideas

[nestak]

Hey,

first of all, thank you for the initiative and the invitation to discuss ideas!
I've read the implementation of the NitroDigest and have some questions and suggestions regarding it.

Thorough the code I see a lot of "doer" classes, for example "writer", "summarizer", "processor", etc.

I think this happens, because usually we are constrained on time, and focus on implementation. So, a writing procedure becomes an object which is a "writer" with a "write" method, or a "summarizer" object with a "summary" method.

So was naming intended, or it came out of usual conventions we see being copied everywhere (controller, provider, writer, etc.)?

Interesting reading in regard of the above, could be what Carlo Pescio wrote on his blog: Your Coding Conventions Are Hurting You. I think he has some good points.

I thought it would be nice to do something about it, so I experimented a bit on naming (see below). Let me know what do you think about this direction.

Redesign attempt

I asked myself a few questions:

• Is passing around data good, or data should be local to the object instance?
• If I need to pass some data, should I pass a primitive or an object?
• Should I focus on behaviours? How can I describe what the object/class is about by its name? (Because the name will influence the implementation.) Maybe I should think about what kind of data that object holds and operates on?
• Should main function have any business logic (if statements), or loops? (seems like micro-management).
• How can I keep data local to the object.

My first (not good) attempt was:

# Rename BaseSummarizer to SummaryService.
# This would be the interface or base/abstract class.
class SummaryService:
    def summarize(text):

# Get rid of ClaudeSummarizer, ChatGPTSummarizer, and OllamaSummarizer
# and introduce SummaryNetworkService.
# The reason is that all of them just send a http request, so I figured
# we could have one implementation of that, and only provide (somehow)
# request headers and body, based on user input.
class SummaryNetworkService(SummarySevice):
    def summarize(text, service_name='ollama'):

# Rename EmailProcessor to ESP, which is short of Email Service Provider.
# (This is quite bad, and improved upon in the next attempt.)
# I figure the only exposed thing could be to fetch/load the things we want.
# We don't want to manage connections at this level, and delegate that details
# to ESP object. It should open/close connections as needed.
class ESP:
    def fetch_newsletters(): -> Newsletter[]

class Newsletter:
    def summarize(SummaryService):
    def mark_as_read():

Above is only a rough draft, and a lot is to be desired. What I didn't touch yet, was html_extractor, prompt, and summary_writer classes. Also, dealing with user input would be nice, and having some plan for the main.py file, which now acts as a controller (micro-managing, looping).

Second attempt

After thinking some more, I came up with this:

# Introduce a new class, Digest, which is what this program is all about :)
# What it does is it takes a thing to digest, in our case newsletters,
# and calls a digest service to digest them.
# 
# Next, it can do some useful stuff for us, like write summaries to disk.
# This change will get rid of SummaryWriter, as Digest will take over
# its responsibilities.
class Digest:
    def __init__(self, thing_to_digest, digest_service):
    def write_to_disk(file_path):

# Rename SummaryService to DigestService.
class DigestService:
  def summarize(text):

# Rename SummaryNetworkService to DigestNetworkService.
class DigestNetworkService(DigestService):
  def __init__(self, user_input):
  def summarize(text):

class UserInput:
    # No idea if this should be a class, just a note here
    # that it is something to be taken care of.

# ESP was a bad name, so lets go with EmailService.
class EmailService:
    def fetch_newsletters(UserInput): -> NewsletterCollection

# Rename Newsletters to NewsletterCollection, so it becomes more obvious
# we are going to operate on multiple elements at once.
# 
# Now, we're going to remove summarize() method from here, because we want
# our new Digest class to handle that.
# 
# Next we will introduce to_text() method.
# This will allow to remove html_extractor module, since NewsletterCollection
# will handle this "extracting". Again, we abstract away HTML, as we are
# actually interested in having a text version of newsletters, and dealing with
# HTML is hidden now.
class NewsletterCollection:
    def to_text(): -> string[]
    def mark_read():

Then, we use above classes:

newsletters = EmailService(user_input).fetch_newsletters()

# Here, after creating the digest, we write summaries to disk.
Digest(newsletters, DigestService(user_input)).write_to_disk(file_path)

# Optionally, we can mark fetched newsletters as read.
# Note, we don't pass any identifiers around, since newsletters object
# encapsulates this state, and knows already what to do.
newsletters.mark_read()

More changes

# Potentially, we could merge EmailService and NewsletterCollection.
# That way we could have all IMAP calls localized in one place.
class NewsletterCollection:
    def fetch_from_server(user_input):
    def to_text(): -> string[]
    def mark_read():

Usage:

newsletters = NewsletterCollection()
newsletters.fetch_from_server(user_input)
Digest(newsletters, DigestService(user_input)).write_to_disk(file_path)
newsletters.mark_read()

[nestak]

Just to clarify, Digest implementation could be something along the lines of:

class Digest:
    # ...
    def write_to_disk(file_path):
        with open(file_path) as f:
            f.write(self.digest_service.summarize(self.newsletters.to_text()))

[RetroModernDev]

Hi @nestak, thanks for starting this discussion!

Let's start with a quick recap.

Recap

Thorough the code I see a lot of "doer" classes, for example "writer", "summarizer", "processor", etc.

Oh yes. When I started this project, my goal was to create a simple tool for myself that would summarize emails and remove noise and marketing BS from them.

In the beginning, I had one class EmailReader.cs (it was in C#) that did everything:

• connecting to IMAP
• read emails
• Communicate with LLM
• summarize them

Of course, it worked, but it was hard to read and develop.

Then I split this one big class into smaller ones that have clear responsibilities.

In the end, I have a lot of "doer" classes, as you mentioned.

In the C # implementation, I tried to follow more or less clean architecture principle.

Then, I refactored the app to a Python app.

Now you started this discussion and this is perfect timing, because the app is really small and we can consider some options and implement them easily. 🎉

Operation-centric naming vs. domain-centric naming

As I see it we talk about operation-centric names ("doers") vs. domain-centric naming

First approach focuses more on implementation details, the second one is more conceptual and I see it can be more flexible.

I have some concerns and open questions about how the second approach will work when we add new features to the app.

Let's see!

Design considerations

1. How might the redesign handle adding new LLM providers? Would you need to modify DigestNetworkService or could you extend it?

As far as I understood, when I want to add a new LLM provider, I need to extend DigestNetworkService.

class DigestNetworkService(DigestService):
  def __init__(self, user_input):
  def summarize(text):

So if I want to specify which LLM should be used, I need to specify it in the user_input, right?

Then, you have to modify DigestNetworkService to support new LLMs and it can be challenging.

How to design this to allow adding new LLMs, without the need to modify existing classes?

I see two possible approaches

Pass LLM provider as dependency

class DigestNetworkService(DigestService):
    def __init__(self, llm_provider):
        self.provider = llm_provider
    
    def summarize(self, text):
        return self.provider.summarize(text)

# Usage:
claude_provider = ClaudeProvider()
digest_service = DigestNetworkService(claude_provider)

Use factory to create DigestrNetworkService

class LLMProviderFactory:
    @staticmethod
    def create_provider(provider_name):
        if provider_name == "claude":  # in real implementation it should not be hardcoded string
            return ClaudeProvider()
        elif provider_name == "gpt":
            return GPTProvider()
        elif provider_name == "ollama":
            return OllamaProvider()
        # New providers can be added here
        else:
            raise ValueError(f"We do not have {provider_name}. Please create a PR to add it and make a world a better place ")

class DigestNetworkService(DigestService):
    def __init__(self, provider_name):
        self.provider = LLMProviderFactory.create_provider(provider_name)
    
    def summarize(self, text):
        return self.provider.summarize(text)

What do you think about these approaches?

Inheritance

I see that DigestNetworService inherits from DigestService:

class DigestService:
  def summarize(text):
    # Base implementation or abstract method

class DigestNetworkService(DigestService):
  def __init__(self, user_input):
  def summarize(text):

What if you later add a LocalDigestService that runs models locally?

For example, imagine that we will add an embedded ML model (like using the TextRank algorithm, or loading a small model with libraries like transformers or spaCy).

The client code expects a DigestService to work regardless of implementation, but the behavior differences could be problematic.

So in this case we have two implementations that behave differently

• DigestNetworService - make HTTP calls
• LocalDigestService - do not make HTTP calls

Challanges I see here:

1. Behavior Differences:

How do we define a consistent output format and quality standard across all summarization methods?

2. Error Handling

How should we standardize errors so clients can handle failures consistently without knowing implementation details?

3. Configuration Requirements:

Which parameters are truly essential to the core contract versus implementation-specific details?

Maybe (really maybe) we can explicitly state what all implementations of DigestService must guarantee (e.g., handling of empty text, expected output format, error cases).

class DigestService:
    def summarize(text): 
        """Returns a summary of the text or raises DigestException"""
        pass

class NetworkDigestService(DigestService):
    def summarize(text):
        try:
            # Network implementation
            return network_result
        except NetworkError as e:
            # Translate to a common exception type
            raise DigestException(f"Network error during summarization: {str(e)}")

Adding new features

The next thing that concerns me is how we can implement new features.

Before I start - I am sorry - ideas for new features were only in my head. If you think it's not a good moment to bring them up to the work table, just let me know.

Multiple content sources

1. How can we implement other content sources besides Newsletters? For example: RSS feeds, web articles, and files?
2. Would you create a common interface like ContentSource with specific implementations?
3. Should EmailService be generalized to a broader concept?

New summarizers

I mentioned it before I think, but let me ask it in again in a little different way

1. How would you incorporate ML-based summarizers like TextRank alongside LLM-based approaches?

Proposal

What do you think about an approach like this:

# Content source interfaces and implementations
class ContentSource:
    def fetch_content(self, parameters): pass

class EmailContentSource(ContentSource):
    def fetch_content(self, parameters):
        # Implementation to fetch from email
        pass

class RSSContentSource(ContentSource):
    def fetch_content(self, parameters):
        # Implementation to fetch from RSS
        pass
        
class FileContentSource(ContentSource):
    def fetch_content(self, parameters):
        # Implementation to read from files
        pass

# Digest service interfaces and implementations
class DigestService:
    def summarize(self, content): pass

# LLM-based summarization services
class LLMDigestService(DigestService):
    def __init__(self, llm_provider):
        self.provider = llm_provider
        
    def summarize(self, content):
        return self.provider.generate_summary(content)

# LLM Provider interface
class LLMProvider:
    def generate_summary(self, content): pass
    
class ClaudeProvider(LLMProvider):
    def __init__(self, api_key, model="claude-3-opus-20240229"):
        self.api_key = api_key
        self.model = model
        
    def generate_summary(self, content):
        # Implementation for Claude API
        pass
        
class GPTProvider(LLMProvider):
    def __init__(self, api_key, model="gpt-4"):
        self.api_key = api_key
        self.model = model
        
    def generate_summary(self, content):
        # Implementation for GPT API
        pass
        
class OllamaProvider(LLMProvider):
    def __init__(self, host="http://localhost:11434", model="llama2"):
        self.host = host
        self.model = model
        
    def generate_summary(self, content):
        # Implementation for Ollama API
        pass

# ML-based summarization service
class TextRankDigestService(DigestService):
    def __init__(self, ratio=0.2):
        self.ratio = ratio
        
    def summarize(self, content):
        # Implementation using TextRank algorithm
        pass

# Orchestrator that connects content sources with digest services
class DigestOrchestrator:
    def __init__(self, content_source, digest_service):
        self.content_source = content_source
        self.digest_service = digest_service
    
    def create_digest(self, parameters):
        content = self.content_source.fetch_content(parameters)
        return self.digest_service.summarize(content)

(Sorry, I have there a DigestOrchestrator which is another "doer" class, but looks like my mind is not able to change mindset easily xD)

Anyway, take a look on usage examples:

# Example 1: Summarize emails with Claude
email_source = EmailContentSource()
claude_provider = ClaudeProvider(api_key="your_api_key")
claude_service = LLMDigestService(claude_provider)
email_claude_orchestrator = DigestOrchestrator(email_source, claude_service)

# Run the orchestration
email_summary = email_claude_orchestrator.create_digest({"folder": "INBOX", "since": "1d"})

# Example 2: Summarize RSS feeds with GPT
rss_source = RSSContentSource() 
gpt_provider = GPTProvider(api_key="your_gpt_key")
gpt_service = LLMDigestService(gpt_provider)
rss_gpt_orchestrator = DigestOrchestrator(rss_source, gpt_service)

# Run the orchestration
rss_summary = rss_gpt_orchestrator.create_digest({"url": "https://news.example.com/feed"})

# Example 3: Summarize local files with TextRank
file_source = FileContentSource()
textrank_service = TextRankDigestService(ratio=0.3)  
file_textrank_orchestrator = DigestOrchestrator(file_source, textrank_service)

# Run the orchestration
file_summary = file_textrank_orchestrator.create_digest({"path": "/path/to/documents"})

Wrap up

• I wrote about the codebase evolved from a single class (EmailReader.cs) to multiple "doer" classes with clear responsibilities
• We're considering shifting from operation-centric naming ("doers") to domain-centric naming for better flexibility
• I proposed two approaches for adding new LLM providers: dependency injection and factory pattern
• I raised challenges with the inheritance hierarchy of DigestService
• I proposed a solution with interfaces for content sources and digest services
• Proposed architecture supports multiple content types (email, RSS, files) and summarization methods (various LLMs, TextRank)
• Dependency injection allows for flexible combinations of content sources and summarizers
• The orchestrator pattern connects content sources with digest services
• I wait for your feedback

Thanks again!

[nestak]

Wow, thanks for this writeup, you raised great points!
I need to think some more, and will come back with answers.

[nestak]

Hey, my answers and some "food for thought":

How might the redesign handle adding new LLM providers? Would you need to modify DigestNetworkService or could you extend it?

As far as I understood, when I want to add a new LLM provider, I need to extend DigestNetworkService.

My idea was to define interface in DigestService. Then have DigestNetworkService to support a bunch of (currently implemented) backing services which use HTTP.

If by extending you mean inheriting from DigestNetworkSevice, then no, that was not what I wanted to do.

• When adding a new LLM I thought rather about adding methods to DigestNetworkService.
• If we need to add something different, then inherit from DigestService which provides only an interface. In current implementation DigestService would not even be needed.

More concrete example:

class DigestNetworkService(DigestService):
    def __init__(self, user_input):
        self.user_input = user_input # "user_input" is just args

        # this can be done in different ways
        # in this example I am using dictionary
        self.backends: {
            'ollama': self.__ollama,
            'foo': self.__foo
        }

    # could be a static method
    def available_backends(self):
        return list(self.backends.keys())

    def summarize(self, text):
        backed = self.user_input.summarizer
        if not text:
            return ""
        elif self.backends[backend]:
            return self.backends[backend](text)
        else:
            raise NotImplementedError(
                f"No such backend found: {backend}")
    
    def __ollama(self, text):
        # http call implementation
        # ...

# main.py
    backends = DigestNetworkService().available_backends()
    parser.add_argument("--summarizer", choices=backends, default=backends[0],

So if I want to specify which LLM should be used, I need to specify it in the user_input, right?

For now, lets think of "user_input" as just renamed args object – this is how I thought about it. It holds this information already in the summarizer property.

In my examples, I pass the whole args object, instead of primitives. I should have explained that right away, sorry about that.

How to design this to allow adding new LLMs, without the need to modify existing classes?

Why such concern?

We will always be forced to modify some class or calling code to "add"/"register" a new LLM backend (all those if statements in a factory, or main function). What should not change, is existing functionality and interface. I guess that's what you meant, but correct me if I am wrong.

Regarding passing in a provider

I don't like this approach, because client code needs to have all those if statements checking args.summarizer and instantiating classes.

Regarding using factory

I like this approach better. This is similar to example I given above, but separates implementations into another classes.

On inheritance

1. Behavior Differences:

How do we define a consistent output format and quality standard across all summarization methods?

I thought input and output of summarize() method would always be text. What do you mean by quality standards?

2. Error Handling

How should we standardize errors so clients can handle failures consistently without knowing implementation details?

I am not sure about this, why "standardize" errors?

3. Configuration Requirements:

Which parameters are truly essential to the core contract versus implementation-specific details?

I think the core is text in, text out.

On new features

How can we implement other content sources besides Newsletters? For example: RSS feeds, web articles, and files?

I think we basically need two methods – fetch() (to load the content from whatever location); and to_text() which transforms this content to format acceptable by summarize().

Would you create a common interface like ContentSource with specific implementations?

Probably yes, having above methods fetch() and to_text().

Should EmailService be generalized to a broader concept?

I previously mentioned, that I would merge EmailService with NewsletterCollection, so it could be a type of "ContentSource".

How would you incorporate ML-based summarizers like TextRank alongside LLM-based approaches?

I am not sure yet, I would need to look into specifics.

On proposal

Hmm, looks like you changed one set of "doers" to another set: "summarizers" vs "providers" :)

Regarding "DigestOrchestrator"... yeah, I don't like it :) I would stick to "Digest" or "Summary".

Other than that, like before, it leaves to client code (main) to handle all the logic with choosing to instantiate correct classes. I think we could explore other directrions, and hide all the if statements in a factory or something similar. Make the objects do something useful for us (delegate), and not be tied to knowing which class to instantiate.

If I understood examples correctly, anytime the arguments are different, client code needs to handle that. What would you say if, there would be only one composition in client code?

[RetroModernDev]

Hi @nestak, thanks for your interesting ideas and point of view. 🙏

When adding a new LLM I thought rather about adding methods to DigestNetworkService.

For me it's not scalable, because when I have more and more providers, I will have one big class. Also testing can be difficult because to test this class, I need to mock bunch of services.

I prefer to have smaller files/classes.

---

How to design this to allow adding new LLMs, without the need to modify existing classes?

Why such concern?

We will always be forced to modify some class or calling code to "add"/"register" a new LLM backend (all those if statements in a factory, or main function). What should not change, is existing functionality and interface. I guess that's what you meant, but correct me if I am wrong.

I was thinking about something that allows adds plugins to the app. So rather than modifying existing classes I would like to register a plugin which adds a new LLM for example.

---

I thought input and output of summarize() method would always be text. What do you mean by quality standards?

During my tests of summarizing newsletters using different providers and models (with the same prompt) I have some observations:
Claude-generated summary might naturally be a list of items with perfect content summary and links that works

Llama 2 for the same prompt and newsletter returns sometimes longer summary, sometimes in bullet lists, sometimes as paragraphs. Links are typically broken.

I tested a solution that sends the second request with different prompt that forces Llama to summarize text in a way that I want.

So when I am talking about quality standards, I mean that returned summary should be in the same quality, no matter which provider I use.

---

I am not sure about this, why "standardize" errors?

Each implementation might throw different types of exceptions:

• Network-based summarizers could throw HTTP errors, timeouts, or authentication failures
• Local ML models might throw memory errors or model loading failures
• TextRank algorithms might throw parsing errors

So we should standarize errors to expicity show what errors can be thrown, regardless of provider user. Example

• DigestServiceException as the base exception
• PrompTooLargeException for when prompt exceeds max tokens that provider can handle
• ServiceUnavailableException for when a service can't be reached

---

If I understood examples correctly, anytime the arguments are different, client code needs to handle that. What would you say if, there would be only one composition in client code?

That would be amazing. I experimented with something like pipeline architecture:

NitroDigest.create()
    .read(emailReader)
    .normalize(htmlExtractor)
    .summarize(claude)
    .save(fileManager)

I have working demo on my local, but it's AI generated and I will not push it to the internet before I refactor it 😄

Also, I would like to explore a direction with hexagonal architecture.

---

I hope I answered your question and things are clearer than before.

Thanks for your contribution and have a nice Bank Holiday!

--
Marcin