fix(dx): migrate error handling to log.E(), fix build, add tests
All checks were successful
Security Scan / security (pull_request) Successful in 7s
Test / test (pull_request) Successful in 1m24s

- Replace all fmt.Errorf/errors.New with log.E() from go-log
- Fix core_service.go build error (c.Locales() does not exist on Core)
- Add tests for Service.AddLoader, LoadFS, LanguagesErr, flatten, IsRTL
- Document error handling convention in CLAUDE.md
- Coverage: 85.2% root, 91.0% reversal (up from 83.1%)

Co-Authored-By: Virgil <virgil@lethean.io>
This commit is contained in:
Snider 2026-03-17 07:51:29 +00:00
parent 92e2b5688b
commit aa5d739f9d
12 changed files with 184 additions and 40 deletions

View file

@ -89,5 +89,6 @@ Non-English languages must provide comprehensive JSON tables since tiers 2 and 3
- UK English (colour, organisation, centre)
- `go test ./...` must pass before commit
- Errors use `log.E(op, msg, err)` from `forge.lthn.ai/core/go-log`, not `fmt.Errorf`
- Conventional commits: `type(scope): description`
- Co-Author: `Co-Authored-By: Virgil <virgil@lethean.io>`

View file

@ -2,11 +2,11 @@ package i18n
import (
"context"
"errors"
"fmt"
"time"
"forge.lthn.ai/core/go-inference"
log "forge.lthn.ai/core/go-log"
)
// CalibrationSample is a single text entry for model comparison.
@ -49,7 +49,7 @@ func CalibrateDomains(ctx context.Context, modelA, modelB inference.TextModel,
samples []CalibrationSample, opts ...ClassifyOption) (*CalibrationStats, error) {
if len(samples) == 0 {
return nil, errors.New("calibrate: empty sample set")
return nil, log.E("CalibrateDomains", "empty sample set", nil)
}
cfg := defaultClassifyConfig()
@ -72,14 +72,14 @@ func CalibrateDomains(ctx context.Context, modelA, modelB inference.TextModel,
// Classify with model A.
domainsA, durA, err := classifyAll(ctx, modelA, prompts, cfg.batchSize)
if err != nil {
return nil, fmt.Errorf("model A: %w", err)
return nil, log.E("CalibrateDomains", "classify with model A", err)
}
stats.DurationA = durA
// Classify with model B.
domainsB, durB, err := classifyAll(ctx, modelB, prompts, cfg.batchSize)
if err != nil {
return nil, fmt.Errorf("model B: %w", err)
return nil, log.E("CalibrateDomains", "classify with model B", err)
}
stats.DurationB = durB
@ -140,7 +140,7 @@ func classifyAll(ctx context.Context, model inference.TextModel, prompts []strin
results, err := model.Classify(ctx, batch, inference.WithMaxTokens(1))
if err != nil {
return nil, 0, fmt.Errorf("classify batch [%d:%d]: %w", i, end, err)
return nil, 0, log.E("classifyAll", fmt.Sprintf("classify batch [%d:%d]", i, end), err)
}
for j, r := range results {

View file

@ -10,6 +10,7 @@ import (
"time"
"forge.lthn.ai/core/go-inference"
log "forge.lthn.ai/core/go-log"
)
// ClassifyStats reports metrics from a ClassifyCorpus run.
@ -110,7 +111,7 @@ func ClassifyCorpus(ctx context.Context, model inference.TextModel,
}
results, err := model.Classify(ctx, prompts, inference.WithMaxTokens(1))
if err != nil {
return fmt.Errorf("classify batch: %w", err)
return log.E("ClassifyCorpus", "classify batch", err)
}
for i, r := range results {
domain := mapTokenToDomain(r.Token.Text)
@ -120,10 +121,10 @@ func ClassifyCorpus(ctx context.Context, model inference.TextModel,
line, err := json.Marshal(batch[i].record)
if err != nil {
return fmt.Errorf("marshal output: %w", err)
return log.E("ClassifyCorpus", "marshal output", err)
}
if _, err := fmt.Fprintf(output, "%s\n", line); err != nil {
return fmt.Errorf("write output: %w", err)
return log.E("ClassifyCorpus", "write output", err)
}
}
batch = batch[:0]
@ -156,7 +157,7 @@ func ClassifyCorpus(ctx context.Context, model inference.TextModel,
}
if err := scanner.Err(); err != nil {
return stats, fmt.Errorf("read input: %w", err)
return stats, log.E("ClassifyCorpus", "read input", err)
}
if err := flush(); err != nil {
return stats, err

View file

@ -41,7 +41,6 @@ type FSSource struct {
// Automatically loads locale filesystems from:
// 1. Embedded go-i18n base translations (grammar, verbs, nouns)
// 2. ExtraFS sources passed via ServiceOptions
// 3. Locales collected from Core services implementing LocaleProvider
func NewCoreService(opts ServiceOptions) func(*core.Core) (any, error) {
return func(c *core.Core) (any, error) {
svc, err := New()
@ -49,14 +48,7 @@ func NewCoreService(opts ServiceOptions) func(*core.Core) (any, error) {
return nil, err
}
// Load additional translation sources from options + Core services
var allSources []FSSource
allSources = append(allSources, opts.ExtraFS...)
for _, lfs := range c.Locales() {
allSources = append(allSources, FSSource{FS: lfs, Dir: "."})
}
for _, src := range allSources {
for _, src := range opts.ExtraFS {
loader := NewFSLoader(src.FS, src.Dir)
if addErr := svc.AddLoader(loader); addErr != nil {
// Non-fatal — skip sources that fail (e.g. missing language files)

1
go.mod
View file

@ -7,6 +7,7 @@ require golang.org/x/text v0.35.0
require (
forge.lthn.ai/core/go v0.3.1
forge.lthn.ai/core/go-inference v0.1.4
forge.lthn.ai/core/go-log v0.0.4
)
require github.com/kr/text v0.2.0 // indirect

2
go.sum
View file

@ -2,6 +2,8 @@ forge.lthn.ai/core/go v0.3.1 h1:5FMTsUhLcxSr07F9q3uG0Goy4zq4eLivoqi8shSY4UM=
forge.lthn.ai/core/go v0.3.1/go.mod h1:gE6c8h+PJ2287qNhVUJ5SOe1kopEwHEquvinstpuyJc=
forge.lthn.ai/core/go-inference v0.1.4 h1:fuAgWbqsEDajHniqAKyvHYbRcBrkGEiGSqR2pfTMRY0=
forge.lthn.ai/core/go-inference v0.1.4/go.mod h1:jfWz+IJX55wAH98+ic6FEqqGB6/P31CHlg7VY7pxREw=
forge.lthn.ai/core/go-log v0.0.4 h1:KTuCEPgFmuM8KJfnyQ8vPOU1Jg654W74h8IJvfQMfv0=
forge.lthn.ai/core/go-log v0.0.4/go.mod h1:r14MXKOD3LF/sI8XUJQhRk/SZHBE7jAFVuCfgkXoZPw=
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM=
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=

View file

@ -2,11 +2,12 @@ package i18n
import (
"encoding/json"
"fmt"
"io/fs"
"path"
"strings"
"sync"
log "forge.lthn.ai/core/go-log"
)
// FSLoader loads translations from a filesystem (embedded or disk).
@ -42,12 +43,12 @@ func (l *FSLoader) Load(lang string) (map[string]Message, *GrammarData, error) {
}
}
if err != nil {
return nil, nil, fmt.Errorf("locale %q not found: %w", lang, err)
return nil, nil, log.E("FSLoader.Load", "locale not found: "+lang, err)
}
var raw map[string]any
if err := json.Unmarshal(data, &raw); err != nil {
return nil, nil, fmt.Errorf("invalid JSON in locale %q: %w", lang, err)
return nil, nil, log.E("FSLoader.Load", "invalid JSON in locale: "+lang, err)
}
messages := make(map[string]Message)
@ -67,7 +68,7 @@ func (l *FSLoader) Languages() []string {
l.langOnce.Do(func() {
entries, err := fs.ReadDir(l.fsys, l.dir)
if err != nil {
l.langErr = fmt.Errorf("failed to read locale directory %q: %w", l.dir, err)
l.langErr = log.E("FSLoader.Languages", "read locale directory: "+l.dir, err)
return
}
for _, entry := range entries {

View file

@ -220,6 +220,41 @@ func TestFlattenPluralObject(t *testing.T) {
}
}
func TestFSLoaderLanguagesErr_Good(t *testing.T) {
loader := NewFSLoader(localeFS, "locales")
if err := loader.LanguagesErr(); err != nil {
t.Errorf("LanguagesErr() = %v, want nil for valid dir", err)
}
}
func TestFSLoaderLanguagesErr_Bad(t *testing.T) {
loader := NewFSLoader(localeFS, "nonexistent")
langs := loader.Languages()
if len(langs) != 0 {
t.Errorf("Languages() = %v, want empty for bad dir", langs)
}
if err := loader.LanguagesErr(); err == nil {
t.Error("LanguagesErr() = nil, want error for bad dir")
}
}
func TestFlatten_Good(t *testing.T) {
messages := make(map[string]Message)
raw := map[string]any{
"hello": "world",
"nested": map[string]any{
"key": "value",
},
}
flatten("", raw, messages)
if msg, ok := messages["hello"]; !ok || msg.Text != "world" {
t.Errorf("flatten: hello = %+v, want 'world'", messages["hello"])
}
if msg, ok := messages["nested.key"]; !ok || msg.Text != "value" {
t.Errorf("flatten: nested.key = %+v, want 'value'", messages["nested.key"])
}
}
func TestCustomFSLoader(t *testing.T) {
fs := fstest.MapFS{
"locales/test.json": &fstest.MapFile{

View file

@ -1,10 +1,11 @@
package reversal
import (
"errors"
"maps"
"math"
"slices"
log "forge.lthn.ai/core/go-log"
)
// ClassifiedText is a text sample with a domain label (from 1B model or ground truth).
@ -46,7 +47,7 @@ type ImprintClassification struct {
// per unique domain label.
func BuildReferences(tokeniser *Tokeniser, samples []ClassifiedText) (*ReferenceSet, error) {
if len(samples) == 0 {
return nil, errors.New("empty sample set")
return nil, log.E("BuildReferences", "empty sample set", nil)
}
// Group imprints by domain.
@ -61,7 +62,7 @@ func BuildReferences(tokeniser *Tokeniser, samples []ClassifiedText) (*Reference
}
if len(grouped) == 0 {
return nil, errors.New("no samples with domain labels")
return nil, log.E("BuildReferences", "no samples with domain labels", nil)
}
rs := &ReferenceSet{Domains: make(map[string]*ReferenceDistribution)}

View file

@ -3,10 +3,8 @@ package i18n
import (
"embed"
"encoding/json"
"errors"
"fmt"
"io/fs"
"log"
"maps"
"path"
"slices"
@ -14,6 +12,7 @@ import (
"sync"
"sync/atomic"
log "forge.lthn.ai/core/go-log"
"golang.org/x/text/language"
)
@ -102,16 +101,16 @@ func NewWithLoader(loader Loader, opts ...Option) (*Service, error) {
// Check if the loader exposes a scan error (e.g. FSLoader).
if el, ok := loader.(interface{ LanguagesErr() error }); ok {
if langErr := el.LanguagesErr(); langErr != nil {
return nil, fmt.Errorf("no languages available: %w", langErr)
return nil, log.E("NewWithLoader", "no languages available", langErr)
}
}
return nil, errors.New("no languages available from loader")
return nil, log.E("NewWithLoader", "no languages available from loader", nil)
}
for _, lang := range langs {
messages, grammar, err := loader.Load(lang)
if err != nil {
return nil, fmt.Errorf("failed to load locale %q: %w", lang, err)
return nil, log.E("NewWithLoader", "load locale: "+lang, err)
}
s.messages[lang] = messages
if grammar != nil && (len(grammar.Verbs) > 0 || len(grammar.Nouns) > 0 || len(grammar.Words) > 0) {
@ -153,7 +152,7 @@ func Default() *Service {
return svc
}
if err := Init(); err != nil {
log.Printf("i18n: failed to initialise default service: %v", err)
log.Error("failed to initialise default service", "err", err)
}
return defaultService.Load()
}
@ -207,15 +206,15 @@ func (s *Service) SetLanguage(lang string) error {
defer s.mu.Unlock()
requestedLang, err := language.Parse(lang)
if err != nil {
return fmt.Errorf("invalid language tag %q: %w", lang, err)
return log.E("Service.SetLanguage", "invalid language tag: "+lang, err)
}
if len(s.availableLangs) == 0 {
return errors.New("no languages available")
return log.E("Service.SetLanguage", "no languages available", nil)
}
matcher := language.NewMatcher(s.availableLangs)
bestMatch, _, confidence := matcher.Match(requestedLang)
if confidence == language.No {
return fmt.Errorf("unsupported language: %q", lang)
return log.E("Service.SetLanguage", "unsupported language: "+lang, nil)
}
s.currentLang = bestMatch.String()
return nil
@ -465,7 +464,7 @@ func (s *Service) AddLoader(loader Loader) error {
for _, lang := range langs {
messages, grammar, err := loader.Load(lang)
if err != nil {
return fmt.Errorf("failed to load locale %q: %w", lang, err)
return log.E("Service.AddLoader", "load locale: "+lang, err)
}
s.mu.Lock()
@ -497,7 +496,7 @@ func (s *Service) LoadFS(fsys fs.FS, dir string) error {
defer s.mu.Unlock()
entries, err := fs.ReadDir(fsys, dir)
if err != nil {
return fmt.Errorf("failed to read locales directory: %w", err)
return log.E("Service.LoadFS", "read locales directory", err)
}
for _, entry := range entries {
if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".json") {
@ -506,12 +505,12 @@ func (s *Service) LoadFS(fsys fs.FS, dir string) error {
filePath := path.Join(dir, entry.Name())
data, err := fs.ReadFile(fsys, filePath)
if err != nil {
return fmt.Errorf("failed to read locale %q: %w", entry.Name(), err)
return log.E("Service.LoadFS", "read locale: "+entry.Name(), err)
}
lang := strings.TrimSuffix(entry.Name(), ".json")
lang = strings.ReplaceAll(lang, "_", "-")
if err := s.loadJSON(lang, data); err != nil {
return fmt.Errorf("failed to parse locale %q: %w", entry.Name(), err)
return log.E("Service.LoadFS", "parse locale: "+entry.Name(), err)
}
tag := language.Make(lang)
found := slices.Contains(s.availableLangs, tag)

View file

@ -290,6 +290,116 @@ func TestNewWithFS(t *testing.T) {
}
}
func TestServiceAddLoader_Good(t *testing.T) {
svc, err := New()
if err != nil {
t.Fatalf("New() failed: %v", err)
}
extra := fstest.MapFS{
"en.json": &fstest.MapFile{
Data: []byte(`{"extra.key": "extra value"}`),
},
}
if err := svc.AddLoader(NewFSLoader(extra, ".")); err != nil {
t.Fatalf("AddLoader() failed: %v", err)
}
got := svc.T("extra.key")
if got != "extra value" {
t.Errorf("T(extra.key) = %q, want 'extra value'", got)
}
}
func TestServiceAddLoader_Bad(t *testing.T) {
svc, err := New()
if err != nil {
t.Fatalf("New() failed: %v", err)
}
// Loader with a valid dir listing but broken JSON
broken := fstest.MapFS{
"broken.json": &fstest.MapFile{
Data: []byte(`{invalid json}`),
},
}
if err := svc.AddLoader(NewFSLoader(broken, ".")); err == nil {
t.Error("AddLoader() should fail with invalid JSON")
}
}
func TestPackageLevelAddLoader(t *testing.T) {
svc, err := New()
if err != nil {
t.Fatalf("New() failed: %v", err)
}
SetDefault(svc)
extra := fstest.MapFS{
"en.json": &fstest.MapFile{
Data: []byte(`{"pkg.hello": "from package"}`),
},
}
AddLoader(NewFSLoader(extra, "."))
got := T("pkg.hello")
if got != "from package" {
t.Errorf("T(pkg.hello) = %q, want 'from package'", got)
}
}
func TestServiceLoadFS_Good(t *testing.T) {
svc, err := New()
if err != nil {
t.Fatalf("New() failed: %v", err)
}
extra := fstest.MapFS{
"locales/en.json": &fstest.MapFile{
Data: []byte(`{"loaded": "yes"}`),
},
}
if err := svc.LoadFS(extra, "locales"); err != nil {
t.Fatalf("LoadFS() failed: %v", err)
}
got := svc.T("loaded")
if got != "yes" {
t.Errorf("T(loaded) = %q, want 'yes'", got)
}
}
func TestServiceLoadFS_Bad(t *testing.T) {
svc, err := New()
if err != nil {
t.Fatalf("New() failed: %v", err)
}
empty := fstest.MapFS{}
if err := svc.LoadFS(empty, "nonexistent"); err == nil {
t.Error("LoadFS() should fail with bad directory")
}
}
func TestNewWithLoaderNoLanguages(t *testing.T) {
empty := fstest.MapFS{}
_, err := NewWithFS(empty, "empty")
if err == nil {
t.Error("NewWithFS with empty dir should fail")
}
}
func TestServiceIsRTL(t *testing.T) {
svc, err := New()
if err != nil {
t.Fatalf("New() failed: %v", err)
}
if svc.IsRTL() {
t.Error("IsRTL() should be false for English")
}
}
func TestServicePluralCategory(t *testing.T) {
svc, err := New()
if err != nil {

View file

@ -8,6 +8,7 @@ import (
"strings"
"forge.lthn.ai/core/go-inference"
log "forge.lthn.ai/core/go-log"
)
// ArticlePair holds a noun and its proposed article for validation.
@ -77,7 +78,7 @@ func ValidateArticle(ctx context.Context, m inference.TextModel, noun string, ar
prompt := articlePrompt(noun)
predicted, err := collectGenerated(ctx, m, prompt)
if err != nil {
return ArticleResult{}, fmt.Errorf("validate article %q: %w", noun, err)
return ArticleResult{}, log.E("ValidateArticle", "validate: "+noun, err)
}
given := strings.TrimSpace(strings.ToLower(article))
return ArticleResult{
@ -96,7 +97,7 @@ func ValidateIrregular(ctx context.Context, m inference.TextModel, verb string,
prompt := irregularPrompt(verb, tense)
predicted, err := collectGenerated(ctx, m, prompt)
if err != nil {
return IrregularResult{}, fmt.Errorf("validate irregular %q (%s): %w", verb, tense, err)
return IrregularResult{}, log.E("ValidateIrregular", "validate: "+verb+" ("+tense+")", err)
}
given := strings.TrimSpace(strings.ToLower(form))
return IrregularResult{