Fix nine silent-error defects: backup and auto-bank stat guards, dotenv warns instead of failing, glossary rows lifetime, langpack and ingest error wrapping, dead applyBanknote removed
This commit is contained in:
parent
e8d4ac0ed2
commit
5a97020f7e
10 changed files with 92 additions and 48 deletions
|
|
@ -8,8 +8,10 @@ package main
|
|||
// backup files without any special-casing here.
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"io/fs"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"time"
|
||||
|
|
@ -54,8 +56,12 @@ func preflightBackup(cfgPath string, w io.Writer) error {
|
|||
if err != nil {
|
||||
return err
|
||||
}
|
||||
// Only ABSENT means "fresh book". Any other stat error read that way silently skips the guard.
|
||||
if _, err := os.Stat(book.ProjectDB); err != nil {
|
||||
return nil // first run of a new book: no signed bank exists yet, nothing to back up
|
||||
if errors.Is(err, fs.ErrNotExist) {
|
||||
return nil // first run of a new book: no signed bank exists yet, nothing to back up
|
||||
}
|
||||
return fmt.Errorf("pre-flight guard: cannot stat the project database %s: %w", book.ProjectDB, err)
|
||||
}
|
||||
path, err := store.BackupSQLite(book.ProjectDB, backupDirFor(book.ProjectDB), backupStamp(time.Now()))
|
||||
if err != nil {
|
||||
|
|
|
|||
|
|
@ -2,7 +2,10 @@ package main
|
|||
|
||||
import (
|
||||
"bufio"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"io/fs"
|
||||
"os"
|
||||
"strings"
|
||||
)
|
||||
|
|
@ -44,15 +47,26 @@ func parseDotEnv(r io.Reader) []dotenvPair {
|
|||
// already-set variables. Keys come from backend/.env (gitignored); secrets never
|
||||
// end up in configs/the repo. Note: a variable set to an EMPTY string is treated
|
||||
// as unset and will be overridden (current behavior, frozen).
|
||||
func loadDotEnv(path string) {
|
||||
// It WARNS but never fails. A missing file is normal — the keys may come from the real environment —
|
||||
// but a file that exists and cannot be read, or a line os.Setenv refuses (an empty key from a `=VALUE`
|
||||
// typo), used to be swallowed and surfaced much later as an unexplained 401. Warning rather than
|
||||
// returning keeps that visible without taking down `status`/`report`/`export`, which are $0 read-only
|
||||
// projections that must not demand keys at all (D20.4), and lets the remaining valid lines still load.
|
||||
func loadDotEnv(path string, warn io.Writer) {
|
||||
f, err := os.Open(path)
|
||||
if err != nil {
|
||||
if !errors.Is(err, fs.ErrNotExist) {
|
||||
fmt.Fprintf(warn, "tmctl: %s exists but cannot be read (%v) — its keys are NOT loaded\n", path, err)
|
||||
}
|
||||
return
|
||||
}
|
||||
defer f.Close()
|
||||
for _, p := range parseDotEnv(f) {
|
||||
if os.Getenv(p.K) == "" {
|
||||
os.Setenv(p.K, p.V)
|
||||
if os.Getenv(p.K) != "" {
|
||||
continue
|
||||
}
|
||||
if err := os.Setenv(p.K, p.V); err != nil {
|
||||
fmt.Fprintf(warn, "tmctl: %s: malformed line ignored, key %q rejected (%v)\n", path, p.K, err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
package main
|
||||
|
||||
import (
|
||||
"io"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"reflect"
|
||||
|
|
@ -41,7 +42,7 @@ func TestLoadDotEnvDoesNotOverride(t *testing.T) {
|
|||
}
|
||||
t.Setenv("TM_TEST_SET", "from_env")
|
||||
t.Setenv("TM_TEST_UNSET", "") // set to EMPTY counts as unset (frozen behaviour)
|
||||
loadDotEnv(path)
|
||||
loadDotEnv(path, io.Discard)
|
||||
if got := os.Getenv("TM_TEST_SET"); got != "from_env" {
|
||||
t.Fatalf("an already-set variable must win over .env, got %q", got)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -57,8 +57,8 @@ func run() error {
|
|||
return err
|
||||
}
|
||||
|
||||
loadDotEnv(filepath.Join(filepath.Dir(inv.cfgPath), ".env"))
|
||||
loadDotEnv(".env")
|
||||
loadDotEnv(filepath.Join(filepath.Dir(inv.cfgPath), ".env"), os.Stderr)
|
||||
loadDotEnv(".env", os.Stderr)
|
||||
|
||||
ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
|
||||
defer stop()
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import (
|
|||
"archive/zip"
|
||||
"bytes"
|
||||
"encoding/xml"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/url"
|
||||
|
|
@ -826,7 +827,7 @@ func extractXHTML(data []byte) (string, []RubyReading, error) {
|
|||
for {
|
||||
tt := z.Next()
|
||||
if tt == html.ErrorToken {
|
||||
if err := z.Err(); err != io.EOF {
|
||||
if err := z.Err(); !errors.Is(err, io.EOF) {
|
||||
return "", nil, fmt.Errorf("tokenize xhtml: %w", err)
|
||||
}
|
||||
break
|
||||
|
|
|
|||
|
|
@ -334,8 +334,13 @@ func LoadWithOverlay(root, src, tgt, overlayRoot string) (*Pack, error) {
|
|||
// besides the two expected subdirectories, is an operator mistake — and the fold below would swallow it
|
||||
// in silence. langpack_extend is an EXPLICIT opt-in in book.yaml: if it is set, the caller asserts that a
|
||||
// private canon exists, so a missing/typo'd path must stop the load, never degrade recall quietly.
|
||||
if fi, serr := os.Stat(overlayRoot); serr != nil || !fi.IsDir() {
|
||||
return nil, fmt.Errorf("langpack %q overlay root %s is not a readable directory (langpack_extend is an explicit opt-in — a missing or mistyped path would silently drop the book's private canon): %v", pair, overlayRoot, serr)
|
||||
const overlayWhy = "langpack_extend is an explicit opt-in — a missing or mistyped path would silently drop the book's private canon"
|
||||
fi, serr := os.Stat(overlayRoot)
|
||||
if serr != nil {
|
||||
return nil, fmt.Errorf("langpack %q overlay root %s is not readable (%s): %w", pair, overlayRoot, overlayWhy, serr)
|
||||
}
|
||||
if !fi.IsDir() {
|
||||
return nil, fmt.Errorf("langpack %q overlay root %s is not a directory (%s)", pair, overlayRoot, overlayWhy)
|
||||
}
|
||||
rootEnts, rerr := os.ReadDir(overlayRoot)
|
||||
if rerr != nil {
|
||||
|
|
|
|||
|
|
@ -291,21 +291,16 @@ var bankTokenBudget = bankMaxLines*bankWorstLineTokens + EstimateTokens(bankSepa
|
|||
// The literal below IS the arithmetic — it is measured, not asserted (see TestBankTokenBudgetIsDerived).
|
||||
var bankWorstLineTokens = EstimateTokens(strings.Repeat("蛊", 8) + "\t" + strings.Repeat("я", 40) + "\tnickname\n")
|
||||
|
||||
// applyBanknote slices the banknote block off a translator draft (integration points 1–5, 8). It runs
|
||||
// ONLY on the translator role with the channel enabled — the editor never emits banknotes, and a normal
|
||||
// draft has no separator (the slice is then a no-op that returns the raw text unchanged, so a
|
||||
// banknote-OFF run is byte-identical). When a ⟦TM-BANK-v1⟧ block IS present it returns the CLEANED
|
||||
// translation (what classify/coverage/echo/editor/export all see — points 2/3/4) plus that same cleaned
|
||||
// text as the `stripped` export to commit as a derived checkpoint (point 8), and the per-chunk telemetry
|
||||
// (point 10). Candidates are PARSED/accepted only under finish=="stop" (the finish=stop-only gate §4б —
|
||||
// a truncated block is not trusted); the slice itself runs regardless, so the length/echo classify never
|
||||
// sees the footnote's Han src column. Deterministic (a pure function of the raw text + gate state).
|
||||
func (r *Runner) applyBanknote(role, rawText, finish, chunkSource string) (clean, stripped string, flags bankFlags) {
|
||||
clean, stripped, flags, _ = r.applyBanknoteWithEntries(role, rawText, finish, chunkSource)
|
||||
return clean, stripped, flags
|
||||
}
|
||||
|
||||
// applyBanknoteWithEntries is applyBanknote plus the PARSED entries — the WHAT itself.
|
||||
// applyBanknoteWithEntries slices the banknote block off a translator draft (integration points 1–5, 8)
|
||||
// and returns the PARSED entries — the WHAT itself. It runs ONLY on the translator role with the channel
|
||||
// enabled — the editor never emits banknotes, and a normal draft has no separator (the slice is then a
|
||||
// no-op that returns the raw text unchanged, so a banknote-OFF run is byte-identical). When a ⟦TM-BANK-v1⟧
|
||||
// block IS present it returns the CLEANED translation (what classify/coverage/echo/editor/export all see —
|
||||
// points 2/3/4) plus that same cleaned text as the `stripped` export to commit as a derived checkpoint
|
||||
// (point 8), and the per-chunk telemetry (point 10). Candidates are PARSED/accepted only under
|
||||
// finish=="stop" (the finish=stop-only gate §4б — a truncated block is not trusted); the slice itself runs
|
||||
// regardless, so the length/echo classify never sees the footnote's Han src column. Deterministic (a pure
|
||||
// function of the raw text + gate state).
|
||||
//
|
||||
// Until the mini-run of 25.07 the entries were dropped into `_` here (D39.36): the model proposed a
|
||||
// rendering for every new term, the parser accepted it, and the code kept only the counters. The owner
|
||||
|
|
|
|||
|
|
@ -2,7 +2,9 @@ package pipeline
|
|||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io/fs"
|
||||
"os"
|
||||
"sort"
|
||||
"strings"
|
||||
|
|
@ -381,8 +383,12 @@ func (r *Runner) autoBankPath() string { return r.Book.ProjectDB + ".auto-bank.y
|
|||
// whatever the emission wrote (auto/draft). A file hand-edited to say `approved` is refused loudly,
|
||||
// because that would be a signature nobody gave.
|
||||
func (r *Runner) loadAutoBank(signed []store.GlossaryEntry) (rows []store.GlossaryEntry, dropped []string, err error) {
|
||||
// Only ABSENT means "auto mode has not run"; an unreadable file must not drop the mined rows.
|
||||
if _, serr := os.Stat(r.autoBankPath()); serr != nil {
|
||||
return nil, nil, nil // absent → the auto mode has not run yet (or the book never uses it)
|
||||
if errors.Is(serr, fs.ErrNotExist) {
|
||||
return nil, nil, nil // the auto mode has not run yet (or the book never uses it)
|
||||
}
|
||||
return nil, nil, fmt.Errorf("pipeline: stat auto-bank %s: %w", r.autoBankPath(), serr)
|
||||
}
|
||||
entries, err := membank.LoadGlossarySeed(r.autoBankPath())
|
||||
if err != nil {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
package store
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"errors"
|
||||
)
|
||||
|
|
@ -135,25 +136,10 @@ func (s *Store) ReplaceBank(bookID string, entries []GlossaryEntry, voices []Voi
|
|||
defer tx.Rollback()
|
||||
|
||||
// Snapshot the prior (src,sense,window)→dst to detect editorial-time changes (B1).
|
||||
prior := map[[4]any]string{}
|
||||
rows, err := tx.QueryContext(ctx, `SELECT src, sense, since_ch, until_ch, dst FROM glossary WHERE book_id = ?`, bookID)
|
||||
prior, err := priorRenderings(ctx, tx, bookID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
for rows.Next() {
|
||||
var src, sense, dst string
|
||||
var since, until int
|
||||
if err := rows.Scan(&src, &sense, &since, &until, &dst); err != nil {
|
||||
rows.Close()
|
||||
return err
|
||||
}
|
||||
prior[[4]any{src, sense, since, until}] = dst
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
rows.Close()
|
||||
return err
|
||||
}
|
||||
rows.Close()
|
||||
|
||||
for _, table := range []string{"glossary_aliases", "glossary", "voice_profiles", "address_pairs"} {
|
||||
if _, err := tx.ExecContext(ctx, `DELETE FROM `+table+` WHERE book_id = ?`, bookID); err != nil {
|
||||
|
|
@ -404,3 +390,24 @@ func boolToInt(b bool) int {
|
|||
}
|
||||
return 0
|
||||
}
|
||||
|
||||
// priorRenderings reads the (src,sense,window)→dst map a replace overwrites. Separate function so the
|
||||
// rows close before the caller's DELETEs run on the same tx — a tx holds one connection.
|
||||
func priorRenderings(ctx context.Context, tx *sql.Tx, bookID string) (map[[4]any]string, error) {
|
||||
rows, err := tx.QueryContext(ctx, `SELECT src, sense, since_ch, until_ch, dst FROM glossary WHERE book_id = ?`, bookID)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
defer rows.Close()
|
||||
|
||||
prior := map[[4]any]string{}
|
||||
for rows.Next() {
|
||||
var src, sense, dst string
|
||||
var since, until int
|
||||
if err := rows.Scan(&src, &sense, &since, &until, &dst); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
prior[[4]any{src, sense, since, until}] = dst
|
||||
}
|
||||
return prior, rows.Err()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -37,13 +37,22 @@ func TestMigrationUpgradesAnOlderDatabase(t *testing.T) {
|
|||
t.Fatal(err)
|
||||
}
|
||||
// Premise check: at the OLD head the pack-19 schema must genuinely be absent, or this test would
|
||||
// prove nothing about upgrading.
|
||||
if _, qerr := raw.Query(`SELECT n_voice_flags FROM retrieval_state`); qerr == nil {
|
||||
t.Fatal("the old schema already has the pack-19 columns — the upgrade is not being exercised")
|
||||
}
|
||||
if _, qerr := raw.Query(`SELECT 1 FROM voice_profiles`); qerr == nil {
|
||||
t.Fatal("the old schema already has voice_profiles — the upgrade is not being exercised")
|
||||
// prove nothing about upgrading. A successful query means it is already there.
|
||||
mustNotQuery := func(query, why string) {
|
||||
t.Helper()
|
||||
rows, qerr := raw.Query(query)
|
||||
if qerr != nil {
|
||||
return // the column/table is absent — exactly the premise this test needs
|
||||
}
|
||||
defer rows.Close()
|
||||
if rows.Err() == nil {
|
||||
t.Fatal(why)
|
||||
}
|
||||
}
|
||||
mustNotQuery(`SELECT n_voice_flags FROM retrieval_state`,
|
||||
"the old schema already has the pack-19 columns — the upgrade is not being exercised")
|
||||
mustNotQuery(`SELECT 1 FROM voice_profiles`,
|
||||
"the old schema already has voice_profiles — the upgrade is not being exercised")
|
||||
if err := raw.Close(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue