User1342 commited on
Commit
1aab82b
·
1 Parent(s): 5703731

Meet the model where it writes: a palette either way round, a row separator it can type, several conversions in one call, and a date unit that defaults

Browse files
.gitignore CHANGED
@@ -1,4 +1,6 @@
1
  __pycache__/
 
 
2
  *.py[cod]
3
  .env
4
  .DS_Store
 
1
  __pycache__/
2
+ .pytest_cache/
3
+ .ruff_cache/
4
  *.py[cod]
5
  .env
6
  .DS_Store
distinct_skills/handlers.py CHANGED
@@ -312,7 +312,8 @@ def csv_table(arguments: Mapping[str, object], skill: SkillDefinition) -> dict[s
312
  raise SkillInputError(
313
  "text needs a header line and at least one row, each on its own line. "
314
  "Put a real line break between rows, like "
315
- "\"name|qty|aisle\\nflour|500 g|baking\""
 
316
  )
317
  if len(lines) > MAX_TABLE_ROWS + 1:
318
  raise SkillInputError(f"text must contain at most {MAX_TABLE_ROWS} rows")
@@ -371,6 +372,37 @@ def csv_table(arguments: Mapping[str, object], skill: SkillDefinition) -> dict[s
371
  # --------------------------------------------------------------------------
372
 
373
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
374
  def unescape_line_breaks(text: str) -> str:
375
  """Treat a literal backslash-n as the line break the caller meant.
376
 
@@ -392,7 +424,17 @@ def unescape_line_breaks(text: str) -> str:
392
 
393
  if "\n" in text or "\r" in text:
394
  return text
395
- return text.replace("\\r\\n", "\n").replace("\\n", "\n")
 
 
 
 
 
 
 
 
 
 
396
 
397
 
398
  def fit_row(cells: list[str], width: int, number: int) -> tuple[list[str], str]:
@@ -518,7 +560,8 @@ def _rows_from_text(arguments: Mapping[str, object]) -> tuple[list[list[str]], l
518
  raise SkillInputError(
519
  "text needs a header line and at least one row, each on its own line. "
520
  "Put a real line break between rows, like "
521
- "\"name|qty|aisle\\nflour|500 g|baking\""
 
522
  )
523
  rows = [[cell.strip() for cell in line.split(separator)] for line in lines]
524
  width = len(rows[0])
@@ -1016,25 +1059,22 @@ def theme_css(arguments: Mapping[str, object], skill: SkillDefinition) -> dict[s
1016
  # table rows: adjust, never silently.
1017
  variables = []
1018
  skipped: list[str] = []
 
1019
  for line in colors_text.splitlines():
1020
  line = line.strip()
1021
  if not line:
1022
  continue
1023
- key, _, value = line.partition(":")
1024
- key = re.sub(r"[\s_]+", "-", key.strip().lower()).strip("-")
1025
- value = value.strip()
1026
- if not re.fullmatch(r"#[0-9A-Fa-f]{3,8}", value) and re.fullmatch(
1027
- r"[0-9A-Fa-f]{6}|[0-9A-Fa-f]{3}", value
1028
- ):
1029
- # A hex value with the hash left off is the commonest near-miss and
1030
- # is unambiguous, so it is completed rather than discarded.
1031
- value = f"#{value}"
1032
- if not re.fullmatch(r"[a-z][a-z0-9-]{0,30}", key):
1033
- skipped.append(f"{key or line!r}: not a usable colour name")
1034
- continue
1035
- if not re.fullmatch(r"#[0-9A-Fa-f]{3,8}", value):
1036
- skipped.append(f"{key}: {value!r} is not a #hex value")
1037
  continue
 
 
 
 
 
 
 
1038
  variables.append(f" --{key}: {value};")
1039
  if not variables:
1040
  raise SkillInputError(
 
312
  raise SkillInputError(
313
  "text needs a header line and at least one row, each on its own line. "
314
  "Put a real line break between rows, like "
315
+ "\"name|qty|aisle\\nflour|500 g|baking\". "
316
+ "If you cannot write a line break, separate the rows with ' // ' instead."
317
  )
318
  if len(lines) > MAX_TABLE_ROWS + 1:
319
  raise SkillInputError(f"text must contain at most {MAX_TABLE_ROWS} rows")
 
372
  # --------------------------------------------------------------------------
373
 
374
 
375
+ #: A colour value, with or without the hash a model forgets.
376
+ _HEX = re.compile(r"#?[0-9A-Fa-f]{3,8}")
377
+
378
+
379
+ def _read_colour_line(line: str) -> tuple[str, str, str]:
380
+ """One palette line as ``(name, #hex, problem)``, however it was written.
381
+
382
+ "#121212: background" is the same palette entry as "background: #121212",
383
+ and refusing it lost a whole stylesheet three calls running. So whichever
384
+ half is a colour is the colour and the other half is the name, the hash is
385
+ added when it was left off, and a line with no colour in it at all is the
386
+ only one that comes back as a problem.
387
+ """
388
+
389
+ left, separator, right = line.partition(":")
390
+ if not separator:
391
+ left, _, right = line.partition(" ")
392
+ parts = [part.strip() for part in (left, right) if part.strip()]
393
+ if not parts:
394
+ return "", "", f"{line!r}: nothing to read"
395
+
396
+ value = next((part for part in parts if _HEX.fullmatch(part)), "")
397
+ if not value:
398
+ return "", "", f"{line!r}: no #hex colour in this line"
399
+ name = next((part for part in parts if part != value), "")
400
+ name = re.sub(r"[\s_]+", "-", name.lower()).strip("-")
401
+ if name and not re.fullmatch(r"[a-z][a-z0-9-]{0,30}", name):
402
+ return "", "", f"{name!r}: not a usable colour name"
403
+ return name, value if value.startswith("#") else f"#{value}", ""
404
+
405
+
406
  def unescape_line_breaks(text: str) -> str:
407
  """Treat a literal backslash-n as the line break the caller meant.
408
 
 
424
 
425
  if "\n" in text or "\r" in text:
426
  return text
427
+ unescaped = text.replace("\\r\\n", "\n").replace("\\n", "\n")
428
+ if "\n" in unescaped:
429
+ return unescaped
430
+ # STILL ONE LINE. A model held to a JSON grammar has to emit a two-
431
+ # character escape to get a line break, and some of them simply will not.
432
+ # " // " is offered in the refusal message as the way out, cannot appear in
433
+ # a well-formed row of a pipe-separated table, and is only ever consulted
434
+ # for a text that has no line breaks at all -- which is a text this tool
435
+ # was about to refuse outright. So it can rescue a table and cannot damage
436
+ # one.
437
+ return unescaped.replace(" // ", "\n") if " // " in unescaped else unescaped
438
 
439
 
440
  def fit_row(cells: list[str], width: int, number: int) -> tuple[list[str], str]:
 
560
  raise SkillInputError(
561
  "text needs a header line and at least one row, each on its own line. "
562
  "Put a real line break between rows, like "
563
+ "\"name|qty|aisle\\nflour|500 g|baking\". "
564
+ "If you cannot write a line break, separate the rows with ' // ' instead."
565
  )
566
  rows = [[cell.strip() for cell in line.split(separator)] for line in lines]
567
  width = len(rows[0])
 
1059
  # table rows: adjust, never silently.
1060
  variables = []
1061
  skipped: list[str] = []
1062
+ unnamed = 0
1063
  for line in colors_text.splitlines():
1064
  line = line.strip()
1065
  if not line:
1066
  continue
1067
+ key, value, note = _read_colour_line(line)
1068
+ if note:
1069
+ skipped.append(note)
 
 
 
 
 
 
 
 
 
 
 
1070
  continue
1071
+ if not key:
1072
+ # A bare hex with no name at all. Naming it positionally invents a
1073
+ # token name and nothing else, which is disclosed below, and is a
1074
+ # better outcome than a stylesheet nobody gets.
1075
+ unnamed += 1
1076
+ key = f"colour-{unnamed}"
1077
+ skipped.append(f"{value} had no name and was called --{key}")
1078
  variables.append(f" --{key}: {value};")
1079
  if not variables:
1080
  raise SkillInputError(
distinct_tools/local.py CHANGED
@@ -521,28 +521,63 @@ CONVERT_UNITS_SPEC = ToolSpec(
521
  "Both units must measure the same kind of thing; converting kilograms to metres "
522
  "is refused rather than guessed. "
523
  "Do not use it for plain arithmetic (use calculate) or for date arithmetic "
524
- "(use calculate_date). One call gives the exact answer, so do not repeat it."
 
 
525
  ),
526
  required_hosts=frozenset(),
527
  input_schema={
528
  "type": "object",
529
  "additionalProperties": False,
530
- "required": ["value", "from_unit", "to_unit"],
531
  "properties": {
532
  "value": {"type": "number", "description": "The quantity to convert."},
533
  "from_unit": {"type": "string", "enum": list(_ALL_UNITS)},
534
  "to_unit": {"type": "string", "enum": list(_ALL_UNITS)},
 
 
 
 
 
 
 
 
 
 
 
 
 
 
535
  },
536
  },
537
  )
538
 
 
 
 
 
539
 
540
  def convert_units_handler(
541
  arguments: Mapping[str, object], context: ToolContext
542
  ) -> dict[str, object]:
543
- """Convert between units of the same dimension, exactly where possible."""
 
 
 
544
 
545
- _reject_unknown(arguments, frozenset({"value", "from_unit", "to_unit"}))
 
 
 
 
 
 
 
 
 
 
 
 
 
546
  raw = arguments.get("value")
547
  if isinstance(raw, bool) or not isinstance(raw, int | float):
548
  raise ToolInputError("value must be a number")
@@ -600,6 +635,37 @@ def convert_units_handler(
600
  }
601
 
602
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
603
  # --------------------------------------------------------------------------
604
  # calculate_date
605
  # --------------------------------------------------------------------------
@@ -696,7 +762,18 @@ def calculate_date_handler(
696
  raise ToolInputError("amount must be a whole number for operation='add'")
697
  if not -400_000 <= amount <= 400_000:
698
  raise ToolInputError("amount must be between -400000 and 400000")
699
- unit = _require_choice(arguments, "unit", _DATE_UNITS)
 
 
 
 
 
 
 
 
 
 
 
700
  try:
701
  result = date + _datetime.timedelta(days=amount * _DATE_UNITS[unit])
702
  except (OverflowError, ValueError) as exc:
 
521
  "Both units must measure the same kind of thing; converting kilograms to metres "
522
  "is refused rather than guessed. "
523
  "Do not use it for plain arithmetic (use calculate) or for date arithmetic "
524
+ "(use calculate_date). One call gives the exact answer, so do not repeat it. "
525
+ "To convert several quantities, put them all in `conversions` in ONE call "
526
+ "rather than calling this tool once per quantity."
527
  ),
528
  required_hosts=frozenset(),
529
  input_schema={
530
  "type": "object",
531
  "additionalProperties": False,
 
532
  "properties": {
533
  "value": {"type": "number", "description": "The quantity to convert."},
534
  "from_unit": {"type": "string", "enum": list(_ALL_UNITS)},
535
  "to_unit": {"type": "string", "enum": list(_ALL_UNITS)},
536
+ "conversions": {
537
+ "type": "array",
538
+ "description": "Several conversions at once, instead of value/from_unit/to_unit.",
539
+ "items": {
540
+ "type": "object",
541
+ "additionalProperties": False,
542
+ "required": ["value", "from_unit", "to_unit"],
543
+ "properties": {
544
+ "value": {"type": "number"},
545
+ "from_unit": {"type": "string", "enum": list(_ALL_UNITS)},
546
+ "to_unit": {"type": "string", "enum": list(_ALL_UNITS)},
547
+ },
548
+ },
549
+ },
550
  },
551
  },
552
  )
553
 
554
+ #: How many conversions one call may carry. Generous for any real request and
555
+ #: still a bound, because a list is a way to ask for arbitrary work in one go.
556
+ MAX_CONVERSIONS = 24
557
+
558
 
559
  def convert_units_handler(
560
  arguments: Mapping[str, object], context: ToolContext
561
  ) -> dict[str, object]:
562
+ """Convert between units of the same dimension, exactly where possible.
563
+
564
+ SIX CONVERSIONS ASKED FOR AT ONCE WERE SIX CALLS, AND THE MODEL STOPPED
565
+ AFTER THREE.
566
 
567
+ "Convert these six quantities" is one request, and making it six calls put
568
+ the burden of remembering how many were left on the part of the system
569
+ least able to carry it. A benchmark workload failed that way every time:
570
+ three conversions done correctly, the other three simply never attempted,
571
+ and an answer confidently presenting half the list. Accepting the list in
572
+ one call moves the counting to code, which can count.
573
+
574
+ The single form is untouched, so nothing that worked before changes.
575
+ """
576
+
577
+ _reject_unknown(arguments, frozenset({"value", "from_unit", "to_unit", "conversions"}))
578
+ listed = arguments.get("conversions")
579
+ if listed is not None:
580
+ return _convert_many(listed, context)
581
  raw = arguments.get("value")
582
  if isinstance(raw, bool) or not isinstance(raw, int | float):
583
  raise ToolInputError("value must be a number")
 
635
  }
636
 
637
 
638
+ def _convert_many(listed: object, context: ToolContext) -> dict[str, object]:
639
+ """Every conversion in the list, or a refusal naming the one that failed.
640
+
641
+ One bad entry does not lose the others: each carries its own answer or its
642
+ own reason, in the order they were asked for, so a model reading the result
643
+ can fix the one it got wrong rather than starting again.
644
+ """
645
+
646
+ if not isinstance(listed, list | tuple) or not listed:
647
+ raise ToolInputError("conversions must be a non-empty list")
648
+ if len(listed) > MAX_CONVERSIONS:
649
+ raise ToolInputError(f"conversions may contain at most {MAX_CONVERSIONS} entries")
650
+ results: list[dict[str, object]] = []
651
+ for index, entry in enumerate(listed, start=1):
652
+ if not isinstance(entry, Mapping):
653
+ results.append({"position": index, "error": "each conversion must be an object"})
654
+ continue
655
+ try:
656
+ answer = convert_units_handler(dict(entry), context)
657
+ except ToolInputError as exc:
658
+ results.append({"position": index, "error": str(exc)})
659
+ continue
660
+ answer["position"] = index
661
+ results.append(answer)
662
+ return {
663
+ "conversions": results,
664
+ "converted": sum(1 for item in results if "result" in item),
665
+ "refused": sum(1 for item in results if "error" in item),
666
+ }
667
+
668
+
669
  # --------------------------------------------------------------------------
670
  # calculate_date
671
  # --------------------------------------------------------------------------
 
762
  raise ToolInputError("amount must be a whole number for operation='add'")
763
  if not -400_000 <= amount <= 400_000:
764
  raise ToolInputError("amount must be between -400000 and 400000")
765
+ # AN OPTIONAL ARGUMENT THAT IS REQUIRED IN PRACTICE IS A TRAP.
766
+ #
767
+ # ``unit`` is not in the schema's ``required`` list, so a model reading the
768
+ # schema leaves it out -- and then "add 45 to this date" was refused with
769
+ # "unit must be a string", which reads like the model sent the wrong type
770
+ # rather than nothing at all. Days is what "add 45" means to everybody, so
771
+ # it is the default, and the answer says which unit it used either way.
772
+ unit = (
773
+ _require_choice(arguments, "unit", _DATE_UNITS)
774
+ if arguments.get("unit") is not None
775
+ else "days"
776
+ )
777
  try:
778
  result = date + _datetime.timedelta(days=amount * _DATE_UNITS[unit])
779
  except (OverflowError, ValueError) as exc:
tests/test_ragged_rows_still_make_a_table.py CHANGED
@@ -154,7 +154,31 @@ def test_a_colour_that_cannot_be_read_is_skipped_and_named() -> None:
154
  sheet, skipped = _theme("primary: #1a1a2e\naccent: rgb(20,20,30)")
155
  assert "--primary: #1a1a2e;" in sheet
156
  assert "--accent" not in sheet
157
- assert skipped and "accent" in skipped[0]
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
158
 
159
 
160
  def test_a_palette_with_nothing_usable_is_still_refused() -> None:
@@ -212,3 +236,130 @@ def test_the_spreadsheet_accepts_the_same_one_line_form() -> None:
212
 
213
  result = spreadsheet_xlsx({"text": "a|b\\n1|2\\n3|4"}, None)
214
  assert result["artifact"]["size_bytes"] > 0
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
154
  sheet, skipped = _theme("primary: #1a1a2e\naccent: rgb(20,20,30)")
155
  assert "--primary: #1a1a2e;" in sheet
156
  assert "--accent" not in sheet
157
+ assert skipped and "rgb" in skipped[0]
158
+
159
+
160
+ def test_a_palette_written_hex_first_is_read_the_right_way_round() -> None:
161
+ """"#121212: background" lost a whole stylesheet, three calls running."""
162
+
163
+ sheet, skipped = _theme("#121212: background\n#3e8e56: accent")
164
+ assert "--background: #121212;" in sheet
165
+ assert "--accent: #3e8e56;" in sheet
166
+ assert skipped == []
167
+
168
+
169
+ def test_a_palette_written_without_a_colon_is_still_read() -> None:
170
+ sheet, _skipped = _theme("background #121212\naccent #3e8e56")
171
+ assert "--background: #121212;" in sheet
172
+ assert "--accent: #3e8e56;" in sheet
173
+
174
+
175
+ def test_a_bare_hex_gets_a_positional_name_and_the_naming_is_disclosed() -> None:
176
+ """Naming an unnamed thing is not inventing data, but it is still declared."""
177
+
178
+ sheet, skipped = _theme("#121212\n#3e8e56")
179
+ assert "--colour-1: #121212;" in sheet
180
+ assert "--colour-2: #3e8e56;" in sheet
181
+ assert len(skipped) == 2 and "had no name" in skipped[0]
182
 
183
 
184
  def test_a_palette_with_nothing_usable_is_still_refused() -> None:
 
236
 
237
  result = spreadsheet_xlsx({"text": "a|b\\n1|2\\n3|4"}, None)
238
  assert result["artifact"]["size_bytes"] > 0
239
+
240
+
241
+ def test_rows_can_be_separated_the_way_the_refusal_offers() -> None:
242
+ """A model held to a JSON grammar sometimes will not emit a line break."""
243
+
244
+ text, adjusted = _csv("name|qty|aisle // flour|500 g|baking // salt|10 g|baking")
245
+ assert "flour,500 g,baking" in text
246
+ assert "salt,10 g,baking" in text
247
+ assert adjusted == []
248
+
249
+
250
+ def test_the_offered_separator_is_only_consulted_when_there_are_no_line_breaks() -> None:
251
+ """It can rescue a table this tool was about to refuse; it cannot damage one."""
252
+
253
+ from distinct_skills.handlers import unescape_line_breaks
254
+
255
+ assert unescape_line_breaks("a // b\nc // d") == "a // b\nc // d"
256
+
257
+
258
+ def test_the_refusal_names_that_way_out() -> None:
259
+ from distinct_skills.handlers import csv_table
260
+
261
+ with pytest.raises(SkillInputError, match=r"' // '"):
262
+ csv_table({"text": "one line, no rows"}, None)
263
+
264
+
265
+ # -- an optional argument that was required in practice ----------------------
266
+
267
+
268
+ def test_adding_to_a_date_without_naming_a_unit_means_days() -> None:
269
+ """"unit must be a string" read as a wrong type when nothing had been sent."""
270
+
271
+ from distinct_tools.local import calculate_date_handler
272
+
273
+ answer = calculate_date_handler(
274
+ {"operation": "add", "date": "2026-03-03", "amount": 45}, None
275
+ )
276
+ assert answer["result"] == "2026-04-17"
277
+ assert answer["unit"] == "days"
278
+
279
+
280
+ def test_a_unit_that_is_given_is_still_honoured() -> None:
281
+ from distinct_tools.local import calculate_date_handler
282
+
283
+ answer = calculate_date_handler(
284
+ {"operation": "add", "date": "2026-03-03", "amount": 2, "unit": "weeks"}, None
285
+ )
286
+ assert answer["result"] == "2026-03-17"
287
+
288
+
289
+ def test_a_unit_that_is_not_a_unit_is_still_refused() -> None:
290
+ from distinct_tools.core import ToolInputError
291
+ from distinct_tools.local import calculate_date_handler
292
+
293
+ with pytest.raises(ToolInputError):
294
+ calculate_date_handler(
295
+ {"operation": "add", "date": "2026-03-03", "amount": 2, "unit": "fortnights"}, None
296
+ )
297
+
298
+
299
+ # -- several things asked for at once ----------------------------------------
300
+
301
+
302
+ def _convert(arguments):
303
+ from distinct_tools.local import convert_units_handler
304
+
305
+ return convert_units_handler(arguments, None)
306
+
307
+
308
+ def test_six_conversions_are_one_call_and_all_six_are_done() -> None:
309
+ """The model did three and stopped. Counting is code's job, not the model's."""
310
+
311
+ answer = _convert(
312
+ {
313
+ "conversions": [
314
+ {"value": n, "from_unit": "lb", "to_unit": "kg"} for n in range(1, 7)
315
+ ]
316
+ }
317
+ )
318
+ assert answer["converted"] == 6
319
+ assert answer["refused"] == 0
320
+ assert [item["position"] for item in answer["conversions"]] == [1, 2, 3, 4, 5, 6]
321
+
322
+
323
+ def test_one_bad_entry_does_not_lose_the_others() -> None:
324
+ answer = _convert(
325
+ {
326
+ "conversions": [
327
+ {"value": 140, "from_unit": "lb", "to_unit": "kg"},
328
+ {"value": 3, "from_unit": "kg", "to_unit": "m"},
329
+ {"value": 5, "from_unit": "mi", "to_unit": "km"},
330
+ ]
331
+ }
332
+ )
333
+ assert answer["converted"] == 2
334
+ assert answer["refused"] == 1
335
+ assert "different things" in answer["conversions"][1]["error"]
336
+
337
+
338
+ def test_the_single_form_is_untouched() -> None:
339
+ """Nothing that worked before changes."""
340
+
341
+ answer = _convert({"value": 140, "from_unit": "lb", "to_unit": "kg"})
342
+ assert answer["result"] == "63.5029318"
343
+ assert "conversions" not in answer
344
+
345
+
346
+ def test_an_empty_list_is_refused_rather_than_answered_with_nothing() -> None:
347
+ from distinct_tools.core import ToolInputError
348
+
349
+ with pytest.raises(ToolInputError):
350
+ _convert({"conversions": []})
351
+
352
+
353
+ def test_a_list_longer_than_the_ceiling_is_refused() -> None:
354
+ from distinct_tools.core import ToolInputError
355
+ from distinct_tools.local import MAX_CONVERSIONS
356
+
357
+ with pytest.raises(ToolInputError):
358
+ _convert(
359
+ {
360
+ "conversions": [
361
+ {"value": 1, "from_unit": "lb", "to_unit": "kg"}
362
+ ]
363
+ * (MAX_CONVERSIONS + 1)
364
+ }
365
+ )