Bug Fix: Learning doesn't happen because exampleDirs is empty (#245)

* The bug is described in #215 (comment) * The bug is that `Config.getTrainingDirs` returns no training directories if config.learner == nil * Prior to #211 config.learner would be non-nil because we had to set the path of the RunMe logs * However, now that we no longer depend on RunMe logs config.learner could be nil and this would return no training directories. In which case learner.Reconcile would not attempt to save any examples * The fix is to allow config.Learner to be nil in GetTrainingDirs and return a suitable default. We also need to ensure the training directory gets created if it doesn't exist. * This is fixed by jlewi/monogo#23 which updates LocalFileHelper to create the directory if it doesn't exist. * My suspicion is that I never hit this bug because i originally created my ~/.foyle/training directory using a version of the code which wasn't using FileHelper and explicitly checked and created the directory. I suspect when I refactored the code to support saving examples to GCS thats when the code to ensure the directory exists got dropped.
jlewi · Sep 18, 2024 · 5f30c46 · 5f30c46
1 parent d67a104
commit 5f30c46
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 4 deletions.
diff --git a/app/go.mod b/app/go.mod
@@ -31,7 +31,7 @@ require (
 	github.com/honeycombio/otel-config-go v1.15.0
 	github.com/jlewi/foyle/protos/go v0.0.0-00010101000000-000000000000
 	github.com/jlewi/hydros v0.0.7-0.20240503183011-8f99ead373fb
-	github.com/jlewi/monogo v0.0.0-20240827035044-f4a3457933da
+	github.com/jlewi/monogo v0.0.0-20240918030136-e0ca1337aea4
 	github.com/liushuangls/go-anthropic/v2 v2.4.1
 	github.com/maxence-charriere/go-app/v9 v9.8.0
 	github.com/oklog/ulid/v2 v2.1.0

diff --git a/app/go.sum b/app/go.sum
@@ -396,6 +396,8 @@ github.com/jlewi/monogo v0.0.0-20240826232127-814ce2b6c0b9 h1:a+B3B/9suHv+8LfXKv
 github.com/jlewi/monogo v0.0.0-20240826232127-814ce2b6c0b9/go.mod h1:s3nTD+owHZ6b+F13JdSpXLtrAH35pOqdwdcZEZ/gwBc=
 github.com/jlewi/monogo v0.0.0-20240827035044-f4a3457933da h1:SBE/ERHbNWTrXG5SNMGYNKIQ+HlCniPzAKB35QfNIu4=
 github.com/jlewi/monogo v0.0.0-20240827035044-f4a3457933da/go.mod h1:s3nTD+owHZ6b+F13JdSpXLtrAH35pOqdwdcZEZ/gwBc=
+github.com/jlewi/monogo v0.0.0-20240918030136-e0ca1337aea4 h1:xQWqHY7FItn2FifFeQ5S/9kPgWpbGpN7qjH1IBihgaM=
+github.com/jlewi/monogo v0.0.0-20240918030136-e0ca1337aea4/go.mod h1:s3nTD+owHZ6b+F13JdSpXLtrAH35pOqdwdcZEZ/gwBc=
 github.com/jlewi/runme/v3 v3.0.0-20240524042602-a01f865c4617 h1:lyXrThCive3plQ/HyiYvklcdQ6F84bZ7DX+dMU01iik=
 github.com/jlewi/runme/v3 v3.0.0-20240524042602-a01f865c4617/go.mod h1:RSMUWGN5b1i4eXEAKbuksB/z8thptDXVKyZ6lRXMcJc=
 github.com/jlewi/runme/v3 v3.0.0-20240524044247-2657f0b08e0f h1:NFOdz6g8E44q5RPLXbUQBK+Ox+3tRqk+NG+rTXWHgcY=

diff --git a/app/pkg/config/config.go b/app/pkg/config/config.go
@@ -67,6 +67,9 @@ type Config struct {
 
 	Replicate *ReplicateConfig `json:"replicate,omitempty" yaml:"replicate,omitempty"`
 	Anthropic *AnthropicConfig `json:"anthropic,omitempty" yaml:"anthropic,omitempty"`
+
+	// TODO(jeremy): We should make the ConfigFile a private field to make it easier to overload e.g. in testing.
+	// GetConfig should set it to viper. Then in getters like GetExampleDirs we don't need to reference viper.
 }
 
 type LearnerConfig struct {
@@ -244,11 +247,11 @@ func (c *Config) GetSessionsDB() string {
 }
 
 func (c *Config) GetTrainingDirs() []string {
-	if c.Learner == nil {
-		return []string{}
+	dirs := make([]string, 0)
+	if c.Learner != nil {
+		dirs = append(dirs, c.Learner.ExampleDirs...)
 	}
 
-	dirs := c.Learner.ExampleDirs
 	// If dirs isn't set default to a local training directory
 	if len(dirs) == 0 {
 		dirs = []string{filepath.Join(c.GetConfigDir(), "training")}

diff --git a/app/pkg/learn/learner.go b/app/pkg/learn/learner.go
@@ -178,6 +178,12 @@ func (l *Learner) Reconcile(ctx context.Context, id string) error {
 
 	log.Info("Found new training example", "blockId", b.GetId())
 
+	if len(expectedFiles) == 0 {
+		cellsProcessed.WithLabelValues("noExampleFiles").Inc()
+		log.Error(err, "No training files found", "id", b.GetId())
+		return errors.Wrapf(err, "No training files found for example %s", b.GetId())
+	}
+
 	// TODO(jeremy): Should we take into account execution status when looking for mistakes?
 
 	// Deep copy the original message