Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/snakemake/cli.py`:
- Around line 1579-1585: The CLI help for the --persistence-backend-db-url
argument currently includes a password-bearing example DSN; update the help text
in the group_behavior.add_argument call for "--persistence-backend-db-url" to
remove the password from the example (e.g., use "postgresql://user@host/db" or
"postgresql://host/db") and add a short note directing users to provide
credentials via profiles or environment-based configuration instead of passing
secrets on the command line.

In `@src/snakemake/persistence/__init__.py`:
- Around line 173-185: The cleanup code that wraps shutil.rmtree and Path.unlink
currently raises WorkflowError without chaining, losing the original exception;
modify the exception handlers around the rmtree call (in the block handling
removal of conda env at variable address) and the yaml unlink block
(yaml_path.unlink()) to catch OSError (or Exception if broader) and re-raise
WorkflowError using exception chaining (raise WorkflowError(f"...: {e}") from e)
so the original filesystem exception and traceback are preserved.

In `@src/snakemake/persistence/db.py`:
- Around line 142-157: The code eagerly creates a stub MetadataRecordORM in
_mark_incomplete which leaves behind empty rows when metadata retention is
disabled; update _mark_incomplete to avoid creating a new MetadataRecordORM when
only marking incomplete (i.e., only session.get(MetadataRecordORM, key) and set
incomplete/external_jobid on existing record, do not instantiate a new
MetadataRecordORM), and also harden _unmark_incomplete to remove stub records
created solely for the incomplete marker (detect records with
record_format_version == 0 and no provenance/metadata fields and call
session.delete(record) instead of clearing incomplete) so
PersistenceBase.finished() / has_metadata() no longer observes dropped metadata.

In `@src/snakemake/persistence/file.py`:
- Around line 240-259: _write_locks currently overwrites
self._lockfile[lock_type] so repeated calls orphan earlier lock files; change
the storage to keep multiple files per type (e.g., make
self._lockfile[lock_type] a list) and append each new lockfile instead of
assigning, using something like self._lockfile.setdefault(lock_type,
[]).append(lockfile) in _write_locks; update _delete_locks to iterate all stored
paths for each lock_type (handle lists of paths, remove each file while keeping
the same OSError handling) and then clear the mapping.
- Around line 138-152: _in _io_delete_, recdirs is computed as a path relative
to subject then passed to os.removedirs which treats paths relative to cwd;
change to call os.removedirs with the actual directory path of the removed
record (use os.path.dirname(recpath) or an absolute path derived from recpath)
instead of the relative recdirs, and keep the existing check so you only call
removedirs when the directory isn't "." to avoid removing unexpected
directories.

In `@src/snakemake/workflow.py`:
- Around line 917-924: The persistence selection currently passes a None db_url
and silently accepts unknown backends; update the code in the block that checks
self.workflow_settings.persistence_backend so that if the backend == "db" you
only include "db_url" in persistence_kwargs when
self.workflow_settings.persistence_backend_db_url is truthy (do not forward
None), and if the backend value is neither "db" nor "file" raise WorkflowError;
ensure you set persistence = DbPersistence (with persistence_kwargs possibly
empty) for "db" and persistence = FilePersistence for the explicit "file"
backend.

---

Nitpick comments:
In `@src/snakemake/persistence/file.py`:
- Around line 154-172: In _io_read remove the redundant recomputation of path:
compute path once by calling self._record_path(subject, id) at the top of the
function and reuse that variable for the os.path.exists check and the subsequent
open(), i.e. delete the second call to self._record_path(subject, id) so the
function uses the single path variable throughout.
- Around line 178-185: The `@lru_cache` on the instance method _read_record_cached
retains references to self and can leak FilePersistence instances; replace it
with an instance-level cache (e.g. self._read_record_cache dict) used inside
_read_record_cached to store/return results keyed by the method's key (and
include self._metadata_path if records vary by path), and update/extend the
existing _clear_cache() to clear that dict; locate usages in the FilePersistence
class (methods _read_record_cached and _clear_cache) and ensure thread-safety if
needed (e.g., simple locking) when accessing the cache.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/snakemake/persistence/db.py`:
- Around line 112-164: _read_record_cached is currently unbounded and not
invalidated on mutations; add a bounded cache and ensure mutations clear it.
Change `@lru_cache`(maxsize=None) on _read_record_cached to a bounded size like
`@lru_cache`(maxsize=1024) (or another appropriate maxsize for your workload), and
call self._clear_cache() at the end of each mutating method (_write_record,
_delete_record, _mark_incomplete, _unmark_incomplete) so the cached results are
invalidated after inserts/updates/deletes; keep the existing _clear_cache
implementation that calls self._read_record_cached.cache_clear().
- Around line 76-98: The DbPersistence __init__ currently ignores the provided
path when db_url is omitted; change the logic so if db_url is None (or omitted)
you construct the sqlite URL from the instance path: compute db_root = self.path
or Path.cwd(), db_file = (Path(db_root) / ".snakemake" /
"metadata.db").resolve(), then set self.engine =
create_engine(f"sqlite:///{db_file}") so the metadata DB lives under the
provided persistence root; reference DbPersistence.__init__, the db_url
parameter, and the self.engine creation when making this change.
- Around line 70-73: The current LockORM/table design is fine but _write_locks()
must not use session.merge() (an upsert) because that allows two concurrent
processes to overwrite each other's locks; change _write_locks() to attempt pure
inserts of new LockORM(...) rows (e.g., session.add_all or individual
session.add) and commit, catching sqlalchemy.exc.IntegrityError to detect
existing rows, then rollback and surface a lock-acquisition failure back to
PersistenceBase.lock(); remove/replace session.merge() calls so any duplicate
primary-key insert fails rather than upserting.

---

Nitpick comments:
In `@src/snakemake/persistence/__init__.py`:
- Around line 483-517: The loop over "for f in job.output" recomputes input file
checksums for every output; move the checksum collection out of the loop and
compute it once per job by evaluating the dict comprehension that builds
"checksums" (based on job.input and self.max_checksum_file_size) before the
loop, filter None values into "input_checksums", and then inside the loop assign
record.input_checksums = input_checksums; adjust placement so symbols _get_key,
_read_record, _write_record, MetadataRecord, RECORD_FORMAT_VERSION, and any uses
of endtime/fallback_time remain unchanged.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/snakemake/persistence/db.py`:
- Around line 166-186: Both _filter_incomplete_keys and _get_external_jobids
build a single large IN(...) which can exceed SQLite bind limits; change each to
process keys in chunks (e.g., chunk_size = 1000) by iterating over chunks of
keys_list, issuing a separate select per chunk (same where predicates:
MetadataRecordORM.target.in_(chunk) + the existing incomplete/external_jobid
filters), collecting and unioning results into the returned set, and avoid early
returns so empty input still returns set(). Ensure you deduplicate across chunks
and use Session(self.engine) for the batched queries in the same way they are
currently opened.

---

Duplicate comments:
In `@src/snakemake/persistence/db.py`:
- Around line 195-199: The _write_locks method currently uses
session.merge(LockORM(...)) which performs upsert semantics and can overwrite an
existing lock; change it to perform insert-only writes so a duplicate
primary-key insertion raises an IntegrityError and surfaces contention. Replace
session.merge(...) with session.add(...) / session.add_all(...) (creating
LockORM instances for each key) and let session.commit() raise on duplicates, or
explicitly catch sqlalchemy.exc.IntegrityError around the commit in _write_locks
to translate it into a clear lock-contention error; refer to the _write_locks
method, LockORM model and Session/engine when making the change.
- Around line 112-164: _read_record_cached uses lru_cache(maxsize=None) and is
never invalidated, so writes/deletes/marks leave stale or None results cached
and unbounded cache can grow unbounded; change the decorator to a bounded size
(e.g., `@lru_cache`(maxsize=1024)) and call self._clear_cache() at the end of
_write_record, _delete_record (before return), _mark_incomplete, and
_unmark_incomplete to invalidate the cache after any mutation so subsequent
_read_record calls see fresh data.