-
Notifications
You must be signed in to change notification settings - Fork 1
feat: per-company invoice classifier configuration #170
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.
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
Merged
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
09b8280
feat: per-company invoice classifier configuration
emilwojtaszek 5e44069
fix: harden per-company classifier config
emilwojtaszek ad673e0
refactor: use TrackedRepo, context API facade, and fix health probe r…
emilwojtaszek db3fe6d
fix: SSRF protection, form-state health check, and stale pending_params
emilwojtaszek 5ce5a9f
refactor: flatten host_allowed? nesting to satisfy credo
emilwojtaszek d5d55e7
fix: invalidate stale probes, block SSRF override, check all resolved…
emilwojtaszek File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| --- | ||
| name: Per-Company Classifier Configuration | ||
| description: Company-scoped invoice classifier settings with env-var fallback, enabling per-company ML models while preserving local dev simplicity. | ||
| tags: [classifier, services, settings, multi-tenant] | ||
| author: emil | ||
| date: 2026-04-23 | ||
| status: Accepted | ||
| --- | ||
|
|
||
| # 0049. Per-Company Classifier Configuration | ||
|
|
||
| Date: 2026-04-23 | ||
|
|
||
| ## Status | ||
|
|
||
| Accepted | ||
|
|
||
| ## Context | ||
|
|
||
| KSeF Hub runs three sidecar services: pdf-renderer, invoice-extractor, and invoice-classifier. All were configured via environment variables, identical for every company. | ||
|
|
||
| Two problems: | ||
|
|
||
| 1. **Per-company ML models** — different companies may need different classifier endpoints (dedicated models trained on their data). A global URL can't support this. | ||
| 2. **Local dev with remote DB** — if config lived only in the database, developers connecting to a shared DB (e.g., Supabase) would get production sidecar URLs instead of localhost, breaking local development. | ||
|
|
||
| PDF renderer and invoice extractor are shared infrastructure that rarely changes — only the classifier needs per-company configuration. | ||
|
|
||
| ## Decision | ||
|
|
||
| ### Env vars remain the default | ||
|
|
||
| All sidecar URLs and tokens stay in `runtime.exs` as environment variables. This preserves local dev: `PDF_RENDERER_URL=http://localhost:3001` just works regardless of which database you connect to. | ||
|
|
||
| ### Classifier gets per-company DB overrides | ||
|
|
||
| A new `classifier_configs` table stores optional per-company overrides: | ||
|
|
||
| | Column | Type | Purpose | | ||
| |--------|------|---------| | ||
| | `company_id` | FK, unique | One config per company | | ||
| | `enabled` | boolean (default false) | When false, env vars are used | | ||
| | `url` | string | Classifier endpoint URL | | ||
| | `api_token_encrypted` | binary | AES-256-GCM encrypted bearer token | | ||
| | `category_confidence_threshold` | float | Auto-apply threshold | | ||
| | `tag_confidence_threshold` | float | Auto-apply threshold | | ||
|
|
||
| When `enabled = true`, the company's DB values override env vars for that company's classification operations. When `enabled = false` (default), env vars drive everything — the DB row is inert. | ||
|
|
||
| ### UI in Settings → Services | ||
|
|
||
| Owner/admin-only settings page at `/c/:company_id/settings/services` with: | ||
| - Enable/disable toggle with fieldset that grays out when disabled | ||
| - Env var values shown as placeholders so users know what they're overriding | ||
| - Health check before save (warns if unreachable, allows save anyway) | ||
| - Collapsible API endpoint documentation | ||
|
|
||
| ### What was considered and rejected | ||
|
|
||
| - **Generic service_configurations table** — started here, but only the classifier varies per company. A generic table added complexity (service_name column, settings JSON map, generic card loop) for no real benefit. | ||
| - **DB-only config (no env vars)** — broke local development when connecting to a remote database. Env vars as the baseline solved this cleanly. | ||
| - **Global (non-company-scoped) config** — one company changing the classifier URL would affect all companies. Unacceptable for a multi-tenant SaaS. | ||
|
|
||
| ## Consequences | ||
|
|
||
| - Classifier client modules still read `Application.get_env` directly. Wiring per-company resolution into the classification pipeline is a separate task (the `resolve_*` functions and client behaviour changes needed). | ||
| - PDF renderer and invoice extractor have no UI — they use env vars exclusively. If per-company needs arise later, the pattern is established. | ||
| - Three migration files exist from the iteration (create generic table → update → replace with classifier-specific). Harmless on fresh DBs but noisy. | ||
|
|
||
| ## Key Files | ||
|
|
||
| | File | Purpose | | ||
| |------|---------| | ||
| | `lib/ksef_hub/service_config.ex` | Context — CRUD, env_defaults | | ||
| | `lib/ksef_hub/service_config/classifier_config.ex` | Ecto schema with conditional validation | | ||
| | `lib/ksef_hub_web/live/settings_live/services.ex` | LiveView — settings page | | ||
| | `priv/repo/migrations/*classifier_configs.exs` | DB migration | |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| defmodule KsefHub.ServiceConfig do | ||
| @moduledoc """ | ||
| Context for per-company sidecar service configuration. | ||
|
|
||
| Currently supports the invoice classifier only (the main per-company use case). | ||
| PDF renderer and invoice extractor use global env vars exclusively. | ||
|
|
||
| When a company's classifier config is **enabled**, its values override | ||
| the global env-var defaults for that company's operations. | ||
| """ | ||
|
|
||
| alias KsefHub.ActivityLog.TrackedRepo | ||
| alias KsefHub.Credentials.Encryption | ||
| alias KsefHub.Repo | ||
| alias KsefHub.ServiceConfig.ClassifierConfig | ||
|
|
||
| @doc "Returns the classifier config for a company, or nil if none exists." | ||
| @spec get_classifier_config(Ecto.UUID.t()) :: ClassifierConfig.t() | nil | ||
| def get_classifier_config(company_id) do | ||
| Repo.get_by(ClassifierConfig, company_id: company_id) | ||
| end | ||
|
|
||
| @doc """ | ||
| Returns the classifier config for a company, creating a disabled default if none exists. | ||
| """ | ||
| @spec get_or_create_classifier_config(Ecto.UUID.t()) :: ClassifierConfig.t() | ||
| def get_or_create_classifier_config(company_id) do | ||
| case get_classifier_config(company_id) do | ||
| nil -> | ||
| %ClassifierConfig{company_id: company_id} | ||
| |> Ecto.Changeset.change() | ||
| |> Repo.insert(on_conflict: :nothing, conflict_target: :company_id) | ||
| |> case do | ||
| {:ok, %{id: nil}} -> Repo.get_by!(ClassifierConfig, company_id: company_id) | ||
| {:ok, config} -> config | ||
| end | ||
|
|
||
| config -> | ||
| config | ||
| end | ||
| end | ||
|
|
||
| @doc """ | ||
| Returns a changeset for a classifier config, suitable for form building and validation. | ||
| """ | ||
| @spec change_classifier_config(ClassifierConfig.t(), map()) :: Ecto.Changeset.t() | ||
| def change_classifier_config(%ClassifierConfig{} = config, attrs \\ %{}) do | ||
| ClassifierConfig.changeset(config, attrs) | ||
| end | ||
|
|
||
| @doc """ | ||
| Updates a classifier config, encrypting the API token if provided. | ||
|
|
||
| Actor metadata (`user_id`, `actor_label`) is passed via `opts` and forwarded | ||
| to TrackedRepo for activity log emission. The `updated_by_id` field is derived | ||
| from `opts[:user_id]` server-side. | ||
| """ | ||
| @spec update_classifier_config(ClassifierConfig.t(), map(), keyword()) :: | ||
| {:ok, ClassifierConfig.t()} | {:error, Ecto.Changeset.t()} | ||
| def update_classifier_config(%ClassifierConfig{} = config, attrs, opts \\ []) do | ||
| changeset = ClassifierConfig.changeset(config, attrs) | ||
|
|
||
| # Set updated_by_id from actor opts, not from form attrs | ||
| changeset = | ||
| case Keyword.get(opts, :user_id) do | ||
| nil -> changeset | ||
| user_id -> Ecto.Changeset.put_change(changeset, :updated_by_id, user_id) | ||
| end | ||
|
|
||
| # Handle API token: Ecto casts "" to nil, so check raw params for explicit blank | ||
| changeset = | ||
| case Map.get(attrs, "api_token") || Map.get(attrs, :api_token) do | ||
| nil -> | ||
| changeset | ||
|
|
||
| "" -> | ||
| Ecto.Changeset.put_change(changeset, :api_token_encrypted, nil) | ||
|
|
||
| token when is_binary(token) -> | ||
| {:ok, encrypted} = Encryption.encrypt(token) | ||
| Ecto.Changeset.put_change(changeset, :api_token_encrypted, encrypted) | ||
| end | ||
|
|
||
| TrackedRepo.update( | ||
| changeset, | ||
| opts ++ [action: "classifier_config.updated"] | ||
| ) | ||
| end | ||
|
|
||
| @doc """ | ||
| Returns the current env-var defaults for display in the UI. | ||
| """ | ||
| @spec env_defaults() :: map() | ||
| def env_defaults do | ||
| %{ | ||
| url: Application.get_env(:ksef_hub, :invoice_classifier_url), | ||
| api_token_configured: Application.get_env(:ksef_hub, :invoice_classifier_api_token) != nil, | ||
| category_confidence_threshold: | ||
| Application.get_env(:ksef_hub, :category_confidence_threshold, 0.71), | ||
| tag_confidence_threshold: Application.get_env(:ksef_hub, :tag_confidence_threshold, 0.95) | ||
| } | ||
| end | ||
| end | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| defmodule KsefHub.ServiceConfig.ClassifierConfig do | ||
| @moduledoc """ | ||
| Per-company configuration for the invoice classifier sidecar. | ||
|
|
||
| When `enabled` is true, these values override the global env-var defaults. | ||
| When `enabled` is false (default), the app uses env vars from `runtime.exs`. | ||
| """ | ||
|
|
||
| use Ecto.Schema | ||
|
|
||
| import Ecto.Changeset | ||
|
|
||
| alias KsefHub.Companies.Company | ||
|
|
||
| @type t :: %__MODULE__{ | ||
| id: Ecto.UUID.t() | nil, | ||
| company_id: Ecto.UUID.t() | nil, | ||
| enabled: boolean(), | ||
| url: String.t() | nil, | ||
| api_token_encrypted: binary() | nil, | ||
| category_confidence_threshold: float() | nil, | ||
| tag_confidence_threshold: float() | nil, | ||
| updated_by_id: Ecto.UUID.t() | nil, | ||
| inserted_at: DateTime.t() | nil, | ||
| updated_at: DateTime.t() | nil | ||
| } | ||
|
|
||
| @primary_key {:id, :binary_id, autogenerate: true} | ||
| @foreign_key_type :binary_id | ||
| schema "classifier_configs" do | ||
| belongs_to :company, Company | ||
| field :enabled, :boolean, default: false | ||
| field :url, :string | ||
| field :api_token_encrypted, :binary | ||
| field :category_confidence_threshold, :float | ||
| field :tag_confidence_threshold, :float | ||
| field :updated_by_id, :binary_id | ||
|
|
||
| # Virtual field for accepting plaintext token in forms | ||
| field :api_token, :string, virtual: true | ||
|
|
||
| timestamps() | ||
| end | ||
|
|
||
| @doc "Builds a changeset for creating or updating a classifier config." | ||
| @spec changeset(t(), map()) :: Ecto.Changeset.t() | ||
| def changeset(config, attrs) do | ||
| config | ||
| |> cast(attrs, [ | ||
| :enabled, | ||
| :url, | ||
| :api_token, | ||
| :category_confidence_threshold, | ||
| :tag_confidence_threshold | ||
| ]) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| |> validate_when_enabled() | ||
| end | ||
|
|
||
| @spec validate_when_enabled(Ecto.Changeset.t()) :: Ecto.Changeset.t() | ||
| defp validate_when_enabled(changeset) do | ||
| if get_field(changeset, :enabled) do | ||
| changeset | ||
| |> validate_required([:url, :category_confidence_threshold, :tag_confidence_threshold]) | ||
| |> validate_url() | ||
| |> validate_number(:category_confidence_threshold, | ||
| greater_than: 0.0, | ||
| less_than: 1.0, | ||
| message: "must be between 0 and 1" | ||
| ) | ||
| |> validate_number(:tag_confidence_threshold, | ||
| greater_than: 0.0, | ||
| less_than: 1.0, | ||
| message: "must be between 0 and 1" | ||
| ) | ||
| else | ||
| changeset | ||
| end | ||
| end | ||
|
|
||
| @spec validate_url(Ecto.Changeset.t()) :: Ecto.Changeset.t() | ||
| defp validate_url(changeset) do | ||
| validate_change(changeset, :url, fn :url, url -> | ||
| case URI.parse(url) do | ||
| %URI{scheme: scheme, host: host} | ||
| when scheme in ["http", "https"] and is_binary(host) and host != "" -> | ||
| [] | ||
|
|
||
| _ -> | ||
| [url: "must be a valid HTTP(S) URL"] | ||
| end | ||
| end) | ||
| end | ||
| end | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle encryption failures without crashing the save flow.
Line 71 pattern-matches on
Encryption.encrypt/1. If encryption returns{:error, reason}, this raises and turns a recoverable validation/persistence error into a LiveView crash. Convert that failure into a normal changeset error instead.Possible fix
As per coding guidelines, "Use
withfor multi-step operations that can fail in Elixir code".🤖 Prompt for AI Agents