Skip to content

Use activerecord store for the json column - #779

Merged
mattlindsey merged 6 commits into
AllYourBot:mainfrom
strivedi183:use-activerecord-store-for-the-json-column
Aug 26, 2026
Merged

Use activerecord store for the json column#779
mattlindsey merged 6 commits into
AllYourBot:mainfrom
strivedi183:use-activerecord-store-for-the-json-column

Conversation

@strivedi183

Copy link
Copy Markdown
Contributor

Use store accessors for user preferences

Toward #516 — covers users.preferences; the other jsonb columns are deliberately left for follow-ups (see below).

settings-ai-backends

Summary

users.preferences was written wholesale by both update paths, so saving one setting silently destroyed others — collapsing the sidebar wiped an explicitly chosen dark mode, and the settings form erased every key outside its permit list. Both paths now write per-key through typed store accessors, so whole-hash replacement is no longer expressible.

  • Sidebar toggle PATCHes only nav_closed; theme radios emit scalar dark_mode; regression tests prove sibling keys and nested preferences[:feature] survive every write. Saved preferences stop disappearing.
  • New "AI backends" section in settings: a Default / RubyLLM / Built-in SDK radio row per chat-capable driver, rows derived from APIService.drivers so new drivers appear automatically. The RubyLLM radio stays disabled until AIBackend::RubyLLM.supports_driver? covers that driver, so the UI never offers a choice routing can't honor. Choices are stored but not read yet — routing arrives with RubyLLM phased migration feature-flagged #775.
  • Resolution is symmetric: explicit per-driver choice → explicit use_ruby_llm opinion → site default (options.yml, ENV-controlled) → off. use_ruby_llm ships false and nothing in this repo turns it on; to try RubyLLM on one driver, opt in per user (user.features[:openai_ai_backend] = "ruby_llm"). The wiring commit message carries the recipe, and both override directions are pinned by tests.
  • Writes through user.features[] are minimized: only the <driver>_ai_backend choices plus use_ruby_llm are writable; every other option.yml flag raises on read and write, so deployer-scoped flags stay site-controlled.

This lays the per-user opinion storage that the RubyLLM migration (#740) needs — the settings UI, User#ruby_llm?(driver) reader, and ballot box are ready for #775 to consume. It also answers the per-user-toggle question raised on #744 (leverage User.preferences, keep the options.yml flag as the inheritable default).

Follow-ups

  • The remaining nine jsonb columns from Use activerecord store for the json columns #516 (credentials.properties, runs.required_action/last_error/tools/file_ids, steps.details/last_error, assistants.tools, messages.content_tool_calls) — whole-object writes need no partial-write pattern; each can adopt this pattern when it earns it.

The with_defaults getter override handed back a fresh hash on every
call. ActiveRecord store writers mutate that returned value in place,
so any write through it landed in a throwaway copy: saves became
silent no-ops and whole-hash updates remained the only working path —
which is how saving one setting erases others today.

The getter now returns the real stored hash. Defaults move into
hand-written dark_mode/nav_closed readers (dark_mode falls back to
"system"), and views read through those methods instead of raw hash
indexing.
Declares store_accessor :preferences for dark_mode and nav_closed,
replacing hand-written plumbing. Two behaviors are preserved via thin
overrides over the generated methods (the Rails-documented super
pattern):

- dark_mode falls back to "system" when unset (R2)
- nav_closed= coerces form strings ("true"/"false") to real
  booleans while preserving nil as unset (R1)

No serializer change was needed: custom-coded serialized columns
dispatch ActiveRecord's IndifferentHashAccessor, so accessor reads
tolerate both the symbol keys JsonSerializer loads and string keys
accessors write — verified by a save/reload round-trip test.

Scope note: this adopts the store pattern for users.preferences only.
The repo's other jsonb columns stay as-is deliberately —
credentials.properties, runs.required_action/last_error/tools/file_ids,
steps.details/last_error, assistants.tools, and
messages.content_tool_calls are each written as whole objects, so
partial-key writes buy nothing there. Follow-ups can adopt the same
pattern if their shapes ever change.
preferences was written wholesale by both update paths, erasing
sibling keys on every save, while its getter override returned a
fresh defaulted copy per call — making accessor-style writes silent
no-ops even where code tried them.

- preferences returns the real stored hash; defaults live in
  hand-written readers (dark_mode falls back to "system")
- store_accessor for dark_mode/nav_closed with writer coercion
  (form strings become real booleans; nil stays unset)
- declares use_ruby_llm site default plus ruby_llm?/ai_backend
  readers scoped strictly to driver picks (AllYourBot#740 groundwork);
  other features are unaffected
User::Features gives per-driver ai_backend names plus the use_ruby_llm
rollout switch a Hash-conventional, merge-safe read/write interface:

  user.features[:openai_ai_backend] = "sdk"
  user.features[:use_ruby_llm] = true

The box is deliberately narrow: only the AI-backend domain is
accessible — reads and writes alike raise Feature's typo error for
anything else (google_tools, registration, typos). Unset names read
nil (inherit); explicit choices persist distinctly. Derivation
excludes non-chat drivers (brave). feature.rb stays untouched;
interop across save/reload is verified including the nil-context
fallback.

Derived names are exposed via User::Features.derived_backend_names
for the settings UI and the RubyLLM wiring (AllYourBot#740/AllYourBot#775) to consume.
"Pick" is reserved for the user's settings-page action; these names
are derived, not chosen.
UsersController and Settings::PeopleController permit scalar params
and assign accessors individually — whole-hash replacement of
preferences is no longer expressible. The sidebar toggle and theme
radios emit matching scalar params in the same commit; wire-format
tests pin each view's payload, and regression tests prove sibling
keys survive every write.

Adds an "AI backends" section to settings: a three-way radio grid per
chat-capable driver (Default / RubyLLM / Built-in SDK), rows derived
from User::Features.derived_backend_names so new drivers appear
automatically. The RubyLLM radio stays disabled until
AIBackend::RubyLLM.supports_driver? covers the driver — checked
defensively, so it lights up automatically as the RubyLLM rollout
(AllYourBot#740/AllYourBot#775) lands; until then the UI never offers a choice that
routing cannot honor. Default displays what the site-wide setting
currently resolves to. PeopleController persists choices through
apply_backend_choices! against a freshly-loaded user; choices are
stored but not yet read by any backend — routing arrives with the
RubyLLM wiring.

Gentle introduction: use_ruby_llm ships false and stays false —
nothing in this repo (env, compose, CI) turns it on. To introduce
RubyLLM for one driver without touching the site default, opt in per
user: user.features[:openai_ai_backend] = "ruby_llm", or Settings >
AI backends > OpenAI > RubyLLM. Explicit choices always win over the
site default, in both directions; silent users keep the site-wide
SDK path. Coverage: the two "overrides ... site default, per driver"
tests in test/models/user/features_test.rb, plus the controller test
proving the settings UI stores the choice.
@strivedi183

Copy link
Copy Markdown
Contributor Author

Context for anyone connecting this to the earlier RubyLLM work:

@mattlindsey

Copy link
Copy Markdown
Collaborator

@strivedi183 Nice catch. Is this ready to merge?

@mattlindsey

mattlindsey commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Actually just to be picky:

  1. Alignment:
image
  1. Since the end user doesn't actually know what the 'Site Default' is set to (RubyLLM true or false) should we display that somewhere so they know what they're getting if they choose that checkbox?

  2. You are probably correct in this, but I'm a little unsure why there's an extra 'feature' level in the preferences field.

Why

  preferences:
   {dark_mode: "system", feature: {openai_ai_backend: nil, anthropic_ai_backend: nil, gemini_ai_backend: "sdk"}},

Instead of:

  preferences:
   {dark_mode: "system", openai_ai_backend: nil, anthropic_ai_backend: nil, gemini_ai_backend: "sdk"},

4. Strings should display both English and German. 

The settings strings added with the backend choices were hardcoded
English while the rest of the form renders through t(). Moves them
into en/de locales (heading, description with the current site-wide
resolution interpolated, choice labels, per-radio aria labels) and
renders through t() like its neighbors. German translations included;
driver names stay proper nouns.
@strivedi183

Copy link
Copy Markdown
Contributor Author

Actually just to be picky:

  1. Alignment:
image 2. Since the end user doesn't actually know what the 'Site Default' is set to (RubyLLM true or false) should we display that somewhere so they know what they're getting if they choose that checkbox? 3. You are probably correct in this, but I'm a little unsure why there's an extra 'feature' level in the preferences field.

Why

  preferences:
   {dark_mode: "system", feature: {openai_ai_backend: nil, anthropic_ai_backend: nil, gemini_ai_backend: "sdk"}},

Instead of:

  preferences:
   {dark_mode: "system", openai_ai_backend: nil, anthropic_ai_backend: nil, gemini_ai_backend: "sdk"},

4. Strings should display both English and German. 

Hey @mattlindsey , thanks for the quick reply!

For 1)

Screenshot 2026-08-25 at 8 00 55 AM

I uploaded an older screenshot and missed that I uploaded an older incorrect one, sorry about that.

For 2)

But also, I hope it answers the other question on the site default. I just copied the color theme pattern above it, with the dark/light/system.

I am ok changing it if you want, it was just a starting point so I am not committed to it.

For 3)

The feature level is already the way it works in the code base. It's the app's existing storage layout, not something introduced here. Feature.enabled? reads the nested key directly:

# app/models/feature.rb (unchanged from main)
Current.user&.preferences&.dig(:feature, feature.to_sym)

Any per-user opinion that Feature.enabled? can see has to live under that key. Meanwhile, store_accessor is flat by construction. It generates accessors for top-level keys only, so store_accessor :preferences, :"feature.openai_ai_backend" would create a literal flat key named "feature.openai_ai_backend", not nesting, see store_accessor in the Rails 8.1 API docs, or reproducible in a console:

User.store_accessor(:preferences, :"feature.openai_ai_backend")
u = User.new
u.public_send("feature.openai_ai_backend=", "sdk")
u.preferences # => {"feature.openai_ai_backend" => "sdk"}   # literal flat key, no nesting

So reaching preferences[:feature][name] can't be expressed with store_accessor, which is why the User::Features hand-writes []/[]= so writing to the nested location because that's where the existing reader looks.

That said, I do have some ideas to flatten this if you all think it is a good idea. The User::Features API (user.features[:openai_ai_backend]) stays the same either way, only the storage underneath moves:

a. Extract a dedicated JSONB column (features or feature_overrides) with top-level keys, and point the reader at it:

# user.feature_overrides => { openai_ai_backend: "sdk" }
# feature.rb reader becomes:
Current.user&.feature_overrides&.dig(feature.to_sym)

Keeps UI preferences and feature opinions in separate columns but needs a migration.

b. Flatten the keys inside preferences where the User::Features writes them flat:

# today:  preferences => { dark_mode: "system", feature: { openai_ai_backend: "sdk" } }
# flat:   preferences => { dark_mode: "system", openai_ai_backend: "sdk" }
# feature.rb reader becomes:
Current.user&.preferences&.dig(feature.to_sym)

No migration, but UI preferences and feature opinions share one hash. Happy to do either as a follow-up if you'd prefer flat storage.

For 4)

Fixed, the new strings now live in the en/de locales and render through t() like the rest of the form (German included; driver names stay proper nouns). Full disclosure, I had AI write the translations, so not sure if they are accurate.

Thank you for the feedback and let me know if there is anything else, either with my responses or the PR!

@mattlindsey

Copy link
Copy Markdown
Collaborator

Thanks for the explanations! Everything looks fine to me, so if you take it off Draft I'll merge it. Thanks again.

@strivedi183
strivedi183 marked this pull request as ready for review August 25, 2026 20:59
@mattlindsey
mattlindsey merged commit 0630fcb into AllYourBot:main Aug 26, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants