[ggmarshall]

• I have reviewed all the code in this pull request and deem it of the standard expected for this project.

Since #48, TextDB.on() results share nested objects with the database cache and are flagged read-only. But the flag is only checked by __setitem__, __setattr__ and |=. I reproduced these on the LEGEND dataprod config DB:

• pop, update, setdefault, del, clear and every list method (append, sort, …) skip the check and silently change the cache. A pop or append on db.on(t1) shows up in a later db.on(t2) in the same process. This bites long-lived processes (Snakemake, notebooks, dashboards).
• The flag lands on cached files only once an on() has run, so db["file"]...[k] = v succeeds or raises depending on call order.
• AttrsDict(x) and TextDB.map() propagate False down, which unfreezes the shared cache.
• Unpickling resets the flag (e.g. pre-compiled catalog caches), so objects are writable but shared.

This PR:

• checks the flag in every dict mutator;
• converts lists inside read-only AttrsDicts to a ReadOnlyList(list). It is still a list for isinstance, JSON and +, and YAML dumpers are registered so it dumps as a plain list;
• freezes files when TextDB loads them;
• makes the constructor and map() only ever propagate True;
• keeps the flag through pickle, while copy.copy/copy.deepcopy return writable copies;
• updates the on() docstring.

Writable use of AttrsDict and Props.read_from is unchanged.

Validation.

• New tests/test_readonly.py: 28 cases. On main, 23 fail and only the 3 already-guarded mutators plus the copy/usability checks pass. Full suite: 58 passed. pre-commit run -a and nox -s pylint (10.00/10) pass.
• LEGEND configs: TextDB.on() for all 3,643 (timestamp × category) lookups over dataprod/config, dataprod/overrides/*, datasets/statuses and hardware/configuration/channelmaps gives results identical to 1.4.0, and runs faster: 254 s vs 283 s, since freezing skips subtrees that are already frozen.
• Downstream suites against this branch: legend-dataflow (128 passed), legend-dataflow-scripts (83 passed) and legend-data-monitor (190 passed), all the same as with 1.4.0. A raw-tier replay with legend-dataflow gives identical output.

Known limit: plain dicts nested in lists of lists aren't converted to AttrsDict, so they can't be frozen.

Related: #62, legend-exp/legend-dataflow-scripts#59.

🤖 Generated with Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[codecov]

Codecov Report

❌ Patch coverage is 92.85714% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.46%. Comparing base (5b17622) to head (35211c1).
⚠️ Report is 25 commits behind head on main.

Files with missing lines	Patch %	Lines
src/dbetto/attrsdict.py	92.59%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #63      +/-   ##
==========================================
+ Coverage   91.72%   92.46%   +0.74%     
==========================================
  Files           6        6              
  Lines         568      664      +96     
==========================================
+ Hits          521      614      +93     
- Misses         47       50       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[gipert]

I rebased this branch on main (dropping the merge commit) and force-pushed. The only conflict was in AttrsDict.__ior__: main now updates the dict in place (#65), so the resolved version keeps that and calls _check_writable() first.

I also pushed 67b18ca, which removes some redundant work without changing behavior: __copy__ reuses the constructor, __setitem__ no longer scans the elements of a ReadOnlyList (this runs on every on() call through Props.add_to), and freezing no longer copies dict items or builds temporary lists.

Two larger suggestions I left for you, since they rework the design:

1. One freeze method instead of two code paths. Propagating the flag is currently split between the module function _set_readonly and AttrsDict.__setattr__("__readonly__"), which call each other. The "never unfreeze shared children" rule is also repeated in __init__ and map(). A single one-way AttrsDict._freeze() method (recursive through dicts and lists, no warning) could be the only mechanism. __setattr__("__readonly__", ...) would then be a thin wrapper that warns when unfreezing, __init__ and map() would just call self._freeze(), and textdb.py would no longer need to import the private _set_readonly.
2. Freeze while loading, not after. TextDB.__getitem__ builds the AttrsDict (one walk of the tree) and then freezes it (a second walk). A quick benchmark with 5000 entries shows freezing adds about 50% to the load time of each file. Building the frozen objects directly in the same pass would avoid that. The overall on() speedup you measured suggests this is not urgent.

Verified locally: nox -s tests (60 passed) and pre-commit run -a.

[gipert]

Suggestion 1 from my previous comment is now implemented in 35211c1. The walk over a dict's contents lives only in _set_readonly(d, flag), and setting __readonly__ only adds the unfreeze warning before calling it, so the suppress_warning argument is gone. ReadOnlyList now makes its contents read-only when it is built, so TextDB freezes a loaded file with loaded.__readonly__ = True or ReadOnlyList(loaded) and no longer imports a private helper. Suggestion 2 (freezing while loading) is still open. Verified locally: nox -s tests (60 passed), pre-commit run -a and nox -s pylint (10.00/10).