feat: base model switched to roberta rebase

[Davidnet]

This is a rebase of #324 due to the building process was failing since it didn't had the react fixes

[Copilot]

Pull request overview

Rebases prior work to update the quantized model artifacts and training/upload configuration, aiming to unblock builds by incorporating missing fixes from the earlier PR.

Changes:

• Replaced/added multiple model/quantized/* artifacts (tokenizer/configs, label mappings, CRF transitions) and updated the LFS pointer for model_quantized.onnx.
• Updated the dataset upload script to additionally push audit_ledger.tsv via a git+LFS workflow.
• Changed training config to use an audit allowlist ledger path.

Reviewed changes

Copilot reviewed 10 out of 12 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
model/quantized/tokenizer_config.json	New tokenizer config content added (now checked in as JSON).
model/quantized/special_tokens_map.json	New special token mappings (now checked in as JSON).
model/quantized/ort_config.json	New ORT quantization config (now checked in as JSON).
model/quantized/model_quantized.onnx	Updated Git LFS pointer to a new quantized ONNX artifact (size increased).
model/quantized/label_mappings.json	Added label mapping JSON content (now checked in as JSON).
model/quantized/crf_transitions.json	Added large CRF transition matrix artifact.
model/quantized/added_tokens.json	Added extra token mapping for [MASK].
model/flows/training_config.toml	Set audit_allowlist to a specific local path.
model/dataset/huggingface/upload_dataset_to_hf.py	Added git-based upload path for audit_ledger.tsv using LFS.
.gitattributes	Added *.onnx LFS rule (repo already has model/quantized/** LFS rule).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

audit_allowlist is set to an absolute, machine-specific path. This will break other developers and CI runs that don’t have /home/hannes/....

Use a repo-relative path (e.g. under model/dataset/...) or make this configurable via an environment variable / CLI arg, and keep the default as an empty string (no filtering) or a relative path checked into the repo.

Suggested change

	audit_allowlist = "/home/hannes/kiji-proxy/model/dataset/data_samples/training_samples/audit_ledger.tsv"
	audit_allowlist = ""

[Copilot]

This embeds HF_TOKEN directly into the clone URL. That token can leak via process listings, shell history/logging, or Git error messages.

Prefer passing auth via GIT_ASKPASS/credential helper or git -c http.extraHeader="Authorization: Bearer …" clone ... so the token isn’t present in the command arguments/URL.

[Copilot]

git commit will fail (exit status 1) when there are no changes (e.g., the target file already exists with identical content). With check=True, that raises and aborts the upload.

Handle the no-op case explicitly (e.g., check git status --porcelain before committing, or allow a non-zero return code specifically for “nothing to commit”).

[Copilot]

This file is extremely large and is being committed as plain JSON rather than an LFS pointer, despite the repo’s .gitattributes rule model/quantized/** filter=lfs .... This will significantly inflate the Git repo size and make diffs/reviews impractical.

Re-add this artifact through Git LFS so the repo stores only the pointer file.

[Copilot]

PR metadata says the base model was switched to RoBERTa, but this tokenizer config is for DeBERTa/SentencePiece (tokenizer_class is DebertaV2Tokenizer, vocab_type is spm, and special tokens are BERT-style [CLS]/[SEP]).

Confirm the checked-in quantized artifacts match the intended base model; if this PR is meant to be a RoBERTa switch, these configs/tokenizer files likely need to come from the RoBERTa export instead.

[hanneshapke]

The confusion comes from the branch name feat/base-model-switched-to-roberta-rebase — which is misleading. The branch likely started as a RoBERTa experiment but the model was switched (back) to DeBERTa. The branch name is
stale, but the code and artifacts are internally consistent.

No code changes needed. You may want to rename the branch if the name is causing confusion in reviews:

git branch -m feat/base-model-switched-to-roberta-rebase feat/pii-model-training-updates

[Copilot]

Pull request overview

Copilot reviewed 13 out of 15 changed files in this pull request and generated 5 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This embeds the Hugging Face token directly in the git clone URL. That token can leak via process listings, crash logs, or git remote config inside the cloned repo. Prefer passing credentials via GIT_ASKPASS/GIT_TERMINAL_PROMPT=0 or git -c http.extraheader=Authorization: Bearer ... so the token isn't part of the command-line URL.

[Copilot]

ext = file_path.suffix can be empty (e.g., files without an extension). In that case lfs_pattern becomes * filter=lfs ..., which would unintentionally track all files in the HF repo with LFS. Add a guard to require a non-empty suffix (or accept an explicit pattern) before writing .gitattributes.

Suggested change

	ext = file_path.suffix # e.g. ".tsv"
	ext = file_path.suffix # e.g. ".tsv"
	if not ext:
	raise ValueError(
	f"Cannot infer an LFS tracking pattern for '{file_path.name}' because it has no file extension. "
	"Refusing to write a catch-all '*' rule to .gitattributes."
	)

[Copilot]

The shutil.copy(file_path, repo_dir / path_in_repo) call will fail if path_in_repo includes directories that don't exist in the cloned repo. Consider ensuring parent.mkdir(parents=True, exist_ok=True) before copying to make this helper safe for non-root paths.

Suggested change

	shutil.copy(file_path, repo_dir / path_in_repo)
	destination_path = repo_dir / path_in_repo
	destination_path.parent.mkdir(parents=True, exist_ok=True)
	shutil.copy(file_path, destination_path)

[Copilot]

git commit will fail with a non-zero exit code when there are no changes to commit (e.g., re-running the script after the file already exists), which will abort the upload. Consider checking git status --porcelain and skipping commit/push when clean, or using git commit --allow-empty only when appropriate.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 15 changed files in this pull request and generated 5 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

ln -sfn ... node_modules/<pkg> will fail (and abort due to set -e) if node_modules/<pkg> already exists as a real directory (e.g., when deps are installed locally rather than hoisted). Consider only creating these symlinks when the destination is missing, or remove/rename existing directories before linking, so packaging works in both workspace-hoisted and non-hoisted setups.

Suggested change

	ln -sfn ../../../node_modules/electron node_modules/electron
	ln -sfn ../../../node_modules/electron-builder node_modules/electron-builder
	ln -sfn ../../../node_modules/electron-updater node_modules/electron-updater
	ln -sfn ../../../node_modules/@sentry node_modules/@sentry
	create_safe_symlink() {
	local source_path="$1"
	local dest_path="$2"
	if [ -L "$dest_path" ]; then
	ln -sfn "$source_path" "$dest_path"
	elif [ ! -e "$dest_path" ]; then
	ln -s "$source_path" "$dest_path"
	else
	echo "ℹ️ Skipping symlink for $dest_path because it already exists and is not a symlink"
	fi
	}
	create_safe_symlink ../../../node_modules/electron node_modules/electron
	create_safe_symlink ../../../node_modules/electron-builder node_modules/electron-builder
	create_safe_symlink ../../../node_modules/electron-updater node_modules/electron-updater
	create_safe_symlink ../../../node_modules/@sentry node_modules/@sentry

[Copilot]

If the detected span is entirely whitespace, the trimming logic can end up with trimmedStart == trimmedEnd but leaves entity.Text set to the original whitespace substring, so it won't be filtered out by the later e.Text != "" check. Consider explicitly setting entity.Text = "" (and aligning positions) when trimming removes everything, or skipping the entity in that case.

Suggested change

	entity.Text = originalText[trimmedStart:trimmedEnd]
	entity.Text = originalText[trimmedStart:trimmedEnd]
	} else {
	entity.Text = ""

[Copilot]

audit_allowlist is set to an absolute, developer-specific path (/home/hannes/...), which will break training runs on other machines/CI. This should remain empty by default or be a repo-relative path (or configurable via env/CLI) so the config is portable.

Suggested change

	audit_allowlist = "/home/hannes/kiji-proxy/model/dataset/data_samples/training_samples/audit_ledger.tsv"
	audit_allowlist = ""

[Copilot]

model_manifest.json includes an entry for itself with a fixed sha256 value. If this manifest is generated by hashing files and then written to model_manifest.json, the recorded hash for model_manifest.json will be stale (it hashes the previous manifest). Consider excluding model_manifest.json from its own files list, or generating the manifest in two passes so the self-hash matches the final content.

Suggested change

	"path": "model_manifest.json",
	"sha256": "cc9036b411d9e17a5c15dd6d5387ad12cd7e7e9787a4812d7515ad8145b8a3a5"
	},
	{

[Copilot]

The symlink targets use ../../../node_modules/... while the script has cd src/frontend from the repo root, so ../../../node_modules resolves to the parent of the repo (likely wrong). Use a path based on $PROJECT_ROOT (or ../../node_modules) so the links reliably point at the repo-root node_modules.

Suggested change

	ln -sfn ../../../node_modules/electron node_modules/electron
	ln -sfn ../../../node_modules/electron-builder node_modules/electron-builder
	ln -sfn ../../../node_modules/electron-updater node_modules/electron-updater
	ln -sfn ../../../node_modules/@sentry node_modules/@sentry
	ln -sfn "$PROJECT_ROOT/node_modules/electron" node_modules/electron
	ln -sfn "$PROJECT_ROOT/node_modules/electron-builder" node_modules/electron-builder
	ln -sfn "$PROJECT_ROOT/node_modules/electron-updater" node_modules/electron-updater
	ln -sfn "$PROJECT_ROOT/node_modules/@sentry" node_modules/@sentry

[hanneshapke]

I updated the tokenization to get rid off the whitespace issue.

After the fix: